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

Insufficient build isolation #206

Closed
abitrolly opened this issue Jan 15, 2021 · 16 comments
Closed

Insufficient build isolation #206

abitrolly opened this issue Jan 15, 2021 · 16 comments
Labels
question Further information is requested

Comments

@abitrolly
Copy link
Contributor

python -m build reuses build directory resulting in files from previous builds to be present in new wheels.

@gaborbernat
Copy link
Contributor

This very likely is a build backend problem and not these tools. https://www.python.org/dev/peps/pep-0517/ imposes no requirements on this. The backend should be able to perform correctly builds even if the build directory is not clean. For example, some backend might use that build directory to cache some files in the build folder, so subsequent builds are faster.

@abitrolly
Copy link
Contributor Author

The PEP however does specify that build frontend SHOULD, by default, create an isolated environment for each build. From that I can assume that if I want a shared cache between environments, it should not be done by default.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 15, 2021

Well, three things there:

  • depends on how you interpret build, I think the spirit there is for each package built, at least that's what reading the reasoning for it reads to me (if package1 build-requires pbr==1.8.1, and package2 build-requires pbr==1.7.2 and It acts as a kind of public health measure to maximize the number of packages that actually do declare accurate build-dependencies.). That part reads to me that build frontends should not reuse the same build environment for different packages. Invoking @pfmoore to (in)validate my read. If we follow your interpretation the PEP-517 would impose that a frontend cannot call build_sdist and build_wheel in the same isolated environment, and it should teardown/setup that for each PEP-517 hook call.
  • SHOULD is not MUST. So the build frontend is not obliged but rather suggested to follow.
  • The isolated environment is the python interpreter and its packages, not the build folder. Just to clarify here which build directory are you referring to here exactly and for what build backend?

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2021

IMO, the intent of PEP 517 is that front ends must have separate build environments for each "set" of differing build requirements. So, if we have

  • p1 build requires pbr==1.8.1
  • p2 build requires pbr==1.8.1
  • p3 build requires pbr==1.7.2

then it is acceptable for the frontend to set up two build environments, A with pbr 1.8.1 and B with pbr 1.7.2, and then use A to build p1 and p2, and B to build p3. In practice, I don't expect front ends to bother with that level of complexity, but it's allowable.

However, this is completely independent from the question of a "build directory". There's nothing in PEP 517 that says that a front end has to create a clean copy of the build directory (as opposed to the build environment). In fact, there's been a lot of discussion about how it's the responsibility of the backend to ensure that it either tidies up build artifacts, or that it isn't affected by left-over files. So I agree with @gaborbernat's comment above that reusing build directories is fine, and it's a backend issue if that doesn't work.

@abitrolly
Copy link
Contributor Author

In fact, there's been a lot of discussion about how it's the responsibility of the backend to ensure that it either tidies up build artifacts, or that it isn't affected by left-over files.

So what was the consensus on that?

@abitrolly
Copy link
Contributor Author

I've left with an impression that PEP 517 fails in the sense that it allows various tools to use source trees as stateful, build-system dependent format for packaging. It should have define the isolation scope for building and caching in tree.

@xavfernandez
Copy link
Member

And if the mentioned backend is setuptools/wheel, you are likely hitting pypa/wheel#147

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2021

So what was the consensus on that?

That it's up to the back end. More specifically, that PEP 517 doesn't mandate any behaviour, and front ends are permitted to run the hook in an existing build directory, and it's down to the backend to work in that situation.

I've left with an impression that PEP 517 fails in the sense that it allows various tools to use source trees as stateful, build-system dependent format for packaging.

No, it allows that, but that's not a failure. It's a deliberate provision for tools/projects that have a need for incremental builds or similar means of optimising repeated builds.

@abitrolly
Copy link
Contributor Author

@xavfernandez the backend is build-backend = "setuptools.build_meta", and yes I am building wheels. With many hacks so far. My project is a single Python module, and I need to distribute binary data with it. So I had to create an empty __init__.py in data directory to use package_data, then add dir into MANIFEST.in, then create setup.cfg listing this dir as a package and adding include_package_data = True, and after that I've got the wheel with needed files. Now I need a bash script to rename the wheel, because the it is Linux only. Last 12 hours I spent trying to figure out why my data is copied multiple times. It appeared that during experiments with package_data I managed to copy it into wrong place just once. To figure out what's happening, I patched pip, setuptools, discovered DISTUTILS_DEBUG flag. Now I am here at the build to see how to pass --no-cache flag to setuptools, and it appears that I can't (#205) because frontend doesn't care, and PEP 517 doesn't specify interface checks.

@pfmoore whatever it is failure or not, I am frustrated by the current state of things.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 15, 2021

it appears that I can't (#205) because frontend doesn't care

We do. And @layday helped you with how you can pass it in. Note, however, this does not help you until setuptools decides to accept those arguments. And that's something you'll need to pick up with the maintainers of that tool, neither the PEP system nor this frontends maintainer has any implication on that side.

PEP 517 doesn't specify interface checks.

Can you clarify what do you mean exactly here?

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2021

@pfmoore whatever it is failure or not, I am frustrated by the current state of things.

Understood. But what we're trying to do is help you find the right people to discuss your frustrations with. As we've explained, this is a backend issue, so you need to talk to the setuptools maintainers if you expect to make any progress. Simply repeating your problems here won't help, as we have no means to address them, because we don't have anything to do with setuptools. And suggesting that we "don't care" is definitely unhelpful, and frankly rather annoying, given that we're trying to help you with your problem.

Sorry if you feel this is insufficient, but I'm not sure what else we can do.

@abitrolly
Copy link
Contributor Author

PEP 517 doesn't specify interface checks.
Can you clarify what do you mean exactly here?

In web development, if there is a frontend and backend, there is also a defined API between them. In this case the API for checking backend configuration and handling configuration errors is not specified. The backend could expose data about its supported configuration options, and let frontend do the sanity check on its side.

@abitrolly
Copy link
Contributor Author

@pfmoore my main frustration is with the non-disputable decision to not enforce correct stateless builds by default. That leaves users with more things to keep in mind than they'd like to. The priority for correct builds in my worldview it to get the repeatable build by default, and address incremental builds and caches on demand.

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2021

Backward compatibility makes it impossible for us to enforce that. Setuptools doesn't provide that facility and we can't mandate something that the majority of projects can't do (because they use setuptools). Again, you need to discuss this with setuptools, if anywhere, and not here.

I'm giving up on this conversation now. We've told you how to progress this, but you seem more interested in continuing to criticise. I'm afraid I don't have anything further to add.

@FFY00
Copy link
Member

FFY00 commented Jan 15, 2021

But you can get that, you are just barking up the wrong tree. A backend can very well provide you with that, this project is just a way to invoke the backend, with an extra functionality of being able to install the backend on an isolated environment and run it there.

You are using a backend that chooses to build in-tree without cleaning up after itself. There is nothing preventing it from doing what you just described.
You are in control of the backend you use, you can choose whatever you like. I don't think forcing the rest of the ecosystem to optimize for often unneeded requirements, when you can do that when you need it, is sensible.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 15, 2021

I think we can close this ticket because we concluded at this point that the underlying issue is in the build backend according to the current specification. If the specification needs to change then you should propose that under https://discuss.python.org and propose a PEP; if the backend needs a fix then you likely need to request a fix on the pypa/wheel#147 side.

We're sorry we don't have something better for you, but this is where the ecosystem stands now. Contributions are welcome if you'd like to take part in chaing that.

@pypa pypa locked as resolved and limited conversation to collaborators Jan 15, 2021
@gaborbernat gaborbernat added the question Further information is requested label Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants