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.

Sign up for GitHub

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

Jump to bottom

Avoid installation failure with newer setuptools #591

Merged
merged 1 commit into from
Oct 22, 2019
Merged

Avoid installation failure with newer setuptools #591

merged 1 commit into from
Oct 22, 2019

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Oct 16, 2019 •
edited
Loading

Implement workaround for avoiding installation errors on system with newer setuptools

AttributeError: 'SpecifierSet' object has no attribute 'split'

Fixes: #590

@ssbarnea
Avoid installation failure with newer setuptools
0680cdf 
Implement workaround for avoiding installation errors on system with
newer setuptools

AttributeError: 'SpecifierSet' object has no attribute 'split'

Longer term we will have to refactor setup.py which is the root
of all evil.

Fixes: #590
Signed-off-by: Sorin Sbarnea <ssbarnea@redhat.com>
@ssbarnea ssbarnea requested a review from webknjaz as a code owner October 16, 2019 13:18
webknjaz
webknjaz suggested changes Oct 16, 2019
View reviewed changes
pyproject.toml
@@ -0,0 +1,8 @@
[build-system]
requires = [
"setuptools == 41.0.0", # See https://github.com/pypa/setuptools/issues/1869
Copy link
Member

Choose a reason for hiding this comment

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

plz don't pin old setuptools. Patch setup.py instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This does not pin setuptools inside the venv, is only the version used for building from source. It would not affect the setuptools version on the system. See:

ssbarnea@imac: ~/os/ansible-lint py3 fix/setuptools-workaround ⚡
$ pip freeze --all                                                                                                                                                        [14:46:39]
pip==19.3
setuptools==41.4.0
wheel==0.33.6

ssbarnea@imac: ~/os/ansible-lint py3 fix/setuptools-workaround ⚡
$ pip install .                                                                                                                                                           [14:46:50]
Processing /Users/ssbarnea/os/ansible-lint
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Processing /Users/ssbarnea/Library/Caches/pip/wheels/89/40/f0/6f51c0707ef61a8459893eb5e016ff4ded415dd47a4f74608e/ansible-2.8.5-cp37-none-any.whl
Collecting ruamel.yaml<1,>=0.15.37; python_version >= "3.7"
  Using cached https://files.pythonhosted.org/packages/fa/90/ecff85a2e9c497e2fa7142496e10233556b5137db5bd46f3f3b006935ca8/ruamel.yaml-0.16.5-py2.py3-none-any.whl
Processing /Users/ssbarnea/Library/Caches/pip/wheels/d9/45/dd/65f0b38450c47cf7e5312883deb97d065e030c5cca0a365030/PyYAML-5.1.2-cp37-cp37m-macosx_10_15_x86_64.whl
Collecting six
  Using cached https://files.pythonhosted.org/packages/73/fb/00a976f728d0d1fecfe898238ce23f502a721c0ac0ecfedb80e0d88c64e9/six-1.12.0-py2.py3-none-any.whl
Collecting cryptography
  Using cached https://files.pythonhosted.org/packages/63/4e/57b7a6bd98906872fcd2531e74b532de2abe17d675a5cf171931fcb4a9e8/cryptography-2.7-cp34-abi3-macosx_10_6_intel.whl
Collecting jinja2
  Using cached https://files.pythonhosted.org/packages/65/e0/eb35e762802015cab1ccee04e8a277b03f1d8e53da3ec3106882ec42558b/Jinja2-2.10.3-py2.py3-none-any.whl
Collecting ruamel.yaml.clib>=0.1.2; platform_python_implementation == "CPython" and python_version < "3.8"
  Using cached https://files.pythonhosted.org/packages/fa/16/0ff4e92c21f275c071acb68f8ddd55b287e4e3b5dfd14c97983cb3506c90/ruamel.yaml.clib-0.2.0-cp37-cp37m-macosx_10_9_x86_64.whl
Collecting asn1crypto>=0.21.0
  Using cached https://files.pythonhosted.org/packages/6e/1e/fb0e487b5229e5fb7b15c6d00b4e8082a3414fe62b1da4c9a905b106e672/asn1crypto-1.1.0-py2.py3-none-any.whl
Collecting cffi!=1.11.3,>=1.8
  Using cached https://files.pythonhosted.org/packages/df/eb/79652fd053b610bda97b4e51665467a9ef7f51e0497d2d38ff1ab5272b58/cffi-1.13.0-cp37-cp37m-macosx_10_6_intel.whl
Collecting MarkupSafe>=0.23
  Using cached https://files.pythonhosted.org/packages/ce/c6/f000f1af136ef74e4a95e33785921c73595c5390403f102e9b231b065b7a/MarkupSafe-1.1.1-cp37-cp37m-macosx_10_6_intel.whl
Processing /Users/ssbarnea/Library/Caches/pip/wheels/f2/9a/90/de94f8556265ddc9d9c8b271b0f63e57b26fb1d67a45564511/pycparser-2.19-py2.py3-none-any.whl
Building wheels for collected packages: ansible-lint
  Building wheel for ansible-lint (PEP 517) ... done
  Created wheel for ansible-lint: filename=ansible_lint-4.1.1rc2.dev3+g0680cdf-cp37-none-any.whl size=55705 sha256=c9ad0269b1756ecb68abeee9d5a478b961cc34f4d96f61d208092f7c7026c856
  Stored in directory: /Users/ssbarnea/Library/Caches/pip/wheels/86/05/80/6f4171540802417b38203a12cb43bd7ee4c930793e4f96b49e
Successfully built ansible-lint
Installing collected packages: asn1crypto, six, pycparser, cffi, cryptography, pyyaml, MarkupSafe, jinja2, ansible, ruamel.yaml.clib, ruamel.yaml, ansible-lint
Successfully installed MarkupSafe-1.1.1 ansible-2.8.5 ansible-lint-4.1.1rc2.dev3+g0680cdf asn1crypto-1.1.0 cffi-1.13.0 cryptography-2.7 jinja2-2.10.3 pycparser-2.19 pyyaml-5.1.2 ruamel.yaml-0.16.5 ruamel.yaml.clib-0.2.0 six-1.12.0
noglob pip install .  9.46s user 6.18s system 59% cpu 26.244 total
Time: 0h:00m:26s

ssbarnea@imac: ~/os/ansible-lint py3 fix/setuptools-workaround ⚡
$ pip freeze --all                                                                                                                                                        [14:47:25]
ansible==2.8.5
ansible-lint==4.1.1rc2.dev3+g0680cdf
asn1crypto==1.1.0
cffi==1.13.0
cryptography==2.7
Jinja2==2.10.3
MarkupSafe==1.1.1
pip==19.3
pycparser==2.19
PyYAML==5.1.2
ruamel.yaml==0.16.5
ruamel.yaml.clib==0.2.0
setuptools==41.4.0
six==1.12.0
wheel==0.33.6

As you can see above, the virtualenv still has the latest setuptools, the one that caused the breakage. Build config is only about how-to-build the wheel, not how to install it.

Re proposal to fix setup.py,.... @webknjaz you are welcomed to fix it yourself, I did not write that code and I looked at it and failed to identify a way to fix it, especially quickly.

Copy link

Choose a reason for hiding this comment

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

thanks zbr for working on that

we need this + the fix at #590 (or does that one also fix this sorin?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we make a pragmatic decision and merge this and debate later about what older versions we need to cover for? We can delay a release based on these concerns but not a merge that is fixing broken CI.

We can later identify which are those cases and add tests for them. As of today, Ansible documentation does not mention any minimal or maximal version of pip, on the opposite it does mention that you may need to upgrade pip. This implies that latest version of pip should work.

Copy link
Member

Choose a reason for hiding this comment

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

This is reasonable. You'll be able to use commit SHA in your pre-commit config.

webknjaz
webknjaz reviewed Oct 16, 2019
View reviewed changes
pyproject.toml
@@ -0,0 +1,8 @@
[build-system]
Copy link
Member

Choose a reason for hiding this comment

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

btw, by adding this you effectively break editable mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

I defer, I tested both pip install . and pip install -e . and they both worked fine (py37/macos). Did you tried them and failed? If so, which environment?

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that PEP517 has no notion of "editable install" atm.

Copy link
Member

Choose a reason for hiding this comment

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

Also, which version of Pip?

Copy link
Member Author

Choose a reason for hiding this comment

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

pip==19.3

Copy link
Member Author

Choose a reason for hiding this comment

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

@webknjaz Also check #592 which is supposed to fix the setup.py -- I do not care which one goes in as long we fix ansible-lint itself. It is a huge pain to introduce workarounds everywhere is used.

Copy link
Member

Choose a reason for hiding this comment

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

Both break things, hence not ready for merge.

Copy link
Member Author

Choose a reason for hiding this comment

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

Both break things, hence not ready for merge.

That sentence is a bit blurry and clearly not actionable. What use-case is broken? What needs to be done to bring it to ready-to-merge status?

Copy link
Member

Choose a reason for hiding this comment

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

PEP517 ignores the existence of editable installs making Pip behavior unpredictable. I know of at least one Pip version that errors out under such circumstances.

Copy link
Member

Choose a reason for hiding this comment

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

The action here is to wait for the decision whether we can drop support for older envs.

@ssbarnea ssbarnea requested a review from webknjaz October 16, 2019 14:37
@ssbarnea ssbarnea mentioned this pull request Oct 22, 2019
Closed
@webknjaz webknjaz merged commit 7a2221a into ansible:master Oct 22, 2019
@webknjaz
Copy link
Member

Thanks, @ssbarnea!

@felddy felddy mentioned this pull request Oct 25, 2019
Merged
@ssbarnea ssbarnea deleted the fix/setuptools-workaround branch July 25, 2020 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marios marios marios left review comments

@bristea bristea bristea approved these changes

@webknjaz webknjaz Awaiting requested review from webknjaz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ansible-lint fails to install from source: AttributeError: 'SpecifierSet' object has no attribute 'split'
4 participants
@ssbarnea @webknjaz @marios @bristea