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

Add venv-based build isolation behind a flag #11619

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

pradyunsg
Copy link
Member

@pradyunsg pradyunsg commented Nov 23, 2022

This is still very much WIP, but it's been something I've been chipping away at for the past few weeks on a regular basis. I'm filing this eagerly, on the basis that "it can't hurt to file early".

I've incorporated some of the choices made in #11602 as a part of this; but that's triggered a bit of a rework of the existing changes I had.

Closes #7778

This will make it easier to organise the additional complexity of having
multiple build environment mechanisms.
This separates the various concerns internally, while still exposing the
same API for the sub-package.
Comment on lines +37 to +53
if sys.platform == "win32":
libpath = os.path.join(
self._temp_dir.path,
"Lib",
"site-packages",
)
else:
libpath = os.path.join(
self._temp_dir.path,
"lib",
f"python{sys.version_info.major}.{sys.version_info.minor}",
"site-packages",
)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use sysconfig here instead? New versions have a venv key for this specific use case, and we can fall back to the prefix scheme otherwise (or maybe limit this to 3.11+).

Copy link
Member Author

Choose a reason for hiding this comment

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

TBH, I'd like to switch over to this in a regular/slightly-long deprecation cycle to let us kill the old isolation logic and apply pressure on redistributors/users to unbreak their Python distributions. But, I digress.

Regarding sysconfig/venv scheme: yea, I think we could but this is matching the current implementation realities across all versions, and isn't particularly complex or difficult to maintain. It's only feasible for this to change in a new Python version which we will have the ability to adapt to. :)

@pradyunsg
Copy link
Member Author

BTW, @uranusjr let me know if you'd mind if I give you co-authorship on a bunch of these commits where I've taken out code from #11602 unchanged basically.

I've been a bit lazy in my commits here but I intend to do that.

@pradyunsg pradyunsg added skip news Does not need a NEWS file entry (eg: trivial changes) and removed skip news Does not need a NEWS file entry (eg: trivial changes) labels Nov 26, 2022
This method is not used or called by anything.
This makes it possible to reuse this logic in places other than the
existing custom `BuildEnvironment` class.
This isn't particularly useful since the `_install_requirements` method
is no longer as complicated.
This enables using a virtual environment directly for handling the
creation of the build environment.
This makes room for a base / protocol for the build environment
interface and for alternative implementations.
This moves shared logic into the base class for all build environments,
providing both runtime protections as well as type checker protections.
This makes it possible for this to have multiple states while still
providing some level of protection via type-checking.
@uranusjr
Copy link
Member

if you'd mind if I give you co-authorship on a bunch of these commits where I've taken out code from #11602 unchanged basically.

Do what you feel is best, GitHub commits is a meaningless metric, OSS work doubly so 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase or merge PR has conflicts with current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip isolated build breaks when a pth file imports setuptools and setup_requires is defined
2 participants