Configuration files may now also be stored under sys.prefix

[zooba]

Adds sys.prefix as a valid location for global configuration.

Fixes #5060

[zooba]

This is the most trivial approach, and figured it was enough to start a concrete discussion rather than the abstract one over on the bug.

My main concern is that I think this approach makes it the config file that will be modified by pip config set --global …? And I'm not sure that's what we want...

[zooba]

My main concern is that I think this approach makes it the config file that will be modified by pip config set --global …

Indeed it is, and I don't like that, so I changed it to insert(0, sys.prefix) instead of .append().

[pfmoore]

Be careful here - we have both a local copy of appdirs and a vendored one, and I don't think they are consistently used. It's a mess that should be sorted at some point (probably by switching to just using the vendored one, which means patching appdirs will be something we'd want to avoid).

[zooba]

In my search of the current source tree, the vendored one is used once in the vendored pkg_resources, and not in pip's codebase itself. And the only function used in pip itself is one that doesn't exist in the vendored one at all, so I suspect we'd be better off removing the vendored one and keeping the fork.

[zooba]

Rerunning CI, and hopefully the macOS build won't fail with an HTTP timeout this time.

[pfmoore]

the vendored one is used once in the vendored pkg_resources

Ah, thanks, it looks like things have changed since I last looked. Sorry for the noise.

[zooba]

So it seems macOS doesn't like files.pythonhosted.org today, for some reason. I'll requeue the build next time I take a look, assuming there's no further discussion or changes needed before then.

[pradyunsg]

I would like this to be handled in a very different manner, by making changes to pip's existing configuration pipeline code. That'll put this new file in the same footing as all the others -- being able to use pip config on it.

Here's how I'd suggest doing this:

• Add the configuration file path in pip._internal.locations
• Add the relevant details to parts of pip._internal.configuration
  • kinds
  • Configuration (_load_config_files, _override_order, valid_load_only)
  • I think there might be a couple of other things to change here
• Add an extra option to pip config to enable the ablity to manage this file via pip's CLI

[zooba]

So in terms of naming, I would consider this the "site config file", but that name is already used to mean global config files (and is already a list of files, which is why I added to that one). Suggestions?

[zooba]

Alternatively, make venv actually mean the current environment, regardless of whether it's a venv or not. Which basically means just removing the running_under_virtualenv() check.

Addressed.

[pradyunsg]

As for the current approach, I've already stated that I'm okay with it, as well as suggested a renaming of venv in that case, modifying pip config in the appropriate manner to deprecate and rename functionality.

#5060 (comment)

[zooba]

suggested a renaming of venv

Did you suggest something to rename it to (which I can't find right now)? Or would you like to?

I personally like the name site, though that may be confusing in the codebase since all the functions for getting global paths are already called site. Otherwise, I like env (just drop the v). Do you have a preference?

(Obviously venv remains as an alias, and I can also add a bit more logic to make sure that it continues to fail if you're not in a virtual environment?)

[pradyunsg]

@zooba I'd suggested ''changing the "venv" to "site" for pip config'' in the comment linked in my previous comment. I think we should, if we make this change (and do cleanups in the "locations" file too, for site vs global clarification, while we're doing that). That said, I do think we should first figure out if this is the change we want to make. :)

make venv actually mean the current environment, regardless of whether it's a venv or not. Which basically means just removing the running_under_virtualenv() check.

@dstufft and @pfmoore didn't seem very warm to this idea when I'd suggested this earlier, in the previously linked comment. I'm not sure how we'd address the concerns raised so I guess inputs from them would be most useful here.

[zooba]

Then I'll implement that change so it's concrete enough to argue about :)

[pfmoore]

My concern was about the fact that this was a request for special-case processing for conda. But as it's so simple to do, I don't really object to the change on a technical basis.

[pradyunsg]

ISTM that there wasn't any existing test to ensure that pip config fails when given multiple "file-options".

Could you add that and change "if not file_options:" to verify the filtered length is 1?

(On mobile)

[zooba]

I added the test.

The if not file_options block already has a check for that - the order of handling there is "no options (use default), one option (use it), else raise". (Personally I prefer to put the error and early-return cases first so that you know you can stop reading there, but I chose not to reorder what was already here.)

[zooba]

This PR is just awaiting maintainer feedback/merge.

[zooba]

Ping @pypa/pip-committers (oh hey, I can annoy all of you at once ;) )

[pradyunsg]

LGTM, except for that one change regarding the deprecation warning. I'll merge once that's fixed. :)

[pradyunsg]

Thanks @zooba for working on this! :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.