-
Notifications
You must be signed in to change notification settings - Fork 27
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
poetry.lock [package.dependencies] markers not being respected #186
Comments
Hi @wjhrdy, thanks for the report. Could you please provide Briefly looking at this, it might be that we are not propagating markers from the dependencies section:
Lines 693 to 694 in 4051992
|
EDIT: I changed the required package from my original post from poetry.lock
pyproject.toml
|
poetry exported requirements.txt
|
here is how pipenv handles it
pipfile.lock (as generated by pipenv)
|
This is the pipfile.lock that
|
/kind bug |
Thanks @wjhrdy for the report and provided content, putting this on my list. In the meantime, if somebody wants to fix this, contributions are welcome. |
I'd like to help with the fix for this but I need some help. @fridex do you want to implement this yourself or mentor me? I almost figured it out, but I need to verify my approach. Using the project mentioned above with the terminado package in the dependencies: From the poetry.lock I cannot tell for sure whether ptyprocess or pywinpty are direct application dependencies or not, so I have to take a look at the pyproject.toml, right? Let's say that ptyprocess is not defined as a dependency directly and poetry.lock contains it just because terminado depends on it. In that case, the main record in pipfile.lock should contain the markers defined in the To be more specific:
and
Produces this
But if I specify for example
and
produces
Where the markers are not moved from terminado definition to pywinpty because we specified pywinpty as a dependency manually. I'm gonna propose my change as WIP PR and we can discuss it. Are there any other corner use-cases I forget about? |
Draft pull request: #188 |
Sure, always happy to help. 👍🏻
Yes, that's correct.
Shouldn't If an environment marker is applied to a dependency, it should not be propagated anyhow - I think it should be applied on the given dependency (regardless if it is direct or indirect dependency). BTW you can double check this behaviour with Pipenv: [[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
[packages]
terminado = "*"
[dev-packages]
[requires]
python_version = "3.8" Pipfile.lock ( {
"_meta": {
"hash": {
"sha256": "ad700aa48ec4a89c24a2b215e61ded207ead3b8a2bf20213b7c3010ca47edb49"
},
"pipfile-spec": 6,
"requires": {
"python_version": "3.8"
},
"sources": [
{
"name": "pypi",
"url": "https://pypi.org/simple",
"verify_ssl": true
}
]
},
"default": {
"ptyprocess": {
"markers": "os_name != 'nt'",
"version": "==0.7.0"
},
"terminado": {
"index": "pypi",
"version": "==0.11.1"
},
"tornado": {
"markers": "python_version >= '3.5'",
"version": "==6.1"
}
},
"develop": {}
} Pipenv also provides a way to specify markers on the direct dependencies: [[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
[packages]
requests = {version="*", markers="os_name == \"nt\""}
[dev-packages]
[requires]
python_version = "3.8" Pipfile.lock: {
"_meta": {
"hash": {
"sha256": "b61e2572e90afe7a935b99229476ecb57daa24dec2df9226028a997363ecc62c"
},
"pipfile-spec": 6,
"requires": {
"python_version": "3.8"
},
"sources": [
{
"name": "pypi",
"url": "https://pypi.org/simple",
"verify_ssl": true
}
]
},
"default": {
"requests": {
"hashes": [
"sha256:6c1246513ecd5ecd4528a0906f910e8f0f9c6b8ec72030dc9fd154dc1a6efd24",
"sha256:b8aa58f8cf793ffd8782d3d8cb19e66ef36f7aba4353eec859e74678b01b07a7"
],
"markers": "os_name == 'nt'",
"version": "==2.26.0"
}
},
"develop": {}
}
|
I'm not sure about the last statement. See this example: Let's say, my application directly depends only on [tool.poetry.dependencies]
python = "^3.7"
yarl = "1.6.3" One of the [[package]]
name = "typing-extensions"
version = "3.10.0.0"
description = "Backported and Experimental Type Hints for Python 3.5+"
category = "main"
optional = false
python-versions = "*"
[[package]]
name = "yarl"
version = "1.6.3"
description = "Yet another URL library"
category = "main"
optional = false
python-versions = ">=3.6"
[package.dependencies]
idna = ">=2.0"
multidict = ">=4.0"
typing-extensions = {version = ">=3.7.4", markers = "python_version < \"3.8\""} And finally, the
This is fine, the marker is there because it should be installed as a dependency of yarl only if the Python version is lower than 3.8. And now my point. Let's say that I (as a maintainer of an application) want to install [tool.poetry.dependencies]
python = "^3.7"
yarl = "1.6.3"
typing-extensions = "3.10.0.0" poetry.lock looks exactly the same, which is kinda surprising for me, but I'm able to respect that the dependency is now direct without any markers in the resulting
I'm not able to reproduce the same process in pipenv because there are bugs in the name normalization and unfortunately, yarl depends on If I depend only on [packages]
werkzeug = "==2.0.1" the marker for the Python version is respected in the lock file:
But if I depend on dataclasses directly, for whatever reason I might have: [packages]
werkzeug = "==2.0.1"
dataclasses = "==0.8" the marker is not there:
And it makes sense to me to implement it this way also in micropipenv even the poetry does not change its locfile when you add/remove the direct dependency in the examples above. Because - using the last example - if the dependency is indirect, it's the responsibility of werkzeug maintainers to specify the correct markers and micropipenv should respect them and transfer them from poetry.lock to pipfile.lock, but if the dependency on dataclasses is direct and specified manually in my configuration file, it's my responsibility to set the correct markers and the markers from werkzeug project should not be taken into account. I'm not really able to reproduce any issue in poetry because it behaves correctly, see this matrix for the same werkzeug example:
After some time with this, I think that the difference in how to handle the markers might depend on Python version we are using - is it the same which the lock file has been prepared for or not? But to mimic the behavior of pipenv described above, we should check whether the dependency is direct or not. |
You are definitely right - the evaluation you described makes perfect sense. I did not realize it and this way we have a bug in Thoth's adviser thoth-station/adviser#2076
It looks like the main difference between Poetry and Pipenv is when the environment markers evaluation is done and what is written to files. If I understand the behaviour you explored above, Pipenv prepares environment markers in advance and writes them to On the other hand, Poetry keeps track of environment markers in raw format in lock files and evaluates them during the installation process - that's how I understand no change in the lock file when you added As the marker evaluation is different in these cases, I think converting poetry files to pipenv.lock will not be that straightforward as the conversion will need to respect what Pipenv does under the hood when generating the lock file. The issue might arise when a dependency is introduced by multiple packages. Let's say we have direct dependencies A and B that both depend on C with the following environment markers:
As Poetry keeps this information in a similar way as written above, there are no issues with the lock file Poetry uses. However, if we want to translate that into a Pipfile.lock in micropipenv, we will need to respect both environment markers (as environment markers need to be joined into one entry in the lock file):
|
Describe the bug
micropipenv
is not handling thesys_platform
marker appropriately and is trying to installpywinpty
andpywin32
on linux.Specifically as a dependencies of the
[email protected]
packageTo Reproduce
Build this Dockerfile
Expected behavior
Respect
sys_platform
andos_name
marker from the poetry.lock file.The expected behavior is working when using poetry exported requirements.txt to install.
The workaround is to export the poetry requirements to requirements.txt and then remove the poetry files. Obviously if we are installing poetry we are defeating the purpose of using micropipenv, so I'd like the solution to not require poetry.
This Dockerfile builds fine with the same dependencies and a
requirements.txt
file.We get this output:
The text was updated successfully, but these errors were encountered: