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

Pass include_dirs to cargo using env #343

Closed
wants to merge 1 commit into from

Conversation

jameshilliard
Copy link
Contributor

@jameshilliard jameshilliard commented Jun 25, 2023

Sometimes the cargo build script needs to know the include_dirs, lets pass this in the SETUPTOOLS_RUST_INCLUDE_DIRS env variable.

See #9132 for example usage.

Sometimes the cargo build script needs to know the include_dirs,
lets pass this in the SETUPTOOLS_RUST_INCLUDE_DIRS env variable.
@davidhewitt
Copy link
Member

davidhewitt commented Jun 25, 2023

How do these get passed to C / C++ compilations? I wonder if we can expose them in the same way and then let downstream projects handle them in the same way as if they were a C/C++ compilation. This might lead to best automatic support.

(e.g. should we maybe set CFLAGS?)

@jameshilliard
Copy link
Contributor Author

How do these get passed to C / C++ compilations?

I think they are just passed to the compiler command line.

I wonder if we can expose them in the same way and then let downstream projects handle them in the same way as if they were a C/C++ compilation.

I mean, that's more or less what I'm trying to do here. The cargo build script in my cryptography pull request passes the include_dirs from the env variable to the compiler using -I command line include arguments(by setting them using build.include(include_path)).

(e.g. should we maybe set CFLAGS?)

This seems like it could potentially cause conflicts if CFLAGS are being set in the env already.

@alex
Copy link
Contributor

alex commented Jun 25, 2023

FWIW, we ended up solving this with pyca/cryptography#9129

@jameshilliard
Copy link
Contributor Author

I'll close this for now then.

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

Successfully merging this pull request may close these issues.

3 participants