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.

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

Dependency on LIBCLANG_PATH is not propagated back to cargo #1766

Closed
nagisa opened this issue Apr 24, 2020 · 7 comments · Fixed by #1809
Closed

Dependency on LIBCLANG_PATH is not propagated back to cargo #1766

nagisa opened this issue Apr 24, 2020 · 7 comments · Fixed by #1809

Comments

@nagisa
Copy link
Member

nagisa commented Apr 24, 2020

I have upgraded clang from 7.0 to 10.0 and set LIBCLANG_PATH (the path changed) to a new clang, however to my surprise cargo build did not rebuild any of the crates that depend on bindgen to generate bindings.

Bindgen should be telling cargo to rerun the build.rs for the crate if following are true:

  1. The used libclang shared library has changed;
  2. LIBCLANG_PATH has changed…

and whatever other things can influence loading of the libclang.

@kulp
Copy link
Member

kulp commented Jun 21, 2020

I do not know all of the things that "can influence loading of the libclang", and my proposed #1809 may or may not sufficiently cover all the cases, but I think it meets the main need here.

@emilio
Copy link
Contributor

emilio commented Jun 21, 2020

Well, #1809 only addresses bindgen's tests. Ideally we should do something a bit more automatic here... Maybe CargoCallbacks should print this at some point, or Bindgen's generate if we detect we're running under Cargo? (what is the right way to detect that? Checking for OUT_DIR?)

@kulp
Copy link
Member

kulp commented Jun 21, 2020

@emilio, perhaps I misunderstand, but my demonstration in #1809 was meant to show that changing LIBCLANG_PATH when building a downstream crate does cause bindings to be regenerated, so this does not affect only bindgen's tests, I think.

@emilio
Copy link
Contributor

emilio commented Jun 22, 2020

Ah, indeed. We invoke testgen::main unconditionally, so it ends up working... I'd say it makes sense for that bit to be outside of the testgen module though, as it's not related to test generation.

But yeah, this should be closed, behavior here after your PR is correct, thanks @kulp!

@emilio emilio closed this as completed Jun 22, 2020
@kulp
Copy link
Member

kulp commented Jun 22, 2020

Good point about testgen -- I had not looked very carefully at where I was adding the code, I admit (I just added it in the vicinity of some other cargo:rerun prints). I will look for a better place.

@kulp
Copy link
Member

kulp commented Jun 22, 2020

I see you are way ahead of me in #1811. Thanks !

@kulp
Copy link
Member

kulp commented Jun 22, 2020

I guess we should probably check BINDGEN_EXTRA_CLANG_ARGS too, from #1537, so I created #1813.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants