-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added a virtualenv-specific configuration file #1364
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
import textwrap | ||
from distutils.util import strtobool | ||
from pip.backwardcompat import ConfigParser, string_types | ||
from pip.locations import default_config_file | ||
from pip.locations import default_config_file, default_config_basename, running_under_virtualenv | ||
from pip.util import get_terminal_size, get_prog | ||
|
||
|
||
|
@@ -136,8 +136,14 @@ def __init__(self, *args, **kwargs): | |
def get_config_files(self): | ||
config_file = os.environ.get('PIP_CONFIG_FILE', False) | ||
if config_file and os.path.exists(config_file): | ||
return [config_file] | ||
return [default_config_file] | ||
files = [config_file] | ||
else: | ||
files = [default_config_file] | ||
if running_under_virtualenv(): | ||
venv_config_file = os.path.join(sys.prefix, default_config_basename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no If it should go in /etc on Unix I could do that - but I wouldn't want to for Windows. Also, should virtualenv create an etc directory? Or should the user be required to create the directory in order to add the per-venv file (and know that they need to do that on Unix but not Windows)? Personally I think it's more compexity than the feature warrants, but I can do it if the Unix people think it's worth it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I didn't know the core venv does that, interesting. Yeah, for the sake of simplicity I agree There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thedrow I'm going to stick with
|
||
if os.path.exists(venv_config_file): | ||
files.append(venv_config_file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so previously, pip has never exercised the multi-file support of ConfigParser. not sure how the layering works with that? are you comfortable with that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did check it - it does what I'd expect (later files take precedence over earlier ones where section/key names clash). Yes, I'm comfortable with it. |
||
return files | ||
|
||
def check_default(self, option, key, val): | ||
try: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -226,3 +226,17 @@ def test_cert(self): | |
options1, args1 = main(['--cert', 'path', 'fake']) | ||
options2, args2 = main(['fake', '--cert', 'path']) | ||
assert options1.cert == options2.cert == 'path' | ||
|
||
|
||
class TestOptionsConfigFiles(object): | ||
|
||
def test_general_option_after_subcommand(self, monkeypatch): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about relabel this test name. "test_venv_config_file_found" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, cut and paste typo. Will fix. |
||
# We only want a dummy object to call the get_config_files method | ||
monkeypatch.setattr(pip.baseparser.ConfigOptionParser, '__init__', lambda self: None) | ||
|
||
# If we are running in a virtualenv and all files appear to exist, | ||
# we should see two config files. | ||
monkeypatch.setattr(pip.baseparser, 'running_under_virtualenv', lambda: True) | ||
monkeypatch.setattr(os.path, 'exists', lambda filename: True) | ||
cp = pip.baseparser.ConfigOptionParser() | ||
assert len(cp.get_config_files()) == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and precedence over a config file specified using PIP_CONFIG_FILE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was seeing PIP_CONFIG_FILE as just changing the location if "the user configuration file", but I'll see if I can word this more clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, it seems to me we should have a real option for
--config-file
. PIP_CONFIG_FILE is the only odd ball case where we have an environment option without a comand option.