Closed
Bug 1366564
Opened 7 years ago
Closed 7 years ago
rust-bindgen can't find Mac headers that aren't in /usr/include: fatal error: 'stdlib.h' file not found
Categories
(Firefox Build System :: General, defect, P2)
Firefox Build System
General
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: cpeterson, Assigned: gps)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Stylo])
Attachments
(1 file, 1 obsolete file)
I tried to build Stylo last night on Mac, but rust-bindgen failed because the clang std headers installed by mach bootstrap can't #include_next the real std headers. I hit build errors like the following when trying to compile the generated bindings code: /Users/chris/.mozbuild/clang/include/c++/v1/stdlib.h:94:15: fatal error: 'stdlib.h' file not found jryans suggested that I run `xcode-select --install`, which fixed the problem. Bobby says xcode-select installs the stdlib headers in /usr/include, which makes them findable by bindgen without complicated autoconf stuff. @ Nick: can rust-bindgen somehow find Mac's headers if they are not in /usr/include ? Or should rust-bindgen or Firefox's configure just detect the missing /usr/include headers and log an error message that tells people to run xcode-select --install?
Flags: needinfo?(nfitzgerald)
Reporter | ||
Updated•7 years ago
|
Blocks: stylo-tooling
Comment 1•7 years ago
|
||
Bindgen doesn't do any fancy include directory finding itself, it completely relies on libclang for that. Users supply the include directories via one of * When using a builder: builder.clang_arg("-I/usr/include") * When using the CLI tool: bindgen <bindgen options> -- <clang options> -I/usr/include Since bindgen doesn't deal with detecting include directories at all right now, I don't think it makes a ton of sense for it to take on this new responsibility. I think it makes most sense for Firefox's build system to do this kind of detection.
Flags: needinfo?(nfitzgerald)
Reporter | ||
Comment 2•7 years ago
|
||
That make sense. Seems like mach bootstrap should just run `xcode-select --install` automatically.
Comment 3•7 years ago
|
||
> $ xcode-select --install
> xcode-select: error: command line tools are already installed, use "Software Update" to install updates
Looks like we'd need to parse that error message, which seems brittle. :/
(In reply to Ralph Giles (:rillian) | needinfo me from comment #3) > > $ xcode-select --install > > xcode-select: error: command line tools are already installed, use "Software Update" to install updates > > Looks like we'd need to parse that error message, which seems brittle. :/ We could potentially check if an example file exists (like /usr/include/stdlib.h from this bug's title), and only run `xcode-select --install` if it's missing? Not sure if that's accurate enough.
Comment 5•7 years ago
|
||
Wouldn't `xcode-select --switch` be more appropriate? (although it requires knowning where the xcode install is) Note `xcode-select --print-path` could be helpful too, and so would `xcrun --show-sdk-path` and other flags of xcrun.
![]() |
||
Comment 7•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #5) > Note `xcode-select --print-path` could be helpful too, and so would `xcrun > --show-sdk-path` and other flags of xcrun. Could we just have bindgen invoke `xcrun --show-sdk-path`, and then concatenate whatever that returns with "/usr/include", so we don't have to rely on the user running xcode-select or similar? Would that be acceptable?
Flags: needinfo?(nfitzgerald)
Comment 8•7 years ago
|
||
We could. But, it still feels more "better" to me to have bindgen invocations do this -- what would be the advantage of bindgen doing this instead of making stylo's bindgen invocation do it? Emilio, opinions?
Flags: needinfo?(nfitzgerald) → needinfo?(emilio+bugs)
![]() |
||
Comment 9•7 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] [β°PST; UTC-8] from comment #8) > We could. But, it still feels more "better" to me to have bindgen > invocations do this -- what would be the advantage of bindgen doing this > instead of making stylo's bindgen invocation do it? > > Emilio, opinions? I meant in stylo's bindgen invocations, but fixing this once-for-all in bindgen itself would work, too!
Comment 10•7 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] [β°PST; UTC-8] from comment #8) > We could. But, it still feels more "better" to me to have bindgen > invocations do this -- what would be the advantage of bindgen doing this > instead of making stylo's bindgen invocation do it? > > Emilio, opinions? We run already the clang binary, so doing this sounds fine. That should probably live in https://github.com/KyleMayes/clang-sys, though.
Flags: needinfo?(emilio+bugs)
Reporter | ||
Updated•7 years ago
|
Comment 11•7 years ago
|
||
Is this still blocking 1375774? Now that we're cross-compiling, this likely isn't an issue on build boxes. If we really want to fix this, see https://stackoverflow.com/a/15371967/8388 for ideas on how to check if `xcode-select --install` is required on different OSX versions.
Reporter | ||
Comment 12•7 years ago
|
||
Mac cross-compiled builds are still blocked by bug 1368083, which is another header file path issue similar to this bug. Nathan thinks Ralph's fix for bug 1368083 will fix (or can be used to fix) this bug.
Reporter | ||
Comment 13•7 years ago
|
||
Assigning to Ralph for now because his fix for bug 1368083 may also fix this build issue.
Assignee: nobody → giles
Comment 14•7 years ago
|
||
I don't believe the fix for bug 1368083 addresses this. It provides a work around in that mach will forward `-isysroot` settings to bindgen, but as I understand it the problem is that clang from the default xcode install will look in the SDK sysroot by default, while bindgen looks in /usr/include by default. Most developers will run the equivalent of `xcode-select --install` at some point, but we should verify that if bindgen requires it.
Reporter | ||
Comment 15•7 years ago
|
||
This issue doesn't need to block building Mac in automation or local builds (bug 1375774) since there is a workaround.
No longer blocks: 1375774
Updated•7 years ago
|
Priority: P3 → --
Updated•7 years ago
|
Priority: -- → P4
Comment 17•7 years ago
|
||
In case someone else finds this and finds that their build doesn't work, even with comment 5. I guess I had an old clang build from brew and it was still installed in /usr/local/Cellar/llvm/HEAD. Stylo seems to expect something in /usr/local/Cellar/llvm/version since that's where it is with xcode. I had to manually uninstall llvm (brew uninstall llvm), remove all locally built versions of clang / llvm, and reinstall llvm. Restarted my terminal just to clear all env flags and it worked.
Updated•7 years ago
|
Comment 18•7 years ago
|
||
FWIW, I experienced the problem with 10.13. running xcode-select --install didn't help at first. However, shortly after a new upgrade appeared in the App Store for a new version of the command line tools (actually too, one for xcode on 10.11), and after installing those it worked.
Assignee | ||
Comment 20•7 years ago
|
||
Raising priority to reflect that people are hitting this in the wild and that the workaround is non-obvious. As I wrote in bug 1403001, configure should ideally detect that we're in a bad state and recommend concrete steps to fix.
Updated•7 years ago
|
No longer blocks: highsierra
Assignee | ||
Comment 21•7 years ago
|
||
We're hashing things out in #build. I'll start on a patch.
Assignee: giles → gps
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8912289 [details] Bug 1366564 - Validate Xcode installation state in configure; https://reviewboard.mozilla.org/r/183642/#review188818 LGTM with comments addressed. ::: build/moz.configure/toolchain.configure:104 (Diff revision 1) > +option('--disable-xcode-checks', > + help='Do not check that Xcode is installed and properly configured') > + > +@depends(host) > +def host_is_mac(host): > + return host.kernel == 'Darwin' You don't use this, and only check the host in on place, so might as well remove it. ::: build/moz.configure/toolchain.configure:113 (Diff revision 1) > + if host.kernel != 'Darwin' or not xcode_checks: > + return > + > + def bad_xcode_select(): > + die('Could not find installed Xcode; run Xcode.app to ensure ' > + 'initial Xcode configuration is performed') I would be more verbose here. We know having the latest Xcode can be important, and I don't know how many people will map 'run Xcode.app' to "Look for Xcode in Applications and install it from the app store if it isn't there, then launch it." ::: build/moz.configure/toolchain.configure:134 (Diff revision 1) > + 'com.apple.pkg.CLTools_Executables', > + onerror=no_cltools) > + > + return res > + > +set_config('XCODE_CLTOOLS_PATH', xcode_command_line_tools) `pkgutil` returns several lines on success, none of which are a path (unless `location: /` counts). Maybe just return the output of `xcode-select -p` instead? It would be nice if there were a way to get this into the dependency graph without cluttering up the build config with information we're not using.
Attachment #8912289 -
Flags: review?(giles) → review+
Comment 27•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8912289 [details] Bug 1366564 - Validate Xcode installation state in configure; https://reviewboard.mozilla.org/r/183642/#review188818 > I would be more verbose here. We know having the latest Xcode can be important, and I don't know how many people will map 'run Xcode.app' to "Look for Xcode in Applications and install it from the app store if it isn't there, then launch it." I believe `xcode-select --install` will prompt to install xcode itself if it isn't present so maybe something like "Please install Xcode from the App Store and launch it to complete initial configuration, or run 'xcode-select --install' from the command line."
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Summary: stylo: rust-bindgen can't find Mac headers that aren't in /usr/include: fatal error: 'stdlib.h' file not found → rust-bindgen can't find Mac headers that aren't in /usr/include: fatal error: 'stdlib.h' file not found
Whiteboard: [Stylo]
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8912289 [details] Bug 1366564 - Validate Xcode installation state in configure; https://reviewboard.mozilla.org/r/183642/#review188924
Comment 30•7 years ago
|
||
Pushed by rgiles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e2ecf684f49e Validate Xcode installation state in configure; r=rillian
Comment 31•7 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] [β°PST; UTC-8] from comment #1) > Bindgen doesn't do any fancy include directory finding itself, it completely > relies on libclang for that. Users supply the include directories via one of > > * When using a builder: > > builder.clang_arg("-I/usr/include") > > * When using the CLI tool: > > bindgen <bindgen options> -- <clang options> -I/usr/include > > Since bindgen doesn't deal with detecting include directories at all right > now, I don't think it makes a ton of sense for it to take on this new > responsibility. I think it makes most sense for Firefox's build system to do > this kind of detection. ... except nobody passes /usr/include to clang on its command line. And if the build reaches rust code, it means at least some C/C++ code has been built already, so clang is also happy that /usr/include doesn't exist. So whatever clang is doing to find the system headers, it seems like bindgen should do it too.
Comment 32•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #31) > ... except nobody passes /usr/include to clang on its command line. And if > the build reaches rust code, it means at least some C/C++ code has been > built already, so clang is also happy that /usr/include doesn't exist. So > whatever clang is doing to find the system headers, it seems like bindgen > should do it too. So we explicitly ask bindgen not to add search path from clang on macOS. Specifically, bindgen skips that when --target is specified [1], and we specify --target for macOS build target [2] because of conflict with -stdlib=libc++ (which was added in servo/servo#15414). I forgot the full rationale why we need to pass -stdlib=libc++ and how that would conflict with search path provided by clang. IIRC, there are some other environment variables / parameters involve for the target macOS version, and clang produces different search paths for different target versions. That was probably a workaround for that. If we can pass in the correct target macOS version, probably we can allow bindgen to add clang search path, so that this would no longer be a problem. [1] https://github.com/rust-lang-nursery/rust-bindgen/blob/e78394959f40193a4bf9e5718dd4f779cb57437a/src/lib.rs#L1508-L1528 [2] https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/style/ServoBindings.toml#15-18
Updated•7 years ago
|
Attachment #8912272 -
Flags: review?(gps)
Updated•7 years ago
|
Attachment #8912272 -
Attachment is obsolete: true
![]() |
||
Comment 33•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e2ecf684f49e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 34•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #31) > ... except nobody passes /usr/include to clang on its command line. And if > the build reaches rust code, it means at least some C/C++ code has been > built already, so clang is also happy that /usr/include doesn't exist. So > whatever clang is doing to find the system headers, it seems like bindgen > should do it too. When I looked at it, the issue was that Apple clang, the system compiler, is configured to use the headers in the default Xcode SDK by default, so it works fine if there's no /usr/include. That search path configuration is a feature of the clang/clang++ front end, and not libclang. So we would have to manually invoke clang to get the default search path, then add that to the configuration bindgen passes to libclang. However, because of minimum-supported-version issues, bindgen doesn't in general use the system libclang. Only the build system knows what C/C++ compiler is actually being used, so to me it seems less confusing to have the build system parse out the include paths and add them to the non-system include directives passed to bindgen rather than passing an extra compiler bindgen should call to do the same thing.
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•