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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching between cargo check and cargo clippy forces pyo3 to rebuild #1551

Closed
nitsky opened this issue Apr 8, 2021 · 8 comments 路 Fixed by #1557
Closed

Switching between cargo check and cargo clippy forces pyo3 to rebuild #1551

nitsky opened this issue Apr 8, 2021 · 8 comments 路 Fixed by #1557

Comments

@nitsky
Copy link

nitsky commented Apr 8, 2021

Hi, I observe that switching between cargo clippy and cargo check forces pyo3 and all downstream crates to rebuild.

馃挜 Reproducing

  1. cargo new --lib example && cd example
  2. Edit Cargo.toml to include pyo3 = { version = "0.13", features = ["extension-module"] }.
  3. Run cargo check.
  4. Run cargo check again, observe no rebuild.
  5. Run cargo clippy.
  6. Run cargo check again, observe pyo3 and all downstream crates rebuild.

馃實 Environment

  • Your operating system and version: Arch Linux
  • Your python version: 3.9
  • How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?: pacman, no virtualenv
  • Your Rust version (rustc --version): rustc 1.51.0 (2fd73fabe 2021-03-23)
  • Your PyO3 version: 0.13.2
  • Have you tried using latest PyO3 main (replace version = "0.x.y" with git = "https://github.com/PyO3/pyo3")?: yes
@nitsky
Copy link
Author

nitsky commented Apr 8, 2021

There are a few places where clippy lints are ignored like so:

#[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_lossless))]

In most other places in the codebase, clippy lints are ignored like so:

#[allow(clippy::cast_lossless)]

I tried replacing the instances of the former with the latter and to my surprise it did not resolve the issue.

@davidhewitt
Copy link
Member

Hi @nitsky I wonder if you are facing a problem similar to #1229?

@nitsky
Copy link
Author

nitsky commented Apr 9, 2021

@davidhewitt The problem is similar. I confirmed that $PATH and $LD_LIBRARY_PATH are not the same when calling cargo check and cargo clippy: When running cargo clippy there are duplicate entries in both environment variables because cargo clippy calls cargo check as a subprocess. Enabling the abi-py36 feature and setting the PYO3_NO_PYTHON environment variable eliminates the rebuild, as noted in the issue you linked. Do you know of a way to set the PYO3_NO_PYTHON environment variable without forcing users of my crate to set it themselves? If that is not possible, is there a good reason that this is not a cargo feature instead?

@davidhewitt
Copy link
Member

Thanks @nitsky that's useful information. I believe I have a fix for you in #1557 which will provide two additional ways to avoid the rebuilds:

  • if running in a Python virtualenv, PyO3 will be able to detect that and use that env's interpreter without checking the problematic variables.
  • setting PYO3_PYTHON variable should also work for your case (as I removed checking of LD_LIBRARY_PATH - I don't think that's needed).

@nitsky
Copy link
Author

nitsky commented Apr 30, 2021

@davidhewitt thank you very much for your work in #1557, that was a good idea to wrap all env var accesses with a function that prints rerun-if-changed. Unfortunately, I still observe that switching between cargo clippy and cargo check triggers rebuilds. I did some more digging and found the issues with cargo below:

rust-lang/cargo#7431
rust-lang/cargo#7432

It turns out that cargo appends $HOME/.cargo/bin to $PATH, and because clippy invokes cargo as a subprocess, that path gets added on multiple times. I don't think there is anything that pyo3 can do beside ignoring $PATH. Is that an option?

@davidhewitt
Copy link
Member

I think not rebuilding when $PATH changes would be incorrect as long as PyO3 continues to detect Python from the $PATH. There could be any number of scenarios when the user intentionally changes the $PATH (e.g. Python virtualenv, but there's many similar tools like pyenv). If PyO3 did not recompile, at best we'd get a miscompilation; there could easily be hard-to-debug errors which users would experience.

We've now got the two mitigations implemented above: either encourage users to work in a virtualenv, or ask them to set PYO3_PYTHON explicitly. Is there a reason that those mitigations are impractical or insufficient for your crate users?


I think the cargo issues you link above describe the problem well and demonstrate that it's not just PyO3 which suffers from cargo's current implementation of the env variable / rerun-if mechanics.

We could imagine that in the future cargo might have support for build scripts to emit a hash of their total output, and if that hash is unchanged then assume the crate does not need rebuilding. e.g. cargo:build-script-result=xyz

In my eyes PyO3's build script is behaving as well as possible given the functionality cargo offers it. Solutions implemented upstream in cargo such as the linked issues or a cargo:build-script-result key would solve this problem across the whole ecosystem.

@birkenfeld
Copy link
Member

birkenfeld commented May 1, 2021

So a hacky solution would be to have a separate build script that is rerun always - well, always if the environment changes at least - (so it needs to be in a separate crate?), let this script do the interpreter selection magic, write the selected interpreter it to a file, and use rerun-if-changed=that-file in the main build.rs...

But maybe cargo can just be fixed to not touch PATH if it already contains what it wants to add.

@Kaiyotech
Copy link

Kaiyotech commented Oct 23, 2023

I'm having this issue as well, but it's not Path that causes it for me, it seems to be VIRTUAL_ENV.

I'm using VSCode 1.83.1, Rust-analyzer using clippy. I ran the builds with the fingerprint, and if I don't modify any files (auto-save is on) then cargo build is clean. But if I modify a file, then pyo3 gets rebuilt. It seems to be complaining that the VIRTUAL_ENV was None and now is (path to my venv). But whenever I check in my PS terminal, VIRTUAL_ENV is there and normal. So I suspect that something is deleting and recreating it whenever a file is saved. Because it doesn't actually change, and it's never None.

A workaround would be cool if there's not a fix yet.

Your operating system and version
Windows 10 Home - 10.0.19045 N/A Build 19045

Your Python version (python --version)
3.9.10

Your Rust version (rustc --version)
rustc 1.72.0 (5680fa18f 2023-08-23)

Your PyO3 version
0.20.0

edit: I was able to solve this by using a command prompt external to vscode with my venv activated. So it's something that VSCode is doing.

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 a pull request may close this issue.

4 participants