Closed Bug 1374727 Opened 7 years ago Closed 7 years ago

nonsensical behavior for check_prog/when/only_when interactions

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: froydnj, Assigned: glandium)

References

Details

Attachments

(1 file)

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.
Assignee: nobody → mh+mozilla
And surely enough, this breaks all builds except stylo, with:
  mozbuild.configure.options.InvalidOptionError: LLVM_CONFIG is not available in this configuration
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
Attachment #8879736 - Flags: review?(cmanchester) → review+
Blocks: 1370007
Depends on: 1382525
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/00ef8018730c
Apply check_prog's `when` to more of what it "expands" to. r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/00ef8018730c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.