rust-cross / cargo-zigbuild

Compile Cargo project with zig as linker
MIT License
1.45k stars 52 forks source link

Robust bindgen support #140

Closed lifthrasiir closed 1 year ago

lifthrasiir commented 1 year ago

It turned out that #74, which added bindgen support, was pretty much incomplete. For example the following C code will fail to compile during bindgen.

#include <stdlib.h>

It should be noted that Zig contains multiple libc header files:

  1. zig/lib/libc/glibc/include etc. are used to build libc on demand. They are not meant to be directly included, and in fact contain a small subset of libc headers which are used by other libc subsystems. Consequently files in this directory will most likely fail to compile; this includes stdlib.h, errno.h, pthread.h and signal.h.
  2. zig/lib/include contains header files shared by all targets. They are mostly platform-specific intrinsics.
  3. zig/lib/libc/include/$TARGET contains header files specific to given target. For given target there may be multiple such directories, and some directories are referenced from Zig but not present and ignored by clang driver.
  4. Finally, zig/lib/libcxx/include and zig/lib/libcxxabi/include are included before any other headers because libc++ itself contains some C headers that are not necessarily in libc (e.g. uchar.h).

Right now cargo-zigbuild only concerns with 1, so while some headers won't compile at all, other headers will inadvertently refer to the system library. This PR makes cargo-zigbuild to include 2 and 3, and if required, 4 as well. There are now explicit tests to include (almost) all standard C and C++ header files.

As noted from a lengthy comment that I don't really repeat here, the current strategy of overriding BINDGEN_EXTRA_CLANG_ARGS is not really ideal. I have considered alternative approaches, and unfortunately none of them would work out of the box. In the long term we may get clang-sys fixed and then put target-specific zig cc wrappers to PATH so that clang-sys can infer a correct search path by itself, but it will take a long time for every one to use a fixed clang-sys. So I've resorted to this hack for now.

lifthrasiir commented 1 year ago

Whoops... I realized that this approach alone won't work in general because bindgen will still look at older clang anyway (which we can't currently change). So if you have a recent enough clang (I believe, 13 or later), this would indeed work, but this PR is still not complete yet.

messense commented 1 year ago

It'd be great if we can merge a solution even if it only works for newer clang versions.

TheButlah commented 1 year ago

log.txt I tried this while compiling to --target aarch64-unknown-linux-gnu and couldn't get it to work. I can't disclose my -sys crate but it was pretty simple, and I've printed the full expansion of the bindgen builder and my environment variables.

lifthrasiir commented 1 year ago

log.txt I tried this while compiling to --target aarch64-unknown-linux-gnu and couldn't get it to work. I can't disclose my -sys crate but it was pretty simple, and I've printed the full expansion of the bindgen builder and my environment variables.

Huh, this looks pretty bad indeed. So to summarize what had happened here, we are trying to build aarch64-unknown-linux-gnu from an aarch64-apple-darwin host, but my version of zigbuild completely failed to detect include directories from zig and only the Xcode include directories are used.

@TheButlah, can you post what the following command prints? It should say something like #include <...> search starts here: and a bunch of include directories, which should have been picked up.

$ /Users/ryan.butler/Library/Caches/cargo-zigbuild/0.16.10/zigcc-aarch64-unknown-linux-gnu.sh -E -x c /dev/null -v
TheButlah commented 1 year ago

Sure, here you go. I messed with some stuff (specifically installing and uninstalling cross-compilation toolchains, llvm, adding llvm to path, etc) since my original message (I think? I don't remember) and wanted both logs to be in sync, so I just reran the cargo zigbuild for the below logs.

command you asked for corresponding cargo zigbuild --target aarch64-unknown-linux-gnu

All wrapper.h does currently is #include <stdio.h>

TheButlah commented 1 year ago
❯ brew list llvm --versions
llvm 16.0.5

❯ llvm-config --libdir --includedir --prefix
/opt/homebrew/Cellar/llvm/16.0.5/lib
/opt/homebrew/Cellar/llvm/16.0.5/include
/opt/homebrew/Cellar/llvm/16.0.5

❯ echo $PATH
/opt/homebrew/opt/mongodb-community@4.4/bin:/opt/homebrew/opt/llvm/bin:/Users/ryan.butler/.local/bin:/Users/ryan.butler/Library/Caches/fnm_multishells/66198_1686694763407/bin:/Users/ryan.butler/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/Users/ryan.butler/.local/bin:/Users/ryan.butler/Library/Caches/fnm_multishells/86670_1685625262144/bin:/Users/ryan.butler/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/Library/Frameworks/Python.framework/Versions/3.10/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Users/ryan.butler/.cargo/bin

❯ echo $CPPFLAGS

❯ echo $LDFLAGS

❯ clang --version
Homebrew clang version 16.0.5
Target: arm64-apple-darwin22.4.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin

❯ xcode-select -p
/Library/Developer/CommandLineTools

❯ zig cc --version
Homebrew clang version 15.0.7
Target: arm64-apple-darwin22.4.0
Thread model: posix
InstalledDir: /usr/bin

❯ zig env
{
 "zig_exe": "/opt/homebrew/Cellar/zig/0.10.1/bin/zig",
 "lib_dir": "/opt/homebrew/Cellar/zig/0.10.1/lib/zig",
 "std_dir": "/opt/homebrew/Cellar/zig/0.10.1/lib/zig/std",
 "global_cache_dir": "/Users/ryan.butler/.cache/zig",
 "version": "0.10.1",
 "target": "aarch64-macos.13.3.1...13.3.1-none"
}

❯ /usr/bin/clang --version
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: arm64-apple-darwin22.4.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
lifthrasiir commented 1 year ago

command you asked for

Thank you. Unfortunately all lines have been truncated past 83 columns so I can't easily see the exact commands zig has composed, but it seems clear that zigcc-aarch64-unknown-linux-gnu doesn't work as we want, especially given that it has no glibc include directories at all. And it turned out that -x c is only properly recognized in the development version of Zig! It was only added this year and I didn't realize that. For now, you can reinstall Zig with the development version (brew install zig --HEAD I believe) and try again.

TheButlah commented 1 year ago

weird, sorry. Here is the un-truncated output on the not-HEAD version of zig. I'll post the HEAD version shortly. command.txt

Edit: brew install zig --HEAD doesn't build for me, so I can't run that.

lifthrasiir commented 1 year ago

I'm now modestly confident about this PR, having passed the CI. Newer commits should work from all supported clang versions. I've also manually checked against 0.9.0 from my local machine (WSL Ubuntu 18.04 LTS, clang 10.0, glibc 2.27), additional verifications would be appreciated though.

@TheButlah I think this PR should work now without any changes---I've now worked around the -x issue.

TheButlah commented 1 year ago

Seems to work! Thanks :3