-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
sys_platform
package specification is behaving differently than the same specification using markers
#5884
Comments
sys_platform
package specification is behaving differently than the same specification using markers
@ChaoticRoman I also observed an erratic behaviour, here: #5883 |
@ChaoticRoman you did not report what pipenv version you are seeing that in. We included a fix for sys_platform markers in |
It is https://github.com/ChaoticRoman/pipenv-bug-demo/actions/runs/5999659742/job/16270232399#step:4:13 |
@ChaoticRoman I guess I am not seeing what you mean about it not working on
|
well, I would expect We had the first way, the one that is simpler and that someone searching for solution would found, and it tried to install the PyGObject on platform where it was not expected to be installed (and whole build failed, so we have found). The second way prevented this. Example https://github.com/ChaoticRoman/pipenv-bug-demo/ serves as a minimal reproducer of this behavior as you can see in clearly different output. |
@ChaoticRoman in my example, which is a clone of |
Sure. The output you have created is replicating the issue as well. From paths like
Check dependencies here: https://deps.dev/pypi/requests/2.31.0/dependencies?filter=relation%3A%22direct%22 Trying that using Switching to So it means that some dependency of |
Interesting -- my patch though for fixing |
@ChaoticRoman Ok I see what you mean, but the result generated when using markers also seems not desirable because locking on the non-platform doesn't resolve the sub-dependencies either -- but this could be an interesting discovery. If resolving the way we do the sys_platform is capturing everything but failing to mark the sub-dependencies as for just that platform, and markers is skipping them all -- maybe its possible to create some hybrid behavior that could capture the resolution of all the dependencies (say I am locking on windows but its only required by linux) but then work in the right platform markers when its only required by a top level Pipfile entry that is sys_platform restricted. I'll have to think more on this. |
@ChaoticRoman I see what is happening, there is logic to actually skip the resolver for markers that don't match, but in the case of sys_platform it gets added to the markers in the generated pip line, rather than part of this check to skip. I think my idea above would make locking a slightly more expensive operation in some cases, but could help solve some of the cross platform locking complaints. Skipping the dependencies and just scraping the top level hash for just that dependency is really also a not so great behavior, so I am hoping there is a path forward that could address both concerns. |
May you elaborate on this? Because of supporting multiple platforms, the project I was fixing was not committing lock files at all. The right way probably would be to have multiple lock files, one for every platform. In any case locking on the state as resolved in the Do I miss something? |
@ChaoticRoman We have not been sold on supporting multiple lock files yet, though some disucssions have taken place. Could you try out what I am thinking with this branch: #5892 Basically what it does is if you have a sys_platform restriction in the Pipfile, it still resolves everything creating a valid lock file, but the sys_platform markers are applied to all dependencies that come from that top level Pipfile entry (in theory, I just worked this out in the past hour).
Its problematic because it skips resolving those dependencies when locked on a different platform, and leads to folks thinking we need multiple lock files (one per platform). I think it is perhaps avoidable. In my prototype branch, I have now the following behavior:
|
Let me try it... OK, I have created new branch in reproducer repo where I will use
Here we go: https://github.com/ChaoticRoman/pipenv-bug-demo/pull/1/checks Still there is on MacOS run unnecessary:
|
@ChaoticRoman It didn't install anything, but it did resolve the dependencies. This could be a big improvement over prior pipenv behaviors, it means the lock file you generate on windows or mac os is installable on linux. |
Allright, this would be interesting to try on the thing that was failing due to |
So I tested it and sadly it is not working. (Switching back to
Failed with:
|
@ChaoticRoman Well it has to build the sdist of the package for it to resolve the package+subdependencies, then resolution will only be able to take place on the platform that can build it. I've been playing around with a concept that allows overriding environment/platform attributes for the resolve phase, but it won't help lock on a system that cannot build the sdist. #5931 |
We have a project that is build on Linux, MacOS and Windows. On Linux, desktop integration use PyGObject, others work differently, so there was no need to install it on other platforms. So we put this into our
Pipfile
:Strangely
pipenv
was trying to install its dependencies on all platforms. After some trial and errors, this helped:I made replication of this issue using GitHub Actions and indeed,
sys_platform = "=='linux'"
andmarkers="sys_platform == 'linux'"
versions behave differently. The first one is downloading requests on other platforms and installing its dependencies (though not intstalling requests themselves):but the second one is not, it is working as you would expect (no unneeded downloads or deps installed):
Is this expected?
The text was updated successfully, but these errors were encountered: