Allow for use_user_site being set to an integer

[takluyver]

This can come from tox config, as described in a comment:
#7002 (comment)

This PR implements @pradyunsg's suggested fix. The alternatives are to delve into how options & config are handled to fix it there, or to get it fixed in tox and hope no-one else hits it.

The # noqa: E712 markers are required to stop flake8 from complaining that the comparisons should use is, as normally they should.

[takluyver]

We need to distinguish false from none, but we could still change the true condition if you prefer.

…

On Thu, 24 Oct 2019, 17:15 Christopher Hunt, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/pip/_internal/commands/install.py <#7258 (comment)>: > @@ -616,11 +616,13 @@ def decide_user_install( If use_user_site is None, the default behaviour depends on the environment, which is provided by the other arguments. """ - if use_user_site is False: + # In some cases (config from tox), use_user_site can be set to an integer + # rather than a bool. Comparing == True/False instead of 'is' allows this. + if use_user_site == False: # noqa: E712 I would use not use_user_site and drop the noqa and comment. ------------------------------ In src/pip/_internal/commands/install.py <#7258 (comment)>: > logger.debug("Non-user install by explicit request") return False - if use_user_site is True: + if use_user_site == True: # noqa: E712 Similar to the above, I would use use_user_site and drop the noqa. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7258?email_source=notifications&email_token=AACQB5MGSN6YEP2UTBF6KSTQQHC33A5CNFSM4JESNLAKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJD5L7I#pullrequestreview-306697725>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQB5MCAATJJI32RF6CVW3QQHC33ANCNFSM4JESNLAA> .

[pfmoore]

How about if user_site is not None and not user_site:? It's more verbose, but says what it means (the value is set, and it's not true) and avoids the code smell of equality-testing booleans.

[pradyunsg]

@pfmoore's suggestion as well as this PR as is, both work for me. :)

[takluyver]

That makes sense. I've pushed a new commit.

[atugushev]

Also it can be reproduced outside of tox. Add user = true to pip.conf and you'll get the same error on pip install six. That happens because ConfigOptionParser uses distutils.util.strtobool() (wich returns int) when parses values from config file:

pip/src/pip/_internal/cli/parser.py

Line 201 in c729a84

val = strtobool(val)

distutils.util:

def strtobool (val):
    """Convert a string representation of truth to true (1) or false (0).

    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.
    """
    val = val.lower()
    if val in ('y', 'yes', 't', 'true', 'on', '1'):
        return 1
    elif val in ('n', 'no', 'f', 'false', 'off', '0'):
        return 0
    else:
        raise ValueError("invalid truth value %r" % (val,))

It could be worth to comment the case with config file.

[atugushev]

Would be nice to have a test fot that case, btw.

[atugushev]

Like this one, but install:

pip/tests/functional/test_wheel.py

Lines 221 to 229 in c729a84

	def test_pip_wheel_with_user_set_in_config(script, data, common_wheels):
	config_file = script.scratch_path / 'pip.conf'
	script.environ['PIP_CONFIG_FILE'] = str(config_file)
	config_file.write_text("[install]\nuser = true")
	result = script.pip(
	'wheel', data.src / 'withpyproject',
	'--no-index', '-f', common_wheels
	)
	assert "Successfully built withpyproject" in result.stdout, result.stdout

[takluyver]

Thanks, copied part of that and added a test.

[pradyunsg]

Thanks y'all! :)