Closed
Bug 1272530
Opened 8 years ago
Closed 8 years ago
Move --{enable,disable}-tests to Python configure
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox49 fixed)
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: chmanchester, Assigned: chmanchester)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52878/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52878/
Attachment #8753570 -
Flags: review?(mh+mozilla)
Comment 2•8 years ago
|
||
Comment on attachment 8753570 [details] MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium https://reviewboard.mozilla.org/r/52878/#review50216 ::: moz.configure:86 (Diff revision 1) > +@depends('--disable-compile-environment', '--disable-tests', '--help') > +def test_defines_include(compile_env, enable_tests, help): > + if compile_env and enable_tests: > + return 'build/moz.configure/testdefines.configure' > + > +include(test_defines_include) For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file. Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs.
Attachment #8753570 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/52878/#review50216 > For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file. > > Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs. Setting _VARIADIC_MAX is gated on the compiler type in the original implementation, and we can't depend on c_compiler et al. when --disable-compile-environment is set. I already didn't port the AC_SUBSTs for that reason (testdefines.configure as it is in this patch only uses set_define).
Comment 4•8 years ago
|
||
(In reply to Chris Manchester (:chmanchester) from comment #3) > https://reviewboard.mozilla.org/r/52878/#review50216 > > > For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file. > > > > Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs. > > Setting _VARIADIC_MAX is gated on the compiler type in the original > implementation, and we can't depend on c_compiler et al. when > --disable-compile-environment is set. After some research, here's the good news: _VARIADIC_MAX was an implementation detail of simulated variadic templates in MSVC 2012. MSVC 2013 and 2015, which are the versions we support, have support for real variadic templates, and _VARIADIC_MAX is not necessary anymore. > > I already didn't port the AC_SUBSTs for that reason (testdefines.configure > as it is in this patch only uses set_define). Err, sorry, somehow, I got confused.
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8753570 [details] MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52878/diff/1-2/
Attachment #8753570 -
Flags: review?(mh+mozilla)
Comment 6•8 years ago
|
||
Comment on attachment 8753570 [details] MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium https://reviewboard.mozilla.org/r/52878/#review51624 ::: moz.configure:83 (Diff revision 2) > +set_define('GTEST_USE_OWN_TR1_TUPLE', > + delayed_getattr(linux_gtest_defines, 'use_own_tr1_tuple')) Note that now that android builds use libc++, this might not be needed anymore. ::: moz.configure (Diff revision 2) > if compile_env: > return 'build/moz.configure/memory.configure' > > include(memory_include) > > - Please don't remove this line
Attachment #8753570 -
Flags: review?(mh+mozilla) → review+
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d33cfe343732
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
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
•