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

Vendored rich kind of depends on attrs #12612

Open
1 task done
thatch opened this issue Apr 3, 2024 · 2 comments
Open
1 task done

Vendored rich kind of depends on attrs #12612

thatch opened this issue Apr 3, 2024 · 2 comments
Labels
project: vendored dependency Related to a vendored dependency type: security Has potential security implications

Comments

@thatch
Copy link

thatch commented Apr 3, 2024

Description

Observed while poetry appears to be running pip commands in parallel:

pip install attrs
pip uninstall importlib-metadata # I don't think the package here matters

Only ImportError is caught here in vendored rich which means that if attrs exists in some sort of half-importable form, it might generate other exceptions like AttributeError.

This particular case was solved by retrying the build, but appears to have:

good:
attr/__init__.py
attr/converters.py
attr/_make.py

bad (empty?)
attr/setters.py

I'm mainly documenting that this exists, more than asking for it to be fixed (which I think would require making edits to the vendored code). I'm a little interested in whether pip is expected to be safe run concurrently on projects that it doesn't claim are deps.

Expected behavior

No response

pip version

24.0

Python version

3.9

OS

linux

How to Reproduce

  1. pip install attrs
  2. truncate -s 0 $(python -c 'import attrs.setters; print(attrs.setters.__file__))
  3. pip install foo

Output

No response

Code of Conduct

@thatch thatch added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Apr 3, 2024
@uranusjr
Copy link
Member

uranusjr commented Apr 8, 2024

I think we have a somewhat similar case with requests, where we (basically) just patch things out to always say the module does not exist. This part does not seem to be practically problematic though since pip should never pass in an attrs object.

@uranusjr uranusjr added type: security Has potential security implications project: vendored dependency Related to a vendored dependency and removed type: bug A confirmed bug or unintended behavior S: needs triage Issues/PRs that need to be triaged labels Apr 8, 2024
@pfmoore
Copy link
Member

pfmoore commented Apr 8, 2024

I'm not particularly worried about this. I would say that we only expect pip to be usable on environments that are not broken (so a non-importable attrs is not a case we would support), and we don't support concurrent use of pip (we don't protect against changes being made to packages in the environment that we are relying on, while we're running).

If poetry is running pip concurrently on the same environment, dealing with race conditions is something they will need to handle themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: vendored dependency Related to a vendored dependency type: security Has potential security implications
Projects
None yet
Development

No branches or pull requests

3 participants