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

Multiple environment markers doesn't work #7722

Closed
4 tasks done
marhoy opened this issue Mar 27, 2023 · 12 comments · Fixed by #7973
Closed
4 tasks done

Multiple environment markers doesn't work #7722

marhoy opened this issue Mar 27, 2023 · 12 comments · Fixed by #7973
Labels
area/solver Related to the dependency resolver kind/bug Something isn't working as expected

Comments

@marhoy
Copy link

marhoy commented Mar 27, 2023

  • Poetry version: 1.4.1
  • Python version: 3.10.7
  • OS version and name: Ubuntu 22.04.2 and Debian 11
  • I am on the latest stable Poetry version, installed using a recommended method.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have consulted the FAQ and blog for any relevant entries or release notes.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

I'm trying to install a different version of torch, depending on the CPU. If I'm on the Raspberry Pi, I want to use the wheel from download.pytorch.org, otherwise I can install from pypi. (The reason for this is a cuda-dependency-issue, but that's not the point here).

I have the following in my pyproject.toml

torch = [
    { markers = "platform_machine == 'aarch64'", url = "https://download.pytorch.org/whl/torch-1.13.1-cp310-cp310-manylinux2014_aarch64.whl"},
    { markers = "platform_machine == 'x86_64'", version = "^1.13.1"},
]

The problem is that the marker-logic doesn't work: Poetry just installs the first of the two alternatives, and ignores the markes.

Am I doing something wrong?

@marhoy marhoy added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Mar 27, 2023
@dimbleby
Copy link
Contributor

duplicate #5506 I expect

@radoering
Copy link
Member

duplicate #5506 I expect

I don't think so. #5506 is about one multiple-constraints dependency and one restricted (single constraint) dependency, which somehow depend on each other.

At first glance this looks as if it should work except when local version identifiers are involved. Then, it could be a duplicate of some other issue.

@marhoy Which versions are locked. Can you share your lock file?

@dimbleby
Copy link
Contributor

you should provide an explicit source = "pypi" in the one you want from pypi, so that poetry knows that it cannot be satisfied by the url whl.

That's still not enough, though, this fix looks like it should finish the job:

diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py
index ed88b57d..79271517 100644
--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -320,12 +320,10 @@ class Provider:
         #
         # We rely on the VersionSolver resolving direct-origin dependencies first.
         direct_origin_package = self._direct_origin_packages.get(dependency.name)
-        if direct_origin_package is not None:
-            packages = (
-                [direct_origin_package]
-                if dependency.constraint.allows(direct_origin_package.version)
-                else []
-            )
+        if direct_origin_package is not None and direct_origin_package.satisfies(
+            dependency, ignore_source_type=False
+        ):
+            packages = [direct_origin_package]
             return PackageCollection(dependency, packages)

         packages = self._pool.find_packages(dependency)

However that would require testcase updates and I'm not sure that I'm interested in this enough to bother! If anyone wants to pick this up, the above is my suggestion.

@radoering radoering added area/solver Related to the dependency resolver and removed status/triage This issue needs to be triaged labels Mar 28, 2023
@jscheel
Copy link

jscheel commented Apr 26, 2023

@dimbleby please forgive me, as I'm fairly new to python, but if this isn't a common use-case, how to you usually deal with dependency development? I'm currently working on the same thing. I have a commonlib that I want to share between projects. In dev, I just want to be able to hack away on it, but when deploying to production, it should install a tagged version from a git repo. The workflow is quite similar to npm link in the JS world. Is there a better way to go about this?

@dimbleby
Copy link
Contributor

if this isn't a common use-case

I have expressed no view on that.

If this is a use case that you are interested in... well I suggested a direction for a fix, above.

@ralbertazzi
Copy link
Contributor

@dimbleby your solution is still incomplete because direct_origin_package.satisfies(dependency) still evaluates to True for two reasons:

  • direct_origin_package.marker is always AnyMarker() as the dependency marker doesn't get transferred to the retrieved package (at least for all direct origin dependencies)
  • Package.satisfies does not check markers at all

They both look easy fixes, but I don't know if this is the right path. After all, the marker belongs to the dependency, not to the package, so the current code may be considered ok. On the other side I see that Package.to_dependency forwards the marker, so there must be some correlation 🤷

Another solution could be transforming Provider._direct_origin_packages into something more sophisticated, where both the dependency and the retrieved package are somehow stored. At that point you could use the dependency marker.

I'll wait for your input on how to proceed, as my confidence on poetry-core is still low 🙏

@dimbleby
Copy link
Contributor

dimbleby commented May 19, 2023

I don't think further change is needed, my suggestion still looks good to me!

you should provide an explicit source = "pypi" in the one you want from pypi, so that poetry knows that it cannot be satisfied by the url whl.

Then direct_origin_package.satisfies(dependency, ignore_source_type=False) returns False - the direct origin package does not come from pypi.

@ralbertazzi
Copy link
Contributor

you should provide an explicit source = "pypi" in the one you want from pypi, so that poetry knows that it cannot be satisfied by the url whl.

I forgot to mention that I didn't in fact consider this part of the solution, as I honestly consider it more of a hack than a robust solution. If markers are incompatible then I shouldn't bother specifying a different source to let Poetry get the incompatibility, rigth?

@dimbleby
Copy link
Contributor

dimbleby commented May 19, 2023

No, I don't think this is a hack. I don't agree that incompatible markers should always force poetry to choose different solutions

eg a requirement like

example = [
    { markers = "platform_machine == 'aarch64'", url = "https://example.com/example-1.0-py3-none-any.whl" },
    { markers = "platform_machine == 'x86_64'", version = "^1.0" },
]

can and should be satisfied just by using the named wheel

the current interpretation of not specifying a source is "any source will do", and that seems quite reasonable to me.

@ralbertazzi
Copy link
Contributor

a requirement like can and should be satisfied just by using the named wheel

True, and I agree that incompatible markers should not always force poetry to choose different solutions. But for the specific case of mixing direct origin dependencies and source dependencies, I wonder:

  1. How many Poetry users would use the multiple constraints feature in a conscious way, knowing that the source requirement can be satisfied by the url package (very few I believe).
  2. How many Poetry users would use the multiple constraints feature not this way - i.e. believing that simply specifying mutually exclusive markers will be enough to let Poetry choose a different package - and fall into the trap. I'd honestly consider myself in this group, and similarly all issues that have been opened concerning this bug.

Still, I can at least provide a PR for the fix that you suggested, which needs to be done anyways. We can then decide to further iterate on it if issues keep coming, or simply provide more insightful documentation :)

@dimbleby
Copy link
Contributor

dimbleby commented May 20, 2023

"source not specified" has to be treated as "any source will do" in case there is an direct dependency on a direct-origin package foo, and also an indirect dependency (ie via some other package) on foo.

Then if that indirect dependency was not allowed to be satisfied by the direct-origin package, there could be no solution.

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/solver Related to the dependency resolver kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants