Closed Bug 1333054 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

(firefox53 affected)

RESOLVED WORKSFORME
Tracking Status
firefox53 --- affected

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

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.
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.
Attachment #8829431 - Flags: review+
Attachment #8829431 - Flags: feedback?(cmanchester)
Keywords: leave-open
also merged down to integration/m-i and autoland
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.
Attachment #8829431 - Flags: feedback?(cmanchester)
Duplicate of this bug: 1332948
Priority: -- → P5
We made the check for llvm-config conditional in bug 1364428; I think we can close this bug.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.