Skip to content
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

Fixed 'pip install --user catkin_tools' #488

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

racko
Copy link
Contributor

@racko racko commented Oct 13, 2017

pip passes --user --prefix= when the user calls pip install --user,
which conflicted with the mutually_exclusive_group being used to parse
these arguments.

The mutually_exclusive_group is not necessary since pip prevents the
user from passing --user together with --prefix anyhow. By not
checking it here again, we can accept pip passing --user --prefix= to
us.


Example:

$ cd catkin_tools
$ pip2 install --user .
[...]
  Running setup.py install for catkin-tools ... error
    Complete output from command /usr/bin/python2 -u -c "import setuptools, tokenize;__file__='/tmp/pip-NhWIKe-build/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-fwXLCn-record/install-record.txt --single-version-externally-managed --compile --user --prefix=:
    usage: -c [--user | --prefix PREFIX]
    -c: error: argument --prefix: not allowed with argument --user/--home
    
    ----------------------------------------
Command "/usr/bin/python2 -u -c "import setuptools, tokenize;__file__='/tmp/pip-NhWIKe-build/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-fwXLCn-record/install-record.txt --single-version-externally-managed --compile --user --prefix=" failed with error code 2 in /tmp/pip-NhWIKe-build/

The pip code that causes this:
https://github.com/pypa/pip/blob/ff5b2013a0a9001a457034481ed918e1a3fea7ef/src/pip/_internal/commands/install.py#L195-L207

pip passes `--user --prefix=` when the user calls `pip install --user`,
which conflicted with the mutually_exclusive_group being used to parse
these arguments.

The mutually_exclusive_group is not necessary since pip prevents the
user from passing `--user` together with `--prefix` anyhow. By not
checking it here again, we can accept pip passing `--user --prefix=` to
us.
@mikepurvis
Copy link
Member

Looks safe to me, but I guess the original check was to protect in the case of non-pip usage, eg python setup.py install --prefix path/to/usr. I don't have a sense what's at stake here, but it would be nice not to block a potentially helpful behaviour from pip.

@wjwwood Any concerns here?

@wjwwood
Copy link
Member

wjwwood commented Mar 12, 2018

@wjwwood Any concerns here?

No, I just haven't had time to test it myself. Glancing at it now, it seems reasonable, but I haven't tested it.

@mikepurvis
Copy link
Member

I confirm from local testing the error seen and that this PR corrects it and makes pip install --user work as expected with catkin_tools. Thanks for the contribution, @racko!

@mikepurvis mikepurvis merged commit 3ead0cb into catkin:master Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants