Closed Bug 1476427 Opened 2 years ago Closed 2 years ago

move bindgen discovery bits to a place where the JS engine can access them

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(3 files)

Cretonne integration depends on bindgen to generate a bunch of stuff, so it needs all the relevant bits (MOZ_CLANG_PATH, etc.) we discover in toolkit/moz.configure.  So things work in a JS shell, we need bindgen things during JS-only builds as well.  So we'd need to move all of those bindgen bits out of toolkit/moz.configure to a better place (.../bindgen.configure?) that's accessible from the JS engine.

Let's cross the bridge of whether we could somehow make a single bindgen invocation take care of everything at some later point and deal with multiple bindgen invocations for now.
This change makes the config variables determined for
bindgen (MOZ_CLANG_PATH et al) available to JS.

The next path ensures that we don't fail shell builds if we can't find the
appropriate variables.
Attachment #8993372 - Flags: review?(core-build-config-reviews)
JS, at least, doesn't need bindgen at this point.  If JS does start
requiring bindgen under certain build configurations, we'll have to
figure out what to do about the bindgen dependency at that point:
require bindgen always, or just require bindgen for the
configuration(s).
Attachment #8993373 - Flags: review?(core-build-config-reviews)
Note that bug 1444141 is making the Rust dependency for the JS engine unconditional...I suppose we could make the bindgen dependency unconditional, too?
Attachment #8993372 - Flags: review?(core-build-config-reviews) → review+
Comment on attachment 8993373 [details] [diff] [review]
part 2 - whitelist projects that require bindgen

Review of attachment 8993373 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/moz.configure/bindgen.configure
@@ +174,5 @@
>  
>      if (not libclang_path and clang_path) or \
>         (libclang_path and not clang_path):
> +        if build_project not in bindgen_projects:
> +            return namespace()

The empty namespace is a little hacky, but I can see how it makes this patch simpler.
Attachment #8993373 - Flags: review?(core-build-config-reviews) → review+
Things the JS engine wants to understand should be js_options.
Attachment #8995268 - Flags: review?(cmanchester)
Attachment #8995268 - Flags: review?(cmanchester) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ede083803917
part 1 - move bindgen configure bits to a separate file; r=chmanchester
https://hg.mozilla.org/integration/mozilla-inbound/rev/f89b91c892b9
part 2 - whitelist projects that require bindgen; r=chmanchester
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8ea1d673cac
part 3 - convert bindgen options into js_options; r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/ede083803917
https://hg.mozilla.org/mozilla-central/rev/f89b91c892b9
https://hg.mozilla.org/mozilla-central/rev/c8ea1d673cac
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.