Invalid values in boolean options in the environment cause traceback

[moshez]

Environment

• pip version: 10.0.1
• Python version: 3.6.6
• OS: Debian Unstable

Description

When setting an environment variable that is supposed to hold a boolean value to an invalid string that is not empty, a traceback happens.

Expected behavior

An error message along the lines of "'blah' is not a valid value for ..., valid values

How to Reproduce

1. pip install pip==10.0.1
2. Run env PIP_REQUIRE_VIRTUALENV='blah' PYTHONPATH=src python -m pip install twisted
3. Observe a traceback

Output

$ env PIP_REQUIRE_VIRTUALENV='blah' PYTHONPATH=src python -m pip install twisted                                                       
Traceback (most recent call last):                                             
  File "/home/moshez/.pyenv/versions/3.6.4/lib/python3.6/runpy.py", line 193, in
 _run_module_as_main                                                           
...
    values = self.get_default_values()
  File "/home/moshez/src/pip/src/pip/_internal/baseparser.py", line 230, in get_default_values
    defaults = self._update_defaults(self.defaults.copy())  # ours             
  File "/home/moshez/src/pip/src/pip/_internal/baseparser.py", line 195, in _update_defaults
    val = strtobool(val)
  File "/home/moshez/.pyenv/versions/3.6.4/lib/python3.6/distutils/util.py", line 317, in strtobool
    raise ValueError("invalid truth value %r" % (val,))                        
ValueError: invalid truth value 'blah'

Note

The documentation of strtobool (seen in the traceback above) says:

"True values are y, yes, t, true, on and 1; false values are n, no, f, false, off and 0. Raises ValueError if val is anything else."

[pradyunsg]

Definitely a place where we can print a better error message. :)

[pradyunsg]

This issue is a good starting point for anyone who wants to help out with pip's development -- it's simple and the process of fixing this should be a good introduction to pip's development workflow. See the discussion above to understand what the desired fix is.

Feel free to mention me (write @pradyunsg in the comment) if you need help. :)

[sinscary]

@pradyunsg can I take this up?

[pradyunsg]

Sure. :)

[pradyunsg]

@sinscary How's it going? Did you get some time to look into this?

[sinscary]

@pradyunsg Sorry, actually I was kinda busy. Will try to finish it by today.

[sinscary]

@pradyunsg what will be an appropriate error message in this case. What I can think of is -
"blah is not a valid value for require-virtualenv".

[pradyunsg]

Yeah. And then have it use the default value that it would.

store_true's default is False, counts default is 0 and so on.

[sinscary]

@pradyunsg So shouldn't it be exiting the process if ValueError occurs?
What I understood from your suggestion is, just print the error message and then assign default values to store_true, store_false and count and move on with the process.

[pradyunsg]

Yeah... That's what I was suggesting.

Actually what you're suggesting also makes sense; pip should error out if the configuration provided to it is incorrect.

[pradyunsg]

Yeah... That's what I was suggesting.

Actually what you're suggesting also makes sense; pip should error out if the configuration provided to it is incorrect.

[sinscary]

I think if pip doesn't error out, it might be confusing for many users(beginners mostly) as they won't be getting the desired behavior as per the configuration they have provided. So I think it will be better to exit the process with an error message rather than setting the default values. what say?

Anyways I am almost done and will raise a PR soon. 😄

[pfmoore]

I agree, I'd error out if the user gives an incorrect value.

[pradyunsg]

Erroring out sounds good to me too. :)

[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.