1333054 - don't complain about llvm-config if we're not building stylo bindgen

Status: RESOLVED WORKSFORME

(Firefox Build System :: General, defect, P5)

Description

[Nathan Froyd [:froydnj]]

If we're not building stylo, then we can find a too-old llvm-config on PATH, examine its version, and declare it as too old for build-time bindgen.  But this is wrong: we're not building stylo at all, so we shouldn't care about the llvm-config version.

I think I'm going to land a semi-hacky patch on m-c to unblock people who run into this today, and then we can discuss the Right Fix.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: band-aid - avoid checking llvm-config version if we're not building stylo/bindgen

This is the band-aid fix.  I dislike the duplication of checking for
--enable-stylo and bindgen in |stylo| and |bindgen_config_paths|, but I'm not
sure how to delay the evaluation of |bindgen_config_paths| until we know we're
going to need it in |stylo|.

The alternative fix is to not |die| in |bindgen_config_paths|, but to pass
error information up in its returned namespace and sort out the errors in
|stylo| itself, but that feels hackish too.

f? to Chris as to whether he has better suggestions, but it's going to be some
post-landing feedback. :)  We can address suggestions in a follow-up.

Comment 2

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f80dc9fc34680105b714a49b4704bb843f5f7004

Comment 3

[Carsten Book [:Tomcat]]

also merged down to integration/m-i and autoland

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8829431 [details] [diff] [review]
band-aid - avoid checking llvm-config version if we're not building stylo/bindgen

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

This is fine. I guess it'd be a bit better to have the version passed up to `stylo` in the namespace returned from `bindgen_config_paths`, and decide whether to do the check and die if it fails there.

Comment 6

[Nathan Froyd [:froydnj]]

We made the check for llvm-config conditional in bug 1364428; I think we can close this bug.

Comment 7

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.