Skip to content

Override leaky env vars#406

Closed
layday wants to merge 40 commits into
pypa:mainfrom
layday:override-leaky-env-vars
Closed

Override leaky env vars#406
layday wants to merge 40 commits into
pypa:mainfrom
layday:override-leaky-env-vars

Conversation

@layday
Copy link
Copy Markdown
Member

@layday layday commented Nov 12, 2021

On top of #361.

layday added 30 commits November 5, 2021 16:54
"build" is the name of the package.
* `IsolatedEnv` reworked to subsume env creation logic.

* `IsolatedEnvBuilder` renamed to `IsolatedEnvManager`.

* `ProjectBuilder.from_isolated_env` added to auto-fill init args from
  an isolated env.

* Made `IsolatedEnv` responsible for customising the environ as a
  prelude to fixing pypa#377.  Removed `ProjectBuilder.scripts_dir`.

* `ProjectBuilder`'s @properties made read-only. Mutating the builder
  is not necessary with the addition of
  `ProjectBuilder.from_isolated_env`.
Fixed passing the absolutised srcdir.
For example `sys.executable` would have been expanded on RTD.
This is to get annotations in `TYPE_CHECKING` blocks to show.

We also mustn't import build before sphinx has a chance to.
There's three modules now which are listed on the sidebar.
This is required to resolve annotations in sphinx which are not
generic at runtime (`os.PathLike`, in particular).
Also link to the correct PR.
@adamjstewart
Copy link
Copy Markdown

Fixes #266

@layday
Copy link
Copy Markdown
Member Author

layday commented Jan 23, 2022

It fixes one particular cause of that error. I think that we should provide a better message for when pip can't be found, that's what #266 ought to fix.

@gaborbernat
Copy link
Copy Markdown
Collaborator

@layday shall we close this now, or you want to rebase it?

@layday
Copy link
Copy Markdown
Member Author

layday commented Jan 25, 2022

This is blocked by #361, no point rebasing before #361 is resolved.

@gaborbernat
Copy link
Copy Markdown
Collaborator

This is blocked by #361,

Approved.

@layday
Copy link
Copy Markdown
Member Author

layday commented Jan 27, 2022

Feel free to cherry-pick whatever makes sense.

@layday layday closed this Jan 27, 2022
@salotz-sitx
Copy link
Copy Markdown

Seeing as #361 isn't going to be merged should this be reopened? Or am I missing what the decision is?

@FFY00
Copy link
Copy Markdown
Member

FFY00 commented Feb 1, 2022

The problem is lack of maintainer time. I was unable to review this in time, so @layday decided to close it. I hope to pick it up in the next few months.

@henryiii
Copy link
Copy Markdown
Contributor

henryiii commented Feb 1, 2022

There's somewhat of a bottleneck waiting on @FFY00 to review #361. I think the plan is still to go with #361, we even had an approval for it.

@salotz-sitx
Copy link
Copy Markdown

Got it, thank you!

@layday
Copy link
Copy Markdown
Member Author

layday commented Feb 1, 2022

It's nothing personal, just having PRs linger fills me with a weird sort of anxiety.

@salotz-sitx
Copy link
Copy Markdown

Whatever makes you more productive :) I just wanted to know if I should keep following along or not.

@henryiii
Copy link
Copy Markdown
Contributor

henryiii commented Feb 1, 2022

We can reopen as needed, so they aren't just sitting open. :) I'm also happy to volunteer to rebase as needed. It's just @FFY00 is the most qualified to review.

@salotz-sitx
Copy link
Copy Markdown

For anyone looking for workarounds coming from the Spack world.

I install Python and all my run time dependencies using Spack but for development tools (like build) I typically just install them with pip in a venv on top of the Spack virtual environment. Installing build alone I get this problem, but build[virtualenv] works for me in this case.

@adamjstewart
Copy link
Copy Markdown

Spack should no longer have this issue in the latest release. I added a patch to unset PYTHONPATH to avoid this issue: spack/spack#28562

@adamjstewart
Copy link
Copy Markdown

Was this issue ever fixed in main or is a patch still needed?

@layday
Copy link
Copy Markdown
Member Author

layday commented Feb 3, 2023

Still needed.

@adamjstewart
Copy link
Copy Markdown

According to #470, the recommended way to bootstrap build is to install it from a wheel. How do I patch a wheel?

@adamjstewart
Copy link
Copy Markdown

Or is there a way to use build to build itself?

@layday
Copy link
Copy Markdown
Member Author

layday commented Feb 3, 2023

You can run build from source to build build with build.

@henryiii
Copy link
Copy Markdown
Contributor

henryiii commented Feb 3, 2023

#361 was replaced by #537, so can't this be redone on top of that? I can't actually tell anymore what this PR was changing due to 361 and this not being regularly rebased (merge commits are evil).

@layday
Copy link
Copy Markdown
Member Author

layday commented Feb 3, 2023

These are the two changes unique to this PR:

@adamjstewart
Copy link
Copy Markdown

The specific bug we're facing is #266, so once #266 is solved we'll no longer need to patch. This patch was sufficient for us to solve the issue, but I'm not sure how hacky that is. It's also outdated, see spack/spack#35326 for my latest patch.

@layday layday deleted the override-leaky-env-vars branch March 1, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants