Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rust-bindgen: wrap with cc-wrapper to provide location of headers #40746

Closed
wants to merge 1 commit into from
Closed

rust-bindgen: wrap with cc-wrapper to provide location of headers #40746

wants to merge 1 commit into from

Conversation

symphorien
Copy link
Member

Motivation for this change

bindgen uses the raw libclang and therefore cannot find any header. This makes it nearly unusable.

This commit wraps bindgen to provide the correct include flags. Reimplementing cc-wrapper would be
hard and error prone, so I tried to reuse cc-wrapper. The result is ugly but I think it will be more robust
against changes in cc-wrapper.

The idea is to make bindgen a fake clang and wrap it with cc-wrapper. Since bindgen has its own
set of flags, we wrap it to remove those extra flags, pass the pure clang flags to cc-wrapper, and
then readd the bindgen-specific flags.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@symphorien
rust-bindgen: wrap with cc-wrapper to provide location of headers
8b5451f 
See wrapper.nix for implementation details.
Note that to parse c++ headers, you must pass `-x c++` as a clang
argument.
@symphorien
Copy link
Member Author

cc @Ralith as maintainer of rust-bindgen

@Ralith
Copy link
Contributor

Ralith commented May 19, 2018

Why not just insert the expansion of $NIX_CFLAGS_COMPILE $NIX_CXXSTDLIB_COMPILE following the first occurrence of -- in the arguments list, or append -- $NIX_CFLAGS_COMPILE $NIX_CXXSTDLIB_COMPILE if there is no occurrence of --? That seems far simpler and less fragile, even if it does encode a bit of cc-wrapper knowledge.

@symphorien
Copy link
Member Author

This would lack glibc, but still.
Honestly, by reading cc-wrapper.sh I thought these variables only existed with an unpredictable @infix_salt@.
I am glad there is an easy solution.

@symphorien symphorien closed this May 19, 2018
@Ralith
Copy link
Contributor

Ralith commented May 19, 2018

I honestly have no idea what's up with the infix salt stuff. Something to do with crosscompiling, which we probably want bindgen to support, at least in theory.

@symphorien
Copy link
Member Author

symphorien commented May 19, 2018 •
edited

Is cross compilation a goal ? and if so, does any of the two solutions work in this context ?

@Ralith
Copy link
Contributor

Ralith commented May 20, 2018

I have no familiarity with the nix cross infrastructure, though it seems like it's been getting a lot of work lately. Maybe have a look at the commit logs for cc-wrapper to find someone to ask?

I think it would be great if rust-bindgen Just Worked when the target machine differs from the host, but support for the common (non-cross) case would be a good start. I have no idea if the rust infrastructure (old or new) handles crosscompilation at all yet; @P-E-Meunier might know more about that.

@P-E-Meunier
Copy link
Contributor

I have never tested cross-compilation. For bindgen, I think it would be cool to add a new subcommand to carnix, that would also output the libraries needed (in default-crate-overrides.nix) to compile the crate.

@Mic92
Copy link
Member

Mic92 commented May 20, 2018 •
edited

I only used rustup from nixpkgs for cross compilation.

@symphorien symphorien mentioned this pull request Aug 19, 2018
Merged
7 tasks
@symphorien symphorien deleted the bindgen-wrapper branch May 18, 2019 16:01
@symphorien symphorien mentioned this pull request May 30, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@symphorien @Ralith @P-E-Meunier @Mic92 @GrahamcOfBorg