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

Allow profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable #364

Merged
merged 1 commit into from Oct 15, 2023
Merged

Allow profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable #364

merged 1 commit into from Oct 15, 2023

Conversation

jefferyto
Copy link
Contributor

This allows the profile to be set dynamically, without having to edit pyproject.toml/setup.py.

@davidhewitt
Copy link
Member

I understand the desire for this (and have wanted it myself) but would like this to be called something other than CARGO_PROFILE as cargo itself does not recognise that environment variable. Perhaps a PR to cargo to implement this upstream would solve your use case and also help more users?

@jefferyto
Copy link
Contributor Author

Perhaps a PR to cargo to implement this upstream would solve your use case and also help more users?

This would be better but I don't have time to wade into cargo internals right now 😅 I couldn't find any mention of this functionality in their issues/PRs, that is slightly surprising.

Also it turns out CARGO_PROFILE is already used to enable an internal profiler, so that name is probably a no-go already.

Would SETUPTOOLS_RUST_PROFILE or SETUPTOOLS_RUST_CARGO_PROFILE be acceptable?

@jefferyto jefferyto changed the title Allow cargo profile to be set by CARGO_PROFILE environment variable Allow cargo profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable Oct 7, 2023
@jefferyto jefferyto changed the title Allow cargo profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable Allow profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable Oct 7, 2023
@jefferyto
Copy link
Contributor Author

I have changed the variable name to SETUPTOOLS_RUST_CARGO_PROFILE.

FYI I'm the Python maintainer at the OpenWrt packages feed; I'm working on improving the building of Rust extensions for the Python packages there. It would be much easier to patch setuptools-rust to accept this environment variable than to patch args=["--profile=..."] (and reading from an environment variable) into every Python package that needs it. (The profile selected would be determined by the OpenWrt build system.)

I'm not a Rust person so getting this isn't cargo isn't my first choice/priority right now.

jefferyto added a commit to jefferyto/openwrt-packages that referenced this pull request Oct 8, 2023
@jefferyto
python-setuptools-rust: Set cargo profile from environment variable
80ba57f 
This adds a patch (submitted upstream in
PyO3/setuptools-rust#364), to read the profile
to pass to cargo from an environment variable.

This also updates the Python include files to set the environment
variable based on values from rust-values.mk.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
jefferyto added a commit to jefferyto/openwrt-packages that referenced this pull request Oct 8, 2023
@jefferyto
python-setuptools-rust: Set cargo profile from environment variable
f2c3fae 
This adds a patch (submitted upstream in
PyO3/setuptools-rust#364), to read the profile
to pass to cargo from an environment variable.

This also updates the Python include files to set the environment
variable based on values from rust-values.mk.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
jefferyto added a commit to jefferyto/openwrt-packages that referenced this pull request Oct 11, 2023
@jefferyto
python-setuptools-rust: Set cargo profile from environment variable
29ca979 
This adds a patch (submitted upstream in
PyO3/setuptools-rust#364), to read the profile
to pass to cargo from an environment variable.

This also updates the Python include files to set the environment
variable based on values from rust-values.mk.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
jefferyto added a commit to jefferyto/openwrt-packages that referenced this pull request Oct 12, 2023
@jefferyto
python-setuptools-rust: Set cargo profile from environment variable
6544ab7 
This adds a patch (submitted upstream in
PyO3/setuptools-rust#364), to read the profile
to pass to cargo from an environment variable.

This also updates the Python include files to set the environment
variable based on values from rust-values.mk.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
(cherry picked from commit 29ca979)
@davidhewitt
Copy link
Member

Sorry for the delayed reply. Understood, in which case to help you move forward let's accept the environment variable here for now.

Can you please add a CHANGELOG entry, and then I'll get this merged and released asap.

@jefferyto
Copy link
Contributor Author

Thanks 🙏 I've updated the CHANGELOG.

@jefferyto
Allow profile to be set by SETUPTOOLS_RUST_CARGO_PROFILE env variable
8489360 
This allows the profile to be set dynamically, without having to edit
pyproject.toml/setup.py.
@jefferyto
Copy link
Contributor Author

Reformatted with black.

davidhewitt
davidhewitt approved these changes Oct 15, 2023
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Will ship now 👍

@davidhewitt davidhewitt merged commit a6e82c2 into PyO3:main Oct 15, 2023
40 checks passed
@jefferyto jefferyto deleted the cargo-profile-env-var branch October 16, 2023 16:08
graysky2 pushed a commit to graysky2/packages that referenced this pull request Oct 23, 2023
@jefferyto @graysky2
python-setuptools-rust: Set cargo profile from environment variable
615b97d 
This adds a patch (submitted upstream in
PyO3/setuptools-rust#364), to read the profile
to pass to cargo from an environment variable.

This also updates the Python include files to set the environment
variable based on values from rust-values.mk.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jefferyto @davidhewitt