PEP 517 isolation breaks test module with relative out-of-source dependency

[pganssle]

Environment

• pip version: 19.0.2
• Python version: 3.8.0a1
• OS: Linux

Description
In the PyO3 project, we have a test module nested inside the examples folder that needs to build against the version of pyo3 in the repository, which is located in ../.. relative to the source root, because it's intended to test that version of PyO3. This is accomplished by specifying the path of the pyo3 root in Cargo.toml.

The problem is that when using PEP 517, the build root gets moved out of the source tree (fair), and we no longer have a reliable way to specify a relative out-of-source dependency.

Given that this is a pretty unusual requirement, I'm going to investigate various ways to work around this from within setup.py (maybe rewrite the Cargo.toml file with an absolute file path as part of an sdist build or write a custom PEP 517 backend), but I thought I'd bring it up as a potential incompatibility with PEP 517.

See PyO3/pyo3#362

[pganssle]

I'll note that I seem to be able to partially fix this by setting TMPDIR so that the temporary directory build occurs within the source tree, but it's still failing in some weird ways - I can't seem to specify the relative path in a way that actually hits pyo3, I either get an error that Cargo can't be found in the directory above pyo3 or two directories *below pyo3`.

Very hard to say what's going on there - possibly this is a problem with setuptools_rust.

[pradyunsg]

Pinging @pfmoore on this.

[pfmoore]

This isn't related to PEP 517 as such. Rather, it's related to pip not using in-place builds. Which is something we've started doing with PEP 517, but we'd discussed on a number of occasions for legacy builds (and TBH, I thought we were doing it, but obviously not...) So I'm going to remove the "PEP 517 impact" label, as that's something of a red herring.

In-place builds was something that was considered during the PEP 517 discussions, and deferred as way too complicated for us to reach any sort of agreement on. At the time, the focus was around incremental builds, for scientific packages that had significant C code build steps, where build times were helped a lot by reusing already built object files. So there's another group of people interested in this area.

But I'm not honestly sure even what is being requested here. You seem to have a project with a dependency that somehow must be ../../<something>, but that doesn't make any sense if you consider that project being made into a sdist. How would you build the sdist without insisting on the user somehow installing that external dependency "by hand" and somehow tying it to the project? Maybe you could simply document that the dependency must be installed manually, and an environment variable set to point to it? That would be more flexible than insisting that the dependency has to be in a specific location relative to the project. But I don't really understand the requirement, so maybe that's not practical for some reason.

I think that a general solution here would require a new PEP to handle it - and given that it was "too hard" to include when we were discussing PEP 517, I think that whoever champions that PEP would need to be prepared for a pretty long and complex discussion...

If we had an "awaiting a new PEP" label, I'd add that here - but I don't know if it's worth adding such a label just for this issue. Regardless, I don't think there's anything for pip to do before we get some sort of standard approach agreed.

[pganssle]

@pfmoore

But I'm not honestly sure even what is being requested here. You seem to have a project with a dependency that somehow must be ../../, but that doesn't make any sense if you consider that project being made into a sdist.

This is not a project that would be made into an sdist, it's actually part of a test suite for a Rust library (PyO3) that provides bindings to the C Python API. It needs to refer to the root of that repository in the Cargo.toml otherwise it will likely build against a version on crates.io. As I said earlier, this is a very unusual situation.

This isn't related to PEP 517 as such. Rather, it's related to pip not using in-place builds. Which is something we've started doing with PEP 517, but we'd discussed on a number of occasions for legacy builds (and TBH, I thought we were doing it, but obviously not...) So I'm going to remove the "PEP 517 impact" label, as that's something of a red herring.

I'm fine with however you want to categorize it, since that's just about how this project tracks its issues, but I think it may be splitting hairs a bit to say that this is not a PEP 517 problem because it's not inherent to PEP 517. I think this is specifically about pip's implementation of PEP 517. This is triggered by the fact that I have a pyproject.toml that was added for PEP 518 support (since this project needs setuptools_rust) and broke when PEP 517 was implemented because the old (now unavailable) PEP 518 implementation did the build in-place.

The current workarounds are using tox, directly invoking setup.py or pinning to an old pip. Removing the pyproject.toml might work as well.

In the end I realize that we have at least been partially counting on something that wasn't necessarily guaranteed (the fact that the build happens in place), and the vast majority of people won't have this specific requirement, so I don't think it needs to be a high priority if it's addressed at all. I just wanted to file a ticket so that the project would be aware of the issue.

[pfmoore]

This is not a project that would be made into an sdist

If that's the case, then I'd personally say that it isn't something I'd expect to be managed with pip. I know there are some people that want pip to be the unified front end for any management of Python code, but that's not my view - I think that by expecting pip to cover everything, we end up with a tool that's too complex for its own good. (I'm a fan of the Unix philosophy of small, focused tools that do one job well).

So maybe my opinions here aren't going to help much ;-)

I think it may be splitting hairs a bit to say that this is not a PEP 517 problem because it's not inherent to PEP 517. I think this is specifically about pip's implementation of PEP 517.

OK. It's a debatable point. But I don't think it matters much.

In the end I realize that we have at least been partially counting on something that wasn't necessarily guaranteed (the fact that the build happens in place), and the vast majority of people won't have this specific requirement, so I don't think it needs to be a high priority if it's addressed at all.

Yes. Pip has never guaranteed that we'll build projects in place, but equally we've always (prior to PEP 517/518) been stalled changing that because of backward compatibility problems. We took the opportunity with PEP 517/518 to make the switch - not least because we were already having to switch to always installing via wheel (because PEP 517 has no "direct install" option) so it was good to do all the changes together.

Longer term, the intention is to move to never even building wheels directly, but rather going build_sdist -> build_wheel -> install. This fixes the current issues we have with copying the source (which did occur in the legacy process, just not in a scenario that affected you, it seems) where the copy is way too slow because a lot of unnecessary files are copied. Of course, whether that intention is ever implemented depends on whether anyone has the time to spend on it (or whether things change in the meantime, and make that plan obsolete...) I suspect that might be an even bigger problem for you.

It's certainly arguable that it's not as easy for projects to "opt out" of PEP 517/518 as we'd like - tools using pyproject.toml for general configuration was an unexpected complication - but I'd still argue that "using PEP 517/518 features" was the best breakpoint we could have found for a change like this, even if it's not ideal for some projects.

I just wanted to file a ticket so that the project would be aware of the issue.

And thanks for doing so. I'm not objecting to the ticket at all, I just want to make sure that the explanation is clear, for people who find the ticket later.

(PS I should say that the history of in-place builds within pip is long, and complex - we've tried to change things in the past, certainly. And my memory of what the behaviour was at any particular time may be wrong. So apologies in advance for any inaccuracies in the above. "These are purely my personal views and recollections" and all that ;-))

[pganssle]

If that's the case, then I'd personally say that it isn't something I'd expect to be managed with pip. I know there are some people that want pip to be the unified front end for any management of Python code

It may be getting a bit too much in the weeds on this specific example, but in this case I would prefer to use pip because this is part of a test suite. It has quirks relating to relative paths because of this, and it's not something any downstream users would install for themselves, but being able to use pip is important because most of the users of the thing it's testing will be using pip to install their extensions, so we should be too.

It is not a problem to create an intermediate sdist since we use tox to test it, and that's step 1 of tox's process. The main thing is that we either need a way to consistently specify what the build location will be, relative to the current directory (I'm not sure why TMPDIR is not working for this, it seems two different parts of the build perform actions at two different levels under the tmpdir), or we need some better way to specify "use a directory relative to the executor's PWD, not relative to Cargo.toml" or something.

We may be able to fix this by creating a custom sdist command that modifies the Cargo.toml in place with an absolute link to pyo3, or something of that nature, though it may be worth looking into whether pip is actually doing build actions at two different levels of nesting under the TMPDIR, in which case keeping that consistent might help bridge the gap for people relying on some features of the in-place nature of the old-style builds. (Let me investigate that one further, it could be that the "two levels" thing is actually some feature of cargo, not something inherent to pip).

[sbidoul]

This issue looks similar to #3500.

[pradyunsg]

Okay, this is blocked on #2195 -- building local directories, via sdist.

[sbidoul]

This should be resolved by #7882 (build local directories in place).

[pradyunsg]

We have now (per #7951) published a beta release of pip, pip 20.1b1. This release includes #7882, which implemented a solution for this issue.

I hope participants in this issue will help us by testing the beta and checking for new bugs. We'd like to identify and iron out any potential issues before the main 20.1 release on Tuesday.

I also welcome positive feedback along the lines of "yay, it works better now!" as well, since the issue tracker is usually full of "issues". :)

[davidhewitt]

Thanks, can confirm that for pip 20.1 the problem reported originally in this ticket is resolved! 🚀

[pfmoore]

@davidhewitt Thanks for the verification on this - we appreciate you taking the time to report back 🙂

Unfortunately, there have been a number of issues with the implementation of in-place builds (which are being tracked under #7555) which means that for now, we need to revert this change. As a result this issue will become a problem again, and we'll therefore be reopening it. Longer-term, we hope to have a solution that addresses the issues that in-place builds solved, but without the impact on other workflows that the current solution had. Sorry for the disruption that this will cause.

(@pradyunsg - can you please include this information in the other issues covered by the in-place build changes, as we discussed?)

[sbidoul]

Now that #7555 has landed for good, I think this can be closed again. Holler if not.