install-wheel: add support for specifying python startup flags #13
Conversation
eli-schwartz
force-pushed
the
script-flags
branch
from
2d14514
to
8c7405b
Compare
| @@ -240,14 +240,17 @@ def install_wheel_impl(args, wheel: Path): | |||
| from installer.sources import WheelFile | |||
| from installer.utils import get_launcher_kind | |||
|
|
|||
| from .scheme import Gpep517WheelDestination | |||
There was a problem hiding this comment.
I don't use relative imports. Be consistent.
gpep517/scheme.py
Outdated
| # SPDX-FileCopyrightText: | ||
| # SPDX-License-Identifier: MIT | ||
|
|
There was a problem hiding this comment.
While these headers are all fun, they're inconsistent with the rest of the codebase.
There was a problem hiding this comment.
I see I forgot to update the first line to include the pypa/installer copyright line.
I don't care what format the headers are in, and I'm not trying to be "fun". I do, however, think that it is important when you are copying code from another project to state where you got it, who wrote it, and what it's licensed under. That is the purpose of having a separate copyright declaration that is not part of "the rest of the codebase".
gpep517/scheme.py
Outdated
|
|
||
| from __future__ import annotations | ||
|
|
||
| import contextlib, io, os, shlex, zipfile |
There was a problem hiding this comment.
Bit disappointing tbh... all it does is make files longer and more annoying to scroll down to the actual content, but so be it.
| from __future__ import annotations | ||
|
|
||
| import contextlib, io, os, shlex, zipfile | ||
| import typing as T |
There was a problem hiding this comment.
Again, all this does is make files longer and more annoying to read, since most type annotations suddenly gain an additional 5 characters of overhead towards the max line length.
Either that or you spend more time managing individual from typing import Foo, Bar, Baz imports than you spend coding.
| # with minor tweaks where classes don't have hooks to control their | ||
| # internals. | ||
|
|
||
| from __future__ import annotations |
There was a problem hiding this comment.
It is required to make if T.TYPE_CHECKING imports not be runtime errors. It is also a small performance improvement for all code that has type annotations, period.
Type annotations are not needed at runtime unless you're using something like beartype or pydantic. Building them into code objects and discarding them is a bit wasteful.
The import is equivalent to surrounding all type annotations with quotes. I find the future import to be more ergonomic, and could add it to other files if you like... ?
eli-schwartz
force-pushed
the
script-flags
branch
2 times, most recently
from
3e79825
to
2b65308
Compare
``` gpep517 install-wheel --shebang-flags='-s' ``` will cause all /usr/bin scripts to pass `-s` to the interpreter in its shebang. Useful for ensuring that system commands do not import mismatched modules from `pip install --user`. Also useful in theory to produce programs that run via -OO, something the python ecosystem rarely remembers is possible because scripts cannot be easily fine-tuned and short of changing the entrypoint script you cannot activate -O. Perhaps we could change that.
eli-schwartz
force-pushed
the
script-flags
branch
from
2b65308
to
07241af
Compare
|
Updated. |
|
Any update on what I should do here? Upstream PyPA/installer hasn't merged any PRs in months other than pre-commit.ci spam and isn't responsive to requests for review. Are we still waiting on them or is it fine to reimplement functionality locally? Are there any responses to the answers I gave on review comments where I disagreed with the review? |
will cause all /usr/bin scripts to pass
-sto the interpreter in its shebang. Useful for ensuring that system commands do not import mismatched modules frompip install --user.Also useful in theory to produce programs that run via -OO, something the python ecosystem rarely remembers is possible because scripts cannot be easily fine-tuned and short of changing the entrypoint script you cannot activate -O. Perhaps we could change that.