Closed
Bug 1256568
Opened 8 years ago
Closed 8 years ago
Move some MOZ_PATH_PROG to moz.configure
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
No description provided.
Assignee | ||
Updated•8 years ago
|
Blocks: pyconfigure
Assignee | ||
Comment 1•8 years ago
|
||
Because some of the existing mozconfigs may be setting some variables, we need to inject those that are handled by moz.configure now. It likely doesn't matter for the variables currently in moz.configure, but it will soon become important when more things are moved to moz.configure. In fact, it is necessary for GENISOIMAGE and DSYMUTIL that we're going to move in this bug, set in automation mozconfigs. The implementation is cumbersome and quite horrible. We could do better by changing the execution model in mozbuild.configure, which is probably necessary for other reasons as well, but that requires more work and testing. Review commit: https://reviewboard.mozilla.org/r/40017/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40017/
Attachment #8730569 -
Flags: review?(cmanchester)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40019/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40019/
Attachment #8730570 -
Flags: review?(cmanchester)
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40021/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40021/
Attachment #8730571 -
Flags: review?(cmanchester)
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40023/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40023/
Attachment #8730572 -
Flags: review?(cmanchester)
Comment 5•8 years ago
|
||
Comment on attachment 8730569 [details] MozReview Request: Bug 1256568 - Inject some variables set in mozconfig into moz.configure https://reviewboard.mozilla.org/r/40017/#review36527 And here I thought the new configure wouldn't have so many hacks :P ::: build/moz.configure/init.configure:241 (Diff revision 1) > + ) > + return early_options > + > +early_options = early_options() > + > +# At the moment, moz.configure doesn't have completely knowledge of all the Nit: "complete"
Attachment #8730569 -
Flags: review+
Comment 6•8 years ago
|
||
Comment on attachment 8730570 [details] MozReview Request: Bug 1256568 - Move awk detection to moz.configure https://reviewboard.mozilla.org/r/40019/#review36529 Cool. I bet this program checking will be significantly faster than shell on Windows! Should be pretty easy to port them all over in bulk too.
Attachment #8730570 -
Flags: review+
Comment 7•8 years ago
|
||
Comment on attachment 8730571 [details] MozReview Request: Bug 1256568 - Move perl detection to moz.configure https://reviewboard.mozilla.org/r/40021/#review36531 ::: js/src/old-configure.in (Diff revision 1) > > MOZ_BUILD_ROOT=`pwd -W 2>/dev/null || pwd -P` > > MOZ_DEFAULT_COMPILER > > -dnl Check for Perl first -- needed for win32 SDK checks If those referenced checks are in fact still written in Perl, I hope you rewrite them to Python when you port them. ::: moz.configure:100 (Diff revision 1) > + > + @depends(perl) > + @checking('for full perl installation') > + @advanced > + def has_full_perl_installation(perl): > + import subprocess I'm not too keen on all this delayed imports for stdlib modules that will almost certainly always be loaded. But if that's the style we're using for moz.configure, so be it. ::: moz.configure:111 (Diff revision 1) > + def require_full_perl_installation(has_full_perl_installation): > + if not has_full_perl_installation: > + error('Cannot find Config.pm or $Config{archlib}. ' > + 'A full perl installation is required.') > + > +perl_version_check('5.006') This is so ancient. Over 15 years actually. Also 5.006 was never actually released. There were various releases of 5.005. But never a 5.006. Since we're using >= in the version check, perhaps we should bump this to the version that was actually released.
Attachment #8730571 -
Flags: review+
Comment 8•8 years ago
|
||
Comment on attachment 8730572 [details] MozReview Request: Bug 1256568 - Move doxygen, zip, unzip, xargs, rpmbuild, genisoimage and dsymutil detection to moz.configure https://reviewboard.mozilla.org/r/40023/#review36533 ::: moz.configure:118 (Diff revision 1) > +check_prog('UNZIP', ('unzip',)) > +check_prog('ZIP', ('zip',)) > +check_prog('XARGS', ('xargs',)) > + > +check_prog('RPMBUILD', ('rpmbuild',), allow_missing=True) > +check_prog('GENISOIMAGE', ('genisoimage',), allow_missing=True) > +check_prog('DSYMUTIL', ('dsymutil', 'llvm-dsymutil'), allow_missing=True) Let's alphabetize these.
Attachment #8730572 -
Flags: review+
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #1) > The implementation is cumbersome and quite horrible. We could do better > by changing the execution model in mozbuild.configure, which is probably > necessary for other reasons as well, but that requires more work and > testing. Filed bug 1256571
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #7) > This is so ancient. Over 15 years actually. Also 5.006 was never actually > released. There were various releases of 5.005. But never a 5.006. Since > we're using >= in the version check, perhaps we should bump this to the > version that was actually released. This actually means 5.6. Which is still 16 years old, but actually exists. http://perldoc.perl.org/perlvar.html#General-Variables $] The revision, version, and subversion of the Perl interpreter, represented as a decimal of the form 5.XXXYYY, where XXX is the version / 1e3 and YYY is the subversion / 1e6. For example, Perl v5.10.1 would be "5.010001".
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #7) > I'm not too keen on all this delayed imports for stdlib modules that will > almost certainly always be loaded. But if that's the style we're using for > moz.configure, so be it. I've been thinking about replacing the @advanced decorator with a @import decorator. Which would have the double advantage of documenting which modules are required for the function without looking at its AST, and would solve your concern.
Assignee | ||
Updated•8 years ago
|
Attachment #8730569 -
Flags: review?(cmanchester)
Assignee | ||
Updated•8 years ago
|
Attachment #8730570 -
Flags: review?(cmanchester)
Assignee | ||
Updated•8 years ago
|
Attachment #8730571 -
Flags: review?(cmanchester)
Assignee | ||
Updated•8 years ago
|
Attachment #8730572 -
Flags: review?(cmanchester)
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b2a1bed177a https://hg.mozilla.org/integration/mozilla-inbound/rev/c71929474f66 https://hg.mozilla.org/integration/mozilla-inbound/rev/4a2a46087a28 https://hg.mozilla.org/integration/mozilla-inbound/rev/7fd99a35ac93
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #6) > Comment on attachment 8730570 [details] > MozReview Request: Bug 1256568 - Move awk detection to moz.configure > > https://reviewboard.mozilla.org/r/40019/#review36529 > > Cool. I bet this program checking will be significantly faster than shell on > Windows! Should be pretty easy to port them all over in bulk too. Note that I don't want to do blind porting. I already left it slip in this series that dsymutil, genisoimage and rpmbuild are platform-specific and should thus be in platform-specific sections. Will file a followup.
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8b2a1bed177a https://hg.mozilla.org/mozilla-central/rev/c71929474f66 https://hg.mozilla.org/mozilla-central/rev/4a2a46087a28 https://hg.mozilla.org/mozilla-central/rev/7fd99a35ac93
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/93df41fba9ac
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•