-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Dependencies of Git Dependencies are not always installed #9066
Comments
I guess the setup reader spots In most cases where setup.py builds up requirements via a series of conditional tests, this mistake doesn't really matter: in the sense that poetry only supports projects where the metadata is the same for all distributions, so a setup that goes "if windows this, if linux that" is fundamentally not compatible with poetry. In this case the code is providing a static list, but doing it in a rather elaborate way. if what you care about is getting this working with kubernetes-client then probably your best bet is to go and offer them a pull request to rearrange things so that poetry doesn't get fooled. eg with open('requirements.txt') as f:
REQUIRES = [line.split(" #")[0] for line in f.readlines()] seems like a simplification they might want anyway, and should be something that poetry recognises it cannot understand |
While that might be the case (and I'm already working off a fork, so I can just do it there), I don't think its too unreasonable to expect poetry to behave "close enough" to "pip install", and not do something completely different, silently. I'll refer you to my comment on the previous issue regarding this same bug: Heuristics are great, right up until they are wrong. I stand by my position that poetry ought to have some mechanism to say "Your guess is wrong, please build it.". I'd be happy to contribute something like this if it would be accepted. |
I'm not a fan of "make it work" flags: if this is going to be fixed, let it be fixed without a config option. I guess it's possible to go in either direction:
I would give serious consideration to removing the setup reader, though I don't know how maintainers would feel about that The python ecosystem is quite different than it was when this code was written, most popular projects now have wheels declaring static metadata. So the trade between "time saved by not building a wheel" and "sometimes being wrong" is in a different place than it was. |
Setup reader was always considered a fallback. I thought we have had pep 517 build before falling back to setup reader. Or I might have incorrectly believed that since an empty list was previously being treated as a bad read and we fell back to a 517 metadata build. I'm curious if we are just way too optimistic about setup reader's parsed list. |
setup reader happens before build and has done for a long time, probably since forever |
If you don't like "make it work" options, and while this might be considered a breaking change, what about needing to opt into the setupreader for path and git dependencies? It seems like it'd be much safer to do the slow, but reliable build by default unless the user explicitly gives the go-ahead for the faster, but riskier option. |
That is still a "make it work" flag. IMO any config option here is a mistake |
To be concrete: the intermediate "keep the setup reader but make it less ambitious" middle option probably looks something like this: diff --git a/src/poetry/utils/setup_reader.py b/src/poetry/utils/setup_reader.py
index 3c233cd9..a348e3b3 100644
--- a/src/poetry/utils/setup_reader.py
+++ b/src/poetry/utils/setup_reader.py
@@ -222,9 +222,6 @@ class SetupReader:
if value is None:
return []
- if isinstance(value, ast.Name):
- value = self._find_variable_in_body(body, value.id)
-
if isinstance(value, ast.Constant) and value.value is None:
return []
@@ -261,9 +258,6 @@ class SetupReader:
if value is None:
return {}
- if isinstance(value, ast.Name):
- value = self._find_variable_in_body(body, value.id)
-
if isinstance(value, ast.Constant) and value.value is None:
return {} (plus corresponding fixes to testcases) If I cared to make this more reliable, and did not want to try and persuade maintainers that the setup reader should be abandoned altogether, that is the approach I would pursue |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Adding
https://github.com/kubernetes-client/python
as a dependency via git does not install its dependencies. Using pip directly installs dependencies, as does installing the package from pypi.Workarounds
Manually copying dependencies to pyproject.toml
Poetry Installation Method
pipx
Operating System
Debian GNU/Linux 12 (bookworm)
Poetry Version
Poetry (version 1.8.1)
Poetry Configuration
Python Sysconfig
No response
Example pyproject.toml
Poetry Runtime Logs
The text was updated successfully, but these errors were encountered: