I landed a patch in bug 1358215 that added an option that looked like: ``` option(env='MOZ_PHOTON_ANIMATIONS', help='Enable Photon UI animations', default=delayed_getattr(milestone, 'is_nightly')) ``` This worked fine except it failed the moz.configure lint test, like: [task 2017-05-09T13:45:15.615001Z] 13:45:15 INFO - ConfigureError: Missing @depends for `result`: '--help' Where `result` here is the function defined inside `delayed_getattr`: https://dxr.mozilla.org/mozilla-central/rev/b21b974d60d3075ae24f6fb1bae75d0f122f28fc/build/moz.configure/util.configure#381 I changed the patch to instead have: ``` @depends(milestone) def is_nightly(milestone): return milestone.is_nightly option(env='MOZ_PHOTON_ANIMATIONS', help='Enable Photon UI animations', default=is_nightly) ``` Which works and passes the lint checks, but I don't really understand the difference. Is the lint check doing the right thing here?

So, with the limitations in how things are detected, this is the expected behavior, sadly. The problem is that delayed_getattr introduces a closure and that's what we allow missing --help for: https://dxr.mozilla.org/mozilla-central/rev/b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5/python/mozbuild/mozbuild/configure/lint.py#84-87 Now, the surprising thing is that the following should trigger the same error but doesn't: https://dxr.mozilla.org/mozilla-central/rev/b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5/mobile/android/moz.configure#64-66 That suggests a bug in the linter.

(In reply to Mike Hommey [:glandium] from comment #1) > Now, the surprising thing is that the following should trigger the same > error but doesn't: > https://dxr.mozilla.org/mozilla-central/rev/ > b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5/mobile/android/moz.configure#64-66 > > That suggests a bug in the linter. Which is now filed as bug 1365477.

So, related to this, there was this part of my message from a few months ago on dev-builds (https://groups.google.com/d/msg/mozilla.dev.builds/XThvzlFdlKU/uUlHt5IxCQAJ): - It can be cumbersome to have to create @depends functions for simple conditionals. We have a bunch of templates that take a "when" argument. There also is imply_option, set_config, set_define and add_old_configure_assignment that all take @depends functions. In many places, we do ad-hoc things like depends(target)(lambda target: target.os == 'OSX') or delayed_getattr(milestone, 'is_nightly') I know Nick didn't like the magic, but after having written to many of those, I'm really considering magic would be nicer. What I was thinking was to make calling a depends function possible, and it would return a special class that create new pseudo depends function for common operations. Such that instead of depends(target)(lambda target: target.os == 'OSX') you could write: target().os == 'OSX' Instead of delayed_getattr(milestone, 'is_nightly') you could write: milestone().is_nightly Instead of @depends(a, b) def foo(a, b): return a or b you could write: foo = a() or b() etc. This would actually help here. Note that actually already have something like the last example since https://hg.mozilla.org/mozilla-central/rev/9c02acca893d .

The patch queue I just posted implements most of the proposals in comment 3, without the parens (such that milestone.is_nightly is equivalent to delayed_getattr(milestone, 'is_nightly'). The magic for == is not there, though, because it's not possible (python expects __cmp__/__eq__ to return an integer/bool and barfs otherwise) I'm not sure whether we want the parens, but it's not hard to add them (and retrospectively in what was added in https://hg.mozilla.org/mozilla-central/rev/9c02acca893d). This fixes the issue with default=delayed_getattr(a, 'b') by making the magic a.b never require a dependency on --help. One thing missing from the queue is replacing the is_nightly function with milestone.is_nightly.

Comment on attachment 8868430 [details] Bug 1363811 - Change TestConfigure.test_depends_or to test more cases. https://reviewboard.mozilla.org/r/140018/#review144252

Comment on attachment 8868431 [details] Bug 1363811 - Modify the name of the DependsFunction.__or__ implementation method. https://reviewboard.mozilla.org/r/140020/#review144256 ::: python/mozbuild/mozbuild/configure/__init__.py:133 (Diff revision 2) > > @staticmethod > - def first_true(iterable): > - # Like the builtin any(), but returns the first element that is true, > - # instead of True. If none are true, returns the last element. > + def or_impl(iterable): > + # Applies "or" to all the items of iterable. > + # e.g. if iterable contains a, b and c, returns `a or b or c`. > + # Equivalent to reduce(lambda a, b: a or b, iterable, False) I don't think this line adds much to the explanation. It also wouldn't be equivalent if we ever allowed this function to be called with an empty iterable.

Comment on attachment 8868432 [details] Bug 1363811 - Allow to combine two DependsFunctions with "&". https://reviewboard.mozilla.org/r/140022/#review144266

Comment on attachment 8868433 [details] Bug 1363811 - Allow "direct" access to namespace attributes from DependsFunctions. https://reviewboard.mozilla.org/r/140024/#review144272

Comment on attachment 8868434 [details] Bug 1363811 - Replace all uses of delayed_getattr(a, 'b') with a.b. https://reviewboard.mozilla.org/r/140026/#review144274 Much better!

Comment on attachment 8868451 [details] Bug 1363811 - Replace is_nightly with milestone.is_nightly. https://reviewboard.mozilla.org/r/140052/#review144276

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/fb362996cf3f Change TestConfigure.test_depends_or to test more cases. r=cmanchester+432261 https://hg.mozilla.org/integration/autoland/rev/aaad90777719 Modify the name of the DependsFunction.__or__ implementation method. r=cmanchester+432261 https://hg.mozilla.org/integration/autoland/rev/d517e034c953 Allow to combine two DependsFunctions with "&". r=cmanchester+432261 https://hg.mozilla.org/integration/autoland/rev/01aa12b787b2 Allow "direct" access to namespace attributes from DependsFunctions. r=cmanchester+432261 https://hg.mozilla.org/integration/autoland/rev/6d99c685aea3 Replace all uses of delayed_getattr(a, 'b') with a.b. r=cmanchester+432261 https://hg.mozilla.org/integration/autoland/rev/d4687badde12 Replace is_nightly with milestone.is_nightly. r=cmanchester+432261

https://hg.mozilla.org/mozilla-central/rev/fb362996cf3f https://hg.mozilla.org/mozilla-central/rev/aaad90777719 https://hg.mozilla.org/mozilla-central/rev/d517e034c953 https://hg.mozilla.org/mozilla-central/rev/01aa12b787b2 https://hg.mozilla.org/mozilla-central/rev/6d99c685aea3 https://hg.mozilla.org/mozilla-central/rev/d4687badde12