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

Pants tests induce PEXWarning from resolve package collisions #20586

Open
cburroughs opened this issue Feb 20, 2024 · 5 comments
Open

Pants tests induce PEXWarning from resolve package collisions #20586

cburroughs opened this issue Feb 20, 2024 · 5 comments
Labels
backend: Python Python backend-related issues bug

Comments

@cburroughs
Copy link
Contributor

Describe the bug
Several examples along the lines of:

$ pants test src/python/pants/backend/python/packaging::
16:38:01.66 [INFO] Canceled: Building 8 requirements for requirements.pex from the 3rdparty/python/user_reqs.lock resolve: PyYAML<7.0,>=6.0, ansicolors==1.1.8, packaging==21.3, pytest<7.1.0,>=6.2.4, toml==0.10.2, types-PyYAML==6.... (49 characters truncated)
16:38:01.66 [INFO] Canceled: Building 8 requirements for requirements.pex from the 3rdparty/python/user_reqs.lock resolve: PyYAML<7.0,>=6.0, ansicolors==1.1.8, packaging==21.3, pytest<7.1.0,>=6.2.4, toml==0.10.2, types-PyYAML==6.... (49 characters truncated)
16:38:06.59 [INFO] Completed: Building 8 requirements for requirements.pex from the 3rdparty/python/user_reqs.lock resolve: PyYAML<7.0,>=6.0, ansicolors==1.1.8, packaging==21.3, pytest<7.1.0,>=6.2.4, toml==0.10.2, types-PyYAML==6.... (49 characters truncated)
16:38:07.68 [INFO] Completed: Building 18 requirements for requirements.pex from the 3rdparty/python/user_reqs.lock resolve: PyYAML<7.0,>=6.0, ansicolors==1.1.8, chevron==0.14.0, fasteners==0.16.3, ijson==3.1.4, node-semver==0.9.0... (247 characters truncated)
16:38:22.19 [INFO] Completed: Building 8 requirements for pytest.pex from the 3rdparty/python/pytest.lock resolve: ipdb, pygments, pytest-asyncio, pytest-cov!=2.12.1,<3.1,>=2.12, pytest-html, pytest-icdiff, pytest-xdist<3,>=2.5, p... (12 characters truncated)
16:38:23.49 [INFO] Completed: Building local_dists.pex
16:38:26.06 [INFO] Completed: Building pytest_runner.pex
16:38:26.06 [INFO] Completed: Building pytest_runner.pex
16:38:26.06 [WARN] /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/7309da7faa3a99f960f878ee917815b3ceb40d69205b7d496d88faa5a8c4db76/pex-2.2.1-py2.py3-none-any.whl/pex/venv/installer.py:241: PEXWarning: Encountered collision populating /home/ecsb/.cache/pants/named_caches/pex_root/venvs/s/ffd73ab4/venv from PEX at pytest_runner.pex:
1. /home/ecsb/.cache/pants/named_caches/pex_root/venvs/d065c874c21c7a9cb6712591555c1b3a1475c9a6/36cca69703bf5917ad2fd12b1106f113b4efab88.lck.work/lib/python3.9/site-packages/typing_extensions.py was provided by:
	sha1:cfffc86f5e733ca91918bf905055735e159e200b -> /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/aa26eb5b539c4e40fcb8a6f653c831d5a28c3443ba97da65ecb041d895c94382/typing_extensions-4.5.0-py3-none-any.whl/typing_extensions.py
	sha1:71f254a5a97d97ba0cd5a3ff1e73cbb8f0b27166 -> /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/25642c956049920a5aa49edcdd6ab1e06d7e5d467fc00e0506c44ac86fbfca02/typing_extensions-4.3.0-py3-none-any.whl/typing_extensions.py
  pex_warnings.warn(message)

16:38:26.06 [WARN] /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/7309da7faa3a99f960f878ee917815b3ceb40d69205b7d496d88faa5a8c4db76/pex-2.2.1-py2.py3-none-any.whl/pex/venv/installer.py:241: PEXWarning: Encountered collision populating /home/ecsb/.cache/pants/named_caches/pex_root/venvs/s/52ab0681/venv from PEX at pytest_runner.pex:
1. /home/ecsb/.cache/pants/named_caches/pex_root/venvs/d065c874c21c7a9cb6712591555c1b3a1475c9a6/1ac9e793bdab269adcc386c5e35a6e179b109ade.lck.work/lib/python3.9/site-packages/typing_extensions.py was provided by:
	sha1:cfffc86f5e733ca91918bf905055735e159e200b -> /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/aa26eb5b539c4e40fcb8a6f653c831d5a28c3443ba97da65ecb041d895c94382/typing_extensions-4.5.0-py3-none-any.whl/typing_extensions.py
	sha1:71f254a5a97d97ba0cd5a3ff1e73cbb8f0b27166 -> /home/ecsb/.cache/pants/named_caches/pex_root/installed_wheels/25642c956049920a5aa49edcdd6ab1e06d7e5d467fc00e0506c44ac86fbfca02/typing_extensions-4.3.0-py3-none-any.whl/typing_extensions.py
  pex_warnings.warn(message)

That is we have one version of typing_extensions in pytest.lock and a different version in (for example) user_reqs.lock. "Normally" lockfiles are disjoint, but in the case of tools they are sometimes munged together.

I'm not sure exactly what -- if anything -- should be done here. Or rather what the recommendation should be for Pants users in the same situation short of #9206

Pants version
2d1ffe9

OS
Are you encountering the bug on MacOS, Linux, or both?

Additional info
See also #20577

@cburroughs cburroughs added the bug label Feb 20, 2024
@huonw huonw added the backend: Python Python backend-related issues label Feb 21, 2024
@huonw huonw added this to the 2.20.x milestone Feb 21, 2024
@huonw
Copy link
Contributor

huonw commented Feb 21, 2024

Good catch!

I would think it'd be good not to ship this and #20577 in a stable release, potentially we should have the new option default to False for 2.20.x (and adjust the docs to indicate that there's a few "spurious" issues)?

This would relieve the pressure of needing to land potentially-broad changes to fix the underlying issues (which we presumably should fix) to unblock 2.20.0.

Thoughts?

@huonw
Copy link
Contributor

huonw commented Feb 21, 2024

(Chucked up #20590 with that idea, in case we want to go with it.)

@cburroughs
Copy link
Contributor Author

A default of False in 2.20 and then True in 2.21 to de-risk 2.20 makes sense to me.

Pants is already pretty chatty so I do lean towards True in 2.21 even if every single case isn't squashed. This issue with typing_extensions is "probably fine" and could trivially be worked around by futzing with the pants internal lockfiles, but the same warning could be telling a user something closer "Your code is broken!" and I don't like swallowing the underlying tool trying to tell them that.

@cburroughs
Copy link
Contributor Author

Although now that I've thought through that a little more, we probably should keep it enabled for Pants itself (in pants.toml) so we keep surfacing new cases as dogfood.

huonw added a commit that referenced this issue Feb 22, 2024
This is a short-term workaround for #20577 and #20586, where Pants'
internal/default use of Pex triggers user-visible warnings, and those
warnings are now visible due to #20480... so, instead of showing them by
default, let's hide them for now. Users with desire for insight into the
warnings can still enable this.

Doing this means we're not having to rush in fixes for the root causes
of these warnings for 2.20.0 stable release, and thus reduce
feature-creep/risk. We can/should reenable warnings by default in a
future release.

Per @cburroughs's idea in
#20586 (comment),
this explicitly switches it on for the Pants repo itself, so we're
eating our own dogfood and catching real and/or spurious errors earlier.
WorkerPants pushed a commit that referenced this issue Feb 22, 2024
This is a short-term workaround for #20577 and #20586, where Pants'
internal/default use of Pex triggers user-visible warnings, and those
warnings are now visible due to #20480... so, instead of showing them by
default, let's hide them for now. Users with desire for insight into the
warnings can still enable this.

Doing this means we're not having to rush in fixes for the root causes
of these warnings for 2.20.0 stable release, and thus reduce
feature-creep/risk. We can/should reenable warnings by default in a
future release.

Per @cburroughs's idea in
#20586 (comment),
this explicitly switches it on for the Pants repo itself, so we're
eating our own dogfood and catching real and/or spurious errors earlier.
huonw added a commit that referenced this issue Feb 22, 2024
…#20593)

This is a short-term workaround for #20577 and #20586, where Pants'
internal/default use of Pex triggers user-visible warnings, and those
warnings are now visible due to #20480... so, instead of showing them by
default, let's hide them for now. Users with desire for insight into the
warnings can still enable this.

Doing this means we're not having to rush in fixes for the root causes
of these warnings for 2.20.0 stable release, and thus reduce
feature-creep/risk. We can/should reenable warnings by default in a
future release.

Per @cburroughs's idea in
#20586 (comment),
this explicitly switches it on for the Pants repo itself, so we're
eating our own dogfood and catching real and/or spurious errors earlier.

Co-authored-by: Huon Wilson <huon@exoflare.io>
@huonw
Copy link
Contributor

huonw commented Feb 22, 2024

Removing this from the 2.20.x milestone, since we landed #20593 as a stop-gap to reduce severity in 2.20.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Development

No branches or pull requests

2 participants