pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.49k stars 3.01k forks source link

Feature request: configurable location for keyring as a subprocess #12750

Open jfly opened 4 months ago

jfly commented 4 months ago

What's the problem this feature will solve?

Today, pip using keyring as a subprocess looks for keyring on the PATH. However, this breaks if you've got a carefully configured systemwide "keyring" binary (say at /usr/bin/keyring), and then inside of a virtualenv you install keyring. The keyring python package will install its own keyring binary in the venv, and shadow your systemwide keyring.

Describe the solution you'd like

Being able to configure the location of my systemwide keyring would solve this: I could hardcode the path to /usr/bin/keyring, and not have to worry about the keyring binary getting shadowed.

Alternative Solutions

I don't know of any workarounds for this other than being careful not to install keyring in a venv.

Additional context

Code of Conduct

Darsstar commented 4 months ago

So https://github.com/pypa/pip/blob/main/src%2Fpip%2F_internal%2Fnetwork%2Fauth.py#L207 fails to filter out the venv's keyring?

keyring‐subprocess is an attempt to implement something like the --keyring-provider subprocess feature as a keyring backend. It looks for a keyring-subprocess executable instead of keyring to avoid the shadowing issue. But the library has to be seeder into new venvs somehow.

jfly commented 1 month ago

(sorry for the delayed response here)

So https://github.com/pypa/pip/blob/main/src%2Fpip%2F_internal%2Fnetwork%2Fauth.py#L207 fails to filter out the venv's keyring?

Ahh, I see! I'll admit, my eyes completely glossed over that code when I saw it. Now I understand what it's doing.

This doesn't work for me, because I lied a bit in my OP, sorry! I'm actually on NixOS, and I often end up in nix shells that have keyring in the PATH (adding dependencies to the PATH is a common pattern with nix dev shells).

@Darsstar, how would you feel about extending the existing "subprocess" keyring provider to take an optional path? I'm thinking something like --keyring-provider=subprocess:/path/to/keyring. If a path isn't provided, then the current behavior (search the PATH, skipping past the surrounding venv) still happens.

If that syntax sounds too funky, I could also see adding a new --keyring-provider-subprocess-path option to let people give us an explicit path to the keyring binary.

I'd be happy to put together a PR implementing this if you're on board with it.

uranusjr commented 1 month ago

The subprocess:<path> syntax is a bit difficult since currently the choices are from an enum. We could change that to be manually verified of course, but the use case doesn’t seem to be common enough for the overhead. I wonder if an environment variable would work? It might fit better for your use case too since you can easily configure it globally instead of needing to pass in the argument for every call.

jfly commented 1 month ago

Yep, an environment variable would totally work for me!

I'll put together a PR, and we can bikeshed over the variable name over there :)

jfly commented 1 month ago

Ok, I'm stumped. I was trying to pattern match the existing code, and I see this PIP_KEYRING_PROVIDER environment variable documented here. But how does it work? I see no references to it in pip's codebase (besides the aforementioned docs), nor any non-test code that instantiates a MultiDomainBasicAuth with a keyring_provider specified.

Is this PIP_KEYRING_PROVIDER env var a real thing? Or is it just some doc-rot?

jfly commented 1 month ago

Ah, nevermind! I see now that we directly manipulate our MultiDomainBasicAuth instance's keyring_provider attribute here, and pip has generic logic for reading its configuration from env vars.

So, I'm sort of inclined to build this as a cli flag, knowing that I'll get an environment variable for free.

uranusjr commented 1 month ago

Something like pip install --keyring-provider=subprocess --keyring-executable <path> probably works