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

Expand the range of supported setuptools. #541

Merged
merged 3 commits into from
Aug 21, 2018

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Aug 18, 2018

We now just exclude the island where setuptools ran a disruptive
de-vendoring experiment.

We now just exclude the island where setuptools ran a disruptive
de-vendoring experiment.
@jsirois
Copy link
Member Author

jsirois commented Aug 18, 2018

I manually tested pex --pre with setuptools versions inside and outside the excluded range, only getting failures in the excluded range. I used https://github.com/pypa/setuptools/blob/master/CHANGES.rst to find the exclusion range in the first place.

The motivation here is to let Pants use a newer version of setuptools so that it can run under python 3.7. See: pantsbuild/pants#6363

@jsirois jsirois requested review from kwlzn and stuhood August 18, 2018 18:31
Copy link
Contributor

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

pex/version.py Outdated
# for pex code so we exclude that range.
__exclusions = '!=34.*,!=35.*'
SETUPTOOLS_REQUIREMENT = 'setuptools>=20.3,<41,{exclusions}'.format(exclusions=__exclusions)
del __exclusions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the use and explicit del of __exclusions struck me as odd here. any reason to not keep the exclusions around as a constant (like SETUPTOOLS_EXCLUSIONS) - or just elide and hardcode in the string literal?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - I'll inline. This was half remembering wickmanisms and half wishing for assignment blocks:

x = block:
  foo
  # NB: bar is very very worth exposition
  bar

  'baz'

assert x == 'baz'

@jsirois jsirois merged commit f125317 into pex-tool:master Aug 21, 2018
@jsirois jsirois deleted the setuptools/bump_ceiling branch August 21, 2018 20:47
@jsirois jsirois mentioned this pull request Aug 24, 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.

2 participants