The lone check for NO_JS_MANIFEST in rules.mk suggests that we need to be looking at EXTRA_COMPONENTS/EXTRA_PP_COMPONENTS. But testing/mochitest/ doesn't define either of those variables, so NO_JS_MANIFEST is just taking up space in this instance.

This patch needs to be applied on top of the patch in bug 1155776. I'm a little out of my depth here as far as the description in context.py goes, so if you'd like to wordsmith that, be my guest.

Now that moz.build can see EXTRA_*COMPONENTS and NO_JS_MANIFEST, we can move some logic from rules.mk (executed every build) to moz.build's emitter.py (executed only at build-backend time). This is, in my opinion, a very cool patch, and it's a shame it's hidden in this mundane bug.

Comment on attachment 8594112 [details] [diff] [review] part 1 - move NO_JS_MANIFEST to moz.build Review of attachment 8594112 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozbuild/frontend/context.py @@ +1298,5 @@ > + Normally, if you have .js files listed in ``EXTRA_COMPONENTS`` or > + ``EXTRA_PP_COMPONENTS``, you are expected to have a corresponding > + .manifest file to go with those .js files. Setting ``NO_JS_MANIFEST`` > + indicates that the relevant .manifest file and entries for those .js > + files are elsehwere (jar.mn, for instance) and this state of affairs elsewhere

Comment on attachment 8594113 [details] [diff] [review] part 2 - move EXTRA_*COMPONENTS manifest check to build-backend time Review of attachment 8594113 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozbuild/frontend/emitter.py @@ +535,5 @@ > + 'or EXTRA_PP_COMPONENTS without a matching ' > + '.manifest file. See ' > + 'https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_2.0 .', > + context); > + ws

Comment on attachment 8594112 [details] [diff] [review] part 1 - move NO_JS_MANIFEST to moz.build The description looks fine as far as I can tell :).

Comment on attachment 8594113 [details] [diff] [review] part 2 - move EXTRA_*COMPONENTS manifest check to build-backend time Very nice! This also helps get the error message right at the end of the build output, rather than buried somewhere in the make invocation, which is awesome. The line after "context);" has some extraneous whitespace... I think Ms2ger would be sad if I didn't point it out. Maybe 'mv .git/hooks/pre-commit.sample .git/hooks/pre-commit' would help?

Comment on attachment 8594113 [details] [diff] [review] part 2 - move EXTRA_*COMPONENTS manifest check to build-backend time This got pushed to try along with other patches I had on top of it, so I noticed a couple of failures. Looks like the mozbuild python tests need updating to work with the new manifest detecting: TEST-UNEXPECTED-FAIL | c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/python/mozbuild/mozbuild/test/backend/test_recursivemake.py | line 257, test_variable_passthru: A .js component was specified in EXTRA_COMPONENTS or EXTRA_PP_COMPONENTS without a matching .manifest file. See https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_2.0 . TEST-UNEXPECTED-FAIL | c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/python/mozbuild/mozbuild/test/frontend/test_emitter.py | line 157, test_variable_passthru: A .js component was specified in EXTRA_COMPONENTS or EXTRA_PP_COMPONENTS without a matching .manifest file. See https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_2.0 . Try push is https://treeherder.mozilla.org/#/jobs?repo=try&revision=3358947a05bd, although these patches are in https://treeherder.mozilla.org/#/jobs?repo=try&revision=23643eec8e8f (which was busted because of my patches)

(In reply to Brian O'Keefe [:bokeefe] from comment #8) > This got pushed to try along with other patches I had on top of it, so I > noticed a couple of failures. Looks like the mozbuild python tests need > updating to work with the new manifest detecting: Thanks for the report, I'll fix that up prior to landing (just have to declare a .manifest file in EXTRA_COMPONENTS).

https://hg.mozilla.org/integration/mozilla-inbound/rev/dacd2e48fbfc https://hg.mozilla.org/integration/mozilla-inbound/rev/00a8bcfe966d https://hg.mozilla.org/integration/mozilla-inbound/rev/06d662c40b95

https://hg.mozilla.org/mozilla-central/rev/dacd2e48fbfc https://hg.mozilla.org/mozilla-central/rev/00a8bcfe966d https://hg.mozilla.org/mozilla-central/rev/06d662c40b95 https://hg.mozilla.org/mozilla-central/rev/95ad98c71c5b