Closed Bug 1294803 Opened 8 years ago Closed 8 years ago

Move ctypes and libffi related variables to moz.configure

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox51 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox51 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system.
58 bytes, text/x-review-board-request
glandium
: review+
Details
Bug 1294803 - Move BUILD_CTYPES to Python configure.
58 bytes, text/x-review-board-request
glandium
: review+
Details
Namely, BUILD_CTYPES and MOZ_SYSTEM_FFI.
Comment on attachment 8781247 [details] Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. https://reviewboard.mozilla.org/r/71680/#review69266 ::: build/moz.configure/ffi.configure:12 (Diff revision 1) > +@depends_when(c_compiler, when=use_system_ffi) > +def system_ffi_spec(info): > + # Vanilla libffi 3.0.9 needs a few patches from upcoming version 3.0.10 > + # for non-GCC compilers. > + if info.type == 'gcc': > + return 'libffi >= 3.0.9' > + return 'libffi > 3.0.9' Part of the solution for you, though, might be to say that we don't need to care about gcc vs. non-gcc anymore. 3.0.10 was released 5 years ago. ::: build/moz.configure/ffi.configure:20 (Diff revision 1) > + # for non-GCC compilers. > + if info.type == 'gcc': > + return 'libffi >= 3.0.9' > + return 'libffi > 3.0.9' > + > +system_ffi = pkg_check_modules('MOZ_FFI', system_ffi_spec, use_system_ffi) random thought: this would be clearer with "condition=use_system_ffi", which makes me wonder why it's "condition" and not "when", like for other things. ::: moz.configure:104 (Diff revision 1) > +include_when('build/moz.configure/ffi.configure', > + when='--enable-compile-environment') This should only be included from js/moz.configure. OTOH --enable-compile-environment and toolchain tests are only available after js/moz.configure is included :-/ I don't have a solution here, but that's something that is hitting me for other things as well, so this is something we need to find a solution for.
Attachment #8781247 - Flags: review?(mh+mozilla)
Comment on attachment 8781248 [details] Bug 1294803 - Move BUILD_CTYPES to Python configure. https://reviewboard.mozilla.org/r/71682/#review69268
Attachment #8781248 - Flags: review?(mh+mozilla)
Comment on attachment 8781248 [details] Bug 1294803 - Move BUILD_CTYPES to Python configure. https://reviewboard.mozilla.org/r/71682/#review69270
Attachment #8781248 - Flags: review+
Comment on attachment 8781247 [details] Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. https://reviewboard.mozilla.org/r/71680/#review69266 > random thought: this would be clearer with "condition=use_system_ffi", which makes me wonder why it's "condition" and not "when", like for other things. `pkg_check_modules` pre-dates `when`, the inconsistency should just be fixed. > This should only be included from js/moz.configure. OTOH --enable-compile-environment and toolchain tests are only available after js/moz.configure is included :-/ > I don't have a solution here, but that's something that is hitting me for other things as well, so this is something we need to find a solution for. Doing these checks in the top-level configure is status-quo (for now). Agree about the issue in general.
Comment on attachment 8781247 [details] Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. https://reviewboard.mozilla.org/r/71680/#review69712 Please file a followup to move the ffi.configure include to js/. Please also add a comment in the commit message wrt always requiring ffi > 3.0.9 because 3.0.10 is already 5 years old.
Attachment #8781247 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c0e6aae0b93d Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/7f6047b2bc09 Move BUILD_CTYPES to Python configure. r=glandium
https://hg.mozilla.org/mozilla-central/rev/c0e6aae0b93d https://hg.mozilla.org/mozilla-central/rev/7f6047b2bc09
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
This breaks artifact builds, because we no longer set BUILD_CTYPES when --disable-compile-environment, and we use that to check whether to recurse into any ctypes related directories. The resulting build fails to find ctypes.jsm during startup. I'll back this out.
Grr...
Status: RESOLVED → REOPENED
status-firefox51: fixed → affected
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/ddc877cac511
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
status-firefox51: fixed → affected
Resolution: FIXED → ---
Attachment #8781247 - Flags: review+ → review?(mh+mozilla)
Attachment #8781248 - Flags: review+ → review?(mh+mozilla)
These patches address comment 12 by putting BUILD_CTYPES in the root moz.configure.
Comment on attachment 8781248 [details] Bug 1294803 - Move BUILD_CTYPES to Python configure. https://reviewboard.mozilla.org/r/71682/#review71308 ::: moz.configure:106 (Diff revision 3) > when='--enable-compile-environment') > include_when('build/moz.configure/warnings.configure', > when='--enable-compile-environment') > > + > +@depends(building_js, '--help') Might as well put it in js/moz.configure.
Attachment #8781248 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8781247 [details] Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. https://reviewboard.mozilla.org/r/71680/#review71312 ::: moz.configure:128 (Diff revision 3) > > set_config('JS_HAS_CTYPES', js_has_ctypes) > set_define('JS_HAS_CTYPES', js_has_ctypes) > add_old_configure_assignment('JS_HAS_CTYPES', js_has_ctypes) > > +@depends('--enable-ctypes', '--enable-compile-environment', '--help') So, here we have a reason why it wouldn't work in js/moz.configure... until bug 1296530. If you don't want to wait for that bug, please file a followup to move --enable-ctypes to js/moz.configure.
Attachment #8781247 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8781247 [details] Bug 1294803 - Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. https://reviewboard.mozilla.org/r/71680/#review71312 > So, here we have a reason why it wouldn't work in js/moz.configure... until bug 1296530. If you don't want to wait for that bug, please file a followup to move --enable-ctypes to js/moz.configure. Filed bug 1297471
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2938c8a38f2b Move BUILD_CTYPES to Python configure. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/474dce563e74 Move MOZ_SYSTEM_FFI to moz.configure in preparation for moving libffi to our build system. r=glandium
https://hg.mozilla.org/mozilla-central/rev/2938c8a38f2b https://hg.mozilla.org/mozilla-central/rev/474dce563e74
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Depends on: 1298740
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: