Closed Bug 1445528 Opened 6 years ago Closed 6 years ago

libloading build errors (in "cc-rs" dependency) if you build with "export CC=[anything]" in environment

Categories

(Core :: General, defect)

57 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr60 --- fixed
firefox61 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files)

I've been wrestling with my mozilla-central build for most of today, mostly without success, to try to address from a "libloading" crate upgrade in this commit: 
  https://hg.mozilla.org/mozilla-central/rev/faa6563df4af

The new version of libloading has a dependency on the "cc" (or "cc-rs") crate, and that "cc" crate doesn't handle quirks in the $CC environmental variable particularly robustly -- and our internal build system does seem to add some quirks to that variable, which break cc and cause a build error (preventing mozilla-central from building successfully, at least on my machine).

emilio helped me work through part of the issues in IRC and we isolated https://github.com/alexcrichton/cc-rs/issues/297 as part of the problem, but sadly that probably won't be sufficient to fix the problems (i.e. we'll still have versions of this issue after we revendor to pull in the cc-rs crate wit that fix).

Could we revert the libloading update for now, until we've got some confidence that we have functional workarounds and/or fixes for this set of issues?
Flags: needinfo?(emilio)
Attached file error output
Here's the error output. This is from a build with a trivial mozconfig:
> mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj
> ac_add_options --enable-debug --disable-optimize

...and with this in my .bash_aliases:
> CC=clang
> CXX=clang++

The key part of the build error output is here:
> 0:09.94 CC = Some(" /usr/bin/clang -std=gnu99")
> [...]
> 0:09.95 running: " /usr/bin/clang -std=gnu99" "-O1" [...]

I'm not sure, but it looks like we're literally running this as a shell command:
$ " /usr/bin/clang -std=gnu99"
...and with the quotes included, that does not work, because " /usr/bin/clang -std=gnu99" is not the name of an executable on my system. Really, we want to run /usr/bin/clang, and pass it -std=gnu99 as its first arg.

The leading space is part of the issue here, and it seems like https://github.com/alexcrichton/cc-rs/issues/297 will fix that once we've got that fix. (Though the source of that space is still mysterious.)

But the "-std=gnu99" suffix seems likely to be just as problematic... (I can't tell for sure, because I don't know how to try out the cc-rs crate update inside of my mozilla-central build.)  We add that gnu99 argument here for some arcane reason:
https://dxr.mozilla.org/mozilla-central/rev/6ff60a083701d08c52702daf50f28e8f46ae3a1c/build/moz.configure/toolchain.configure#483-489
...and it's always worked (and may still be necessary to avoid trouble), but it's now likely causing trouble with this 'cc' tool, so we need to come up with some sort of workaround or fix.
(In reply to Daniel Holbert [:dholbert] from comment #1)
> ...and with this in my .bash_aliases:
> > CC=clang
> > CXX=clang++

(Er, I meant "export CC" and "export CXX" there)

OK, I've isolated what's triggering the issue in my environment in particular, and I think I have a workaround.

I *only* hit this problem if I have a CC variable exported (set to either gcc or clang, doesn't matter) in my environment *outside* of my mozconfig. E.g. set in my .bash_aliases, or manually on the terminal before I kick off my build.


If I make sure I don't have CC set in my environment before I fire off my build, then everything is fine.  I can export CC/CXX to gcc or clang from within my mozconfig and I'm fine -- it only causes trouble here if I do it from outside my mozconfig.
(In reply to Daniel Holbert [:dholbert] from comment #0)
> Could we revert the libloading update for now, until we've got some
> confidence that we have functional workarounds and/or fixes for this set of
> issues?

Hmm, given that we've got a workaround (exporting CC/CXX exclusively in mozconfig), maybe no need to revert quite yet.
Flags: needinfo?(emilio)
Summary: libloading build errors with any sort of non-trivial "CC" variable in → libloading build errors (in "cc-rs" dependency) if you build with "export CC=[anything]" in environment
As an extra data point -- mccr8 reproduced this bug as well, just by running...
 export CC=gcc
...before running ./mach build.
See Also: → 1446660
Running into this on the m-c builds for DXR, though I don't *think* it's exporting CC/CCX.
Looks like all of the involved cc-rs issues (that we've identified so far) have been fixed upstream!

Once there's been a version bump (to 1.0.9 I'm guessing, after today's fix for https://github.com/alexcrichton/cc-rs/issues/300 ), perhaps we could upgrade our copy of this crate to pull in the fixes and hopefully fix this bug?

I don't know what's involved in that process, but I'm guessing :ato does, since he's the only human with hg blame on the directory where our copy of this library lives: https://hg.mozilla.org/mozilla-central/annotate/tip/third_party/rust/cc/
Flags: needinfo?(ato)
The upstream cc-rs 1.0.9 version bump has now happened, btw:
https://github.com/alexcrichton/cc-rs/commit/7fef736f9c9e38acf96de22c1e9379c70a082cf3
Try doing the following:

  $ cargo update -p cc
  $ ./mach vendor rust
Thanks! That does indeed seem to produce changes in my source directory that get me version 1.0.9 of this cc library, and it does seem to save me from this bug.

I guess I might as well just post that patch, to get things moving.  Looks like jgraham has reviewed all changes to the third-party/rust/cc folder, so I'll tag him to review this.
Flags: needinfo?(ato)
Comment on attachment 8961529 [details]
Bug 1445528: Update 'cc' crate to version 1.0.9.

https://reviewboard.mozilla.org/r/230322/#review236528
Attachment #8961529 - Flags: review?(james) → review+
Sorry I didn’t see this earlier, but glad you sorted it out!
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3be2aac2ede6
Update 'cc' crate to version 1.0.9. r=jgraham
https://hg.mozilla.org/mozilla-central/rev/3be2aac2ede6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(In reply to Kendall Libby [:fubar] from comment #5)
> Running into this on the m-c builds for DXR, though I don't *think* it's
> exporting CC/CCX.

Good news -- looks like DXR is working OK now (hooray!).  At least: the Cargo.lock file in DXR does show reflect this bug's patch having landed (it's got library "cc" at version 1.0.9):
https://dxr.mozilla.org/mozilla-central/rev/de32269720d056972b85f4eec5f0a8286de6e3af/Cargo.lock#202
Assignee: nobody → dholbert
Could I please request an uplift of this fix of cc to ESR 60.3?  We are running into this at Suse, in the context of bug #1472538.
Comment on attachment 8961529 [details]
Bug 1445528: Update 'cc' crate to version 1.0.9.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: ESR build fix for some build configurations.

User impact if declined: Build-only code, so none.

Fix Landed on Version: 61

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Build-only.

String or UUID changes made by this patch: none
Attachment #8961529 - Flags: approval-mozilla-esr60?
Comment on attachment 8961529 [details]
Bug 1445528: Update 'cc' crate to version 1.0.9.

NPOTB patch to help downstream builds, approved for ESR 60.3.
Attachment #8961529 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.