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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility with click version 8 #4726

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Conversation

frenzymadness
Copy link
Contributor

In the new version of click, if an option has no type and a default value is of type bool, the default type is bool and the required value for the option is also bool which is not correct for options like --code, --requirements, or --python.

Click 7:

# pipenv install --help | egrep "TEXT|BOOLEAN"
  -c, --code TEXT                 Install packages automatically discovered
  -e, --editable TEXT             An editable Python package URL or path,
  -r, --requirements TEXT         Import a requirements.txt file.
  --extra-index-url TEXT          URLs to the extra PyPI compatible indexes to
  -i, --index TEXT                Target PyPI-compatible package index url.
  --python TEXT                   Specify which version of Python virtualenv
  --pypi-mirror TEXT              Specify a PyPI mirror.

Click 8:

# pipenv install --help | egrep "TEXT|BOOLEAN"
  -c, --code BOOLEAN              Install packages automatically discovered
  -e, --editable TEXT             An editable Python package URL or path,
  -r, --requirements BOOLEAN      Import a requirements.txt file.
  --extra-index-url TEXT          URLs to the extra PyPI compatible indexes to
  -i, --index TEXT                Target PyPI-compatible package index url.
  --python BOOLEAN                Specify which version of Python virtualenv
  --pypi-mirror TEXT              Specify a PyPI mirror.

As you can see above, the new behavior is not a problem for some of the options because --index and --editable have a specific type, --extra-index-url has a string as a default value, etc.

This change should not affect pipenv in any way and should make it more compatible with the new version of click which pipenv surely vendor soon.

Also, explicit is better than implicit 馃槂

@frenzymadness
Copy link
Contributor Author

I think that the failure of the CI is not related to this change. It seems that the vulnerability check in the tests ignores vulnerability with id 350156 but there is another one with id 40291 in pip <21.1 which causes the check to end with return code 1.

There is an open PR with WIP update of pip to the latest version #4606

In the new version of click, if an option has no type and
a default value is of type bool, the required value is also bool.
@frenzymadness
Copy link
Contributor Author

Updated. It seems to be safer to have the default set as an empty string. Otherwise False might be transferred to string and then used which might cause a problem because /usr/bin/false exists for example for --python option.

@frenzymadness
Copy link
Contributor Author

This PR is not complete at all. I'm afraid that there are many more issues than I thought. From what I see now:

@frostming frostming merged commit d10502c into pypa:master Jul 26, 2021
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.

None yet

2 participants