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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Expand Up @@ -35,3 +35,4 @@ pip-log.txt
# Coverage artifacts
.coverage
coverage.xml
pip-wheel-metadata
8 changes: 8 additions & 0 deletions pyproject.toml
View file
Open in desktop
@@ -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.

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.

"setuptools_scm >= 1.15.0",
"setuptools_scm_git_archive >= 1.0",
"wheel",
]
build-backend = "setuptools.build_meta"