Closed Bug 1492716 Opened Last year Closed Last year

help text for --enable-optimize/--disable-optimize is wrong

Categories

(Firefox Build System :: General, enhancement, trivial)

enhancement
Not set
trivial

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(5 files, 1 obsolete file)

https://searchfox.org/mozilla-central/rev/94e37e71ffbfd39e6ad73ebcda5d77cce8d341ae/build/moz.configure/toolchain.configure#35-38
> js_option('--enable-optimize',
>           nargs='?',
>           default=True,
>           help='Enable optimizations via compiler flags')

> $ ./configure --help
> ...
> Usage: configure.py [options]
> 
> Options: [defaults in brackets after descriptions]
> ...
>   --disable-optimize        Enable optimizations via compiler flags

The help shows the opposite option --disable-optimize but keeps using the help for --enable-optimize
That's true for all --disable/--enable options where the default is more elaborated. That said, in this particular case, the default doesn't even vary via something else, so we could statically change it. (also, a lint for default=False or default=True might be good to have because that's kind of pointless when option('--disable-foo') is the same as option('--enable-foo', default=True)
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
prepared a patch, which fixes the help text with static default bool value, and also adds two help texts for non-static default value with bool.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=41266235cde4776b0c940efca44c0af6d1bf3cc8

will as review once the build succeeds on all environment
(at least ./configure --help works on macOS)
* Disallow --enable-* + default=True and --disable-* + default=False
  * added help_enable/help_disable for the case that default is function which returns bool,
    to provide help text for both case
Attachment #9016563 - Flags: review?(mh+mozilla)
Comment on attachment 9016563 [details] [diff] [review]
Fix configure help, and require providing two help strings for boolean option with non-static default.

Review of attachment 9016563 [details] [diff] [review]:
-----------------------------------------------------------------

This patch is doing to many different things at once. For one, I don't think the help_enable/help_disable/help_with/help_without multiplication of options is the way to go about that. Maybe something template-y in help, or something completely automatic. Or completely removing Enable/Disable/With/Without from the help and make all help strings generic. I haven't made my mind yet.

::: python/mozbuild/mozbuild/configure/options.py
@@ +207,5 @@
>                  elif default is False:
> +                    if not default_is_bool_func:
> +                        if prefix == 'disable':
> +                            raise InvalidOptionError(
> +                                '--enable-{name} should be used instead of --disable-{name} with default=False'.format(name=name))

I'd rather leave these checks to lint.py.
Attachment #9016563 - Flags: review?(mh+mozilla)
Attachment #9016563 - Attachment is obsolete: true
Attachment #9017465 - Attachment description: Bug 1492716 - Part 2: Add help_for_enable_disable for configure option with variable default value. r?glandium → Part 2: Add formatting rule to help text for --{enable,disable,with,without}. r?glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/86217b31f832d3033916dfec6474ef5d673e218d
Bug 1492716 - Part 1: Stop using default=True for --enable-* or --with-* configure options. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/e22dc234a52fe16f8ce2ae86b1b63b86211c1572
Bug 1492716 - Part 2: Add formatting rule to help text for --{enable,disable,with,without}. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/561373eda70f3fafcbea47304fcdd106aba12a83
Bug 1492716 - Part 2.1: Support empty string in help formatting rule and fixed some more cases. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/6630144571e3e9e28ae51d6dbede2d70fafc5e9e
Bug 1492716 - Part 3: Add linter for configure option with bool default. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea543fc770afe79edc4c11be6d83bb721c0ee18
Bug 1492716 - Part 4: Add linter for configure option with function default. r=glandium
You need to log in before you can comment on or make changes to this bug.