env: refactor `IsolatedEnv`
The IsolatedEnv is made responsible for env creation to allow pip
to implement custom env creation logic.
~~IsolatedEnvBuilder takes an IsolatedEnv class as an argument
so that bringing your own IsolatedEnv won't require re-implementing
the builder.~~
So from what I can understand, pip should implement its own IsolatedEnv subclass, pass it to the builder, and do something like
with builder as env:
env.install_packages([...])
subprocess.call([env.python_executable, "setup.py", "bdist_wheel"]) # Just to illustrate how to use the env.
right?
Yep, that's right. Typical usage would be along the lines of:
with build.env.IsolatedEnvBuilder(PipIsolatedEnv) as env:
builder = build.ProjectBuilder(
source_dir,
python_executable=env.python_executable,
scripts_dir=env.scripts_dir,
)
env.install_packages(builder.build_system_requires)
env.install_packages(builder.get_requires_for_build('wheel'))
builder.build('wheel', output_dir)
Yep, I just wanted to garner some feedback before diving any deeper.
I could also use this in the bootstrap script that I am using.
I'll try to work on it tomorrow.
No worries, I am currently monkeypatching build.env._create_isolated_env_venv, which is fine because I pull pypa/build as a submodule.
The new API implementation is complete and I'd welcome any feedback. I realise that this PR packs quite a bit and might be difficult to wade through. I could look at splittiing it up into smaller chunks but I wanna make sure everyone's okay with the direction this has taken first.
The CI is also failing.
If anybody can figure out why the docs check is failing I'd be very interested to know...
Right, it's because os.PathLike is not generic on Python 3.8. I'll bump the docs Python to 3.9 in tox.
(I probably won't have time to work on this from next week so I'd appreciate any reviews over the weekend.)
@FFY00 Would you like to review before merging?
Sorry, I'll try to review this today!
For the lineno checks, let's get the function lineno and add the relative lines inside the function. Eg.
>>> import build.env
>>> build.env.IsolatedEnvBuilder.log.__code__.co_firstlineno + 10
138
Done, thanks for the suggestion.
Thank you for the changes! I provided some more feedback. This is starting to look better, but still has some things I think should be changed :sweat_smile:
@FFY00 I'm approving this, anything else left to do?
I'll try to review the new changes this weekend.
Just wanted to let you know that I haven't forgotten about this, I just have a bunch of things going on right now, and the holidays and end of year are making me even busier. I don't think there is any particular reason to rush this, as it is not critical to the affected parties, and they likely are on the same boat as me :sweat_smile:. I will try to get back to this in January.
To recap what has changed here in a major way:
- The env builder is no more.
IsolatedEnvhas been stripped down to the bare essentials - currently that's just the two properties,environandpython_executable. This will allow pip to slot in its own isolated env but it does not make our ownIsolatedEnvamenable to reuse by pip. This is because our requirements are fundamentally different. pip will not want pip to be installed byensurepiporvirtualenv; they will want to expose the running pip to the isolated env. pip maintainers have also not expressed interested in supportingvirtualenvin place of or in addition to the stdlib venv like we do. Furthermore, pip supports a large number of installation options which our single-minded environment does not and that we would not be able to abstract as part of the isolated env protocol. - The runner has been modified to allow removing env vars from the environ - not just replacing them - as a precursor to #406. This has rendered the callbacks from
pep517incompatible with the project builder.
Judging by https://github.com/pypa/pip/pull/10720, pip isn't interested in using build, so I'm closing this.
As I see it, moving to a venv-based build environment in pip is the first step in making it possible for pip to move to using build, so I'm a genuinely confused by the closing note on this issue.
I was given the impression that pip would be using venv directly in unison with pep517 (the library). The pip PR would have closed the pip issue where this PR was conceived and I had not heard from pip maintainers that they were interested in pursuing this further. If there is interest, all the code is still here :)
In that case, do you want to reopen this? :)
This will be one of my priorities for the PyCon sprints.
What can I do to help move this forward? :)
Review my assumptions in https://github.com/pypa/build/pull/361#issuecomment-1007244397 and provide feedback on the isolated env interface - if there are any obvious methods or properties missing, if something doesn't feel right, etc. We are gonna need to rebase but the existing isolated env interface hasn't changed fundamentally since this PR.
Personally stuck on the rebase. I started it, it's a pretty hefty one - the comments were not really very organized / squashed, so it was a lot of manual work. I think I was doing it in steps, or doing a manual merge. I then lost my spot and haven't restarted working on it. I've been traveling for a month and am pretty busy for the next week (workshop), but can probably work on this after that.
- This is because our requirements are fundamentally different.
That is correct, and not a problem. pip will likely create an environment with venv.EnvBuilder and use its internal runner script to run itself within that environment to install packages.
- The runner has been modified to allow removing env vars from the environ
That seems fine? I guess I don't understand the implications of this fully though I imagine that pip's not gonna handle anything about this differently than it does today.
That seems fine? I guess I don't understand the implications of this fully though I imagine that pip's not gonna handle anything about this differently than it does today.
We wanted to be able to clear a bunch of PYTHON* env vars like PYTHONHOME and PYTHONPATH. You can't do that with the stock pep517 subprocess wrappers. I don't remember the specifics anymore, if I'm honest. But it's a breaking change to the public build API.
Yep, we need to create a subprocess with a controlled environment, where we remove some of these keys.