1374727 - nonsensical behavior for check_prog/when/only_when interactions

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

We currently have code similar to the following in mozilla-central:

@depends(stylo_config, '--enable-stylo-build-bindgen')
def building_stylo_bindgen(stylo_config, bindgen_enabled):
    if not bindgen_enabled:
        return False
    return bool(stylo_config.build)

llvm_config = check_prog('LLVM_CONFIG', ('llvm-config-4.0',
                                         'llvm-config40',
                                         'llvm-config-3.9',
                                         'llvm-config39',
                                         'llvm-config',),
                         when=building_stylo_bindgen,
                         what='llvm-config', allow_missing=True)

and despite having LLVM_CONFIG set in a mozconfig included by every automation config, we receive no complaints about LLVM_CONFIG being set in builds that do not build stylo.

If, however, we were to change this to:

with only_when(building_stylo_bindgen):
    llvm_config = check_prog('LLVM_CONFIG', ('llvm-config-4.0',
                                             'llvm-config40',
                                             'llvm-config-3.9',
                                             'llvm-config39',
                                             'llvm-config',),
                             what='llvm-config', allow_missing=True)
    ....

attempting to configure without building stylo as before will complain:

checking whether the target C compiler can be used... yes
Traceback (most recent call last):
  File "/home/froydnj/src/gecko-dev.git/configure.py", line 124, in <module>
    sys.exit(main(sys.argv))
  File "/home/froydnj/src/gecko-dev.git/configure.py", line 29, in main
    sandbox.run(os.path.join(os.path.dirname(__file__), 'moz.configure'))
  File "/home/froydnj/src/gecko-dev.git/python/mozbuild/mozbuild/configure/__init__.py", line 407, in run
    self._value_for(option)
  File "/home/froydnj/src/gecko-dev.git/python/mozbuild/mozbuild/configure/__init__.py", line 475, in _value_for
    return self._value_for_option(obj)
  File "/home/froydnj/src/gecko-dev.git/python/mozbuild/mozbuild/util.py", line 925, in method_call
    cache[args] = self.func(instance, *args)
  File "/home/froydnj/src/gecko-dev.git/python/mozbuild/mozbuild/configure/__init__.py", line 540, in _value_for_option
    % option_string.split('=', 1)[0])
mozbuild.configure.options.InvalidOptionError: LLVM_CONFIG is not available in this configuration

Having to unset all relevant environment variables that might conflict with build options is, IMHO, annoying behavior, but OK, we can live with that.

But it certainly doesn't *look* like there should be any difference between how the two pieces of code work.  Subtle differences like this make it annoying to write non-trivial pieces of moz.configure code.

Comment 1

[Mike Hommey [:glandium] (PTO. Back on Nov 25)]

Attachment: Bug 1374727 - Apply check_prog's `when` to more of what it "expands" to.

check_prog, when used with a `when` argument, doesn't work the same way
as putting it under a `with only_when()` block, while it should. The
difference comes from the fact that `with only_when()` applies the
`when` to every option and depends used in the block (which check_prog
calls a bunch of).

So, we "manually" apply the `when` to all option and depends in
check_prog. An alternative solution would be to put the whole function
under a `with only_when()` block, but that would mean reindenting the
whole function.

Either way, as a consequence, this requires the `when` to have a
dependency on --help for "non-trivial" functions, which fortunately,
there's only one of.

Review commit: https://reviewboard.mozilla.org/r/151076/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151076/

Comment 2

[Mike Hommey [:glandium] (PTO. Back on Nov 25)]

And surely enough, this breaks all builds except stylo, with:
  mozbuild.configure.options.InvalidOptionError: LLVM_CONFIG is not available in this configuration

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8879736 [details]
Bug 1374727 - Apply check_prog's `when` to more of what it "expands" to.

https://reviewboard.mozilla.org/r/151076/#review156920

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00ef8018730c
Apply check_prog's `when` to more of what it "expands" to. r=chmanchester

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/00ef8018730c