Bug 622402 – app-backup/bacula: overcomplex/confusing REQUIRED_USE

• Product: Gentoo Linux
• Status: CONFIRMED ---

[Description Michał Górny 2017-06-21 18:21:43 UTC]

The ebuilds for all versions use the following horrible construct:

  || ( ^^ ( mysql postgres sqlite ) bacula-clientonly )

Nesting ^^ in || is a bad idea. The same construct could be written more readably as:

  !bacula-clientonly? ( ^^ ( mysql postgres sqlite ) )

That would clearly indicate that the ^^ applies only to the case when bacula-clientonly flag is not used.

[Comment 1 Thomas Beierlein 2017-06-27 05:44:04 UTC]

(In reply to Michał Górny from comment #0)
> The ebuilds for all versions use the following horrible construct:
> 
>   || ( ^^ ( mysql postgres sqlite ) bacula-clientonly )
> 
> Nesting ^^ in || is a bad idea.

IIRC it was the suggested construct at the time REQUIRED_USE got introduced.

But you are right, it is not very clear. At least a set of additional parentheses would be helpful.


> The same construct could be written more
> readably as:
> 
>   !bacula-clientonly? ( ^^ ( mysql postgres sqlite ) )
> 
> That would clearly indicate that the ^^ applies only to the case when
> bacula-clientonly flag is not used.

Looks good to me. I will apply it from next version on.

In meantime I will keep the bug open as a reminder.

[Comment 2 Michał Górny 2017-06-27 06:29:27 UTC]

By the way, the modern way of doing it would be IUSE=+server instead of reversed logic 'clientonly' (=noserver) flag.

[Comment 3 Thomas Beierlein 2017-06-27 06:37:35 UTC]

(In reply to Michał Górny from comment #2)
> By the way, the modern way of doing it would be IUSE=+server instead of
> reversed logic 'clientonly' (=noserver) flag.

I know the USE flags are weird constructs and that is for two reasons:
- heritage of old ebuild version, but mainly
- inspired by the weird constructs from upstream (just have a look at 'configure --help').

I am planning to switch the USE flags with next major version. Atm I am thinking if we should prepare a news item for it.

[Comment 4 Michał Górny 2017-08-05 11:36:59 UTC]

Could we change this for all old versions, please? This package seems to have a practice of keeping them like forever, and this clutters the CI warning report. After all, per PMS rules both constructs are 100% equivalent.

[Comment 5 Thomas Beierlein 2017-08-06 09:04:43 UTC]

(In reply to Michał Górny from comment #4)
> Could we change this for all old versions, please? This package seems to
> have a practice of keeping them like forever, and this clutters the CI
> warning report. After all, per PMS rules both constructs are 100% equivalent.

Yes, I have seen it. I will change it in next days.

[Comment 6 Thomas Beierlein 2017-08-11 04:49:12 UTC]

Fixed versions in tree now.