-
Notifications
You must be signed in to change notification settings - Fork 752
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
universal-lock: audit data structures to ensure they can handle multiple versions of the same package #3927
Comments
Here's an example failing [project]
name = "black"
version = "0.1.0"
description = "Default template for a Flit project"
authors = [
{name = "konstin", email = "[email protected]"},
]
dependencies = [
"iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'",
"iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'"
]
requires-python = ">=3.11,<3.13"
license = {text = "MIT"}
[build-system]
requires = ["flit_core>=3.4,<4"]
build-backend = "flit_core.buildapi" |
I see three main types we have to change:
|
I'm working on this piece today. |
Why does the lookahead resolver or the manifest need to track diverging branches? Isn’t it enough to just track the markers, then handle divergence in Urls? |
Let's say we have root:
Here we need to keep track of the markers so that when we ask "what is the url of b for |
It needs to track the requiring markers, just like it tracks the requiring extra, but I don't know that it needs to track divergence. Anyway, give it a shot, the implementation will prove out whether it's necessary or not, I might be wrong here. |
Yes, the lookahead resolver does not actually need to check the divergence, it just needs to remember it which marker-conditionalized branch it currently is. |
## Introduction We support forking the dependency resolution to support conflicting registry requirements for different platforms, say on package range is required for an older python version while a newer is required for newer python versions, or dependencies that are different per platform. We need to extend this support to direct URL requirements. ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'" ] ``` This did not work because `Urls` was built on the assumption that there is a single allowed URL per package. We collect all allowed URL ahead of resolution by following direct URL dependencies (including path dependencies) transitively, i.e. a registry distribution can't require a URL. ## Registry and URL requirement in the same package Consider the following two cases: requirements.in: ```text werkzeug==2.0.0 werkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl ``` pyproject.toml: ```toml dependencies = [ "iniconfig == 1.1.1 ; python_version < '3.12'", "iniconfig @ git+https://github.com/pytest-dev/iniconfig@93f5930e668c0d1ddf4597e38dd0dea4e2665e7a ; python_version >= '3.12'", ] ``` In the first case, we want the URL to override the registry dependency, in the second case we want to fork and have one branch use the registry and the other the URL. We have to know about this in `PubGrubRequirement::from_requirement`, but we only fork after the current method. Instead, we can check if the registry and the URL requirement for the same name are disjoint: If they are, they must end up in different branches on forking, if they aren't, we know the URL overrides the registry. An alternative implementation would be splitting in the lookahead resolver so that all URLs are already tagged by the markers of their fork. This would require duplicating the forking logic while making sure that we follow the exact same precedence rules, which is why i didn't do it. ## Determining the URL of a Requirement For a registry requirement: * If there is a URL override: Use that override, done. * If there is a fork-specific URL: Use that, done. * If there is another requirement in the same package with a URL requirement find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. For a URL requirement: * If there is a URL override: Use that override, done. * Find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. ## URL conflicts pyproject.toml (invalid): ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", ] ``` On the fork state, we keep `ForkUrls` that check for conflicts after forking, rejecting the third case because we added two packages of the same name with different URLs. We need to flatten out the requirements before transformation into pubgrub requirements to get the full list of other requirements which may contain a URL, which was changed in a previous PR: #4430. ## Complex Example a: ```toml dependencies = [ # Force a split "anyio==4.3.0 ; python_version >= '3.12'", "anyio==4.2.0 ; python_version < '3.12'", # Include URLs transitively "b" ] ``` b: ```toml dependencies = [ # Only one is used in each split. "b1 ; python_version < '3.12'", "b2 ; python_version >= '3.12'", "b3 ; python_version >= '3.12'", ] ``` b1: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", ] ``` b2: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", ] ``` b3: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", ] ``` In this example, all packages are url requirements (directory requirements) and the root package is `a`. We first split on `a`, `b` being in each split. In the first fork, we reach `b1`, the fork URLs are empty, we insert the iniconfig 1.1.1 URL, and then we skip over `b2` and `b3` since the mark is disjoint with the fork markers. In the second fork, we skip over `b1`, visit `b2`, insert the iniconfig 2.0.0 URL into the again empty fork URLs, then visit `b3` and try to insert the iniconfig 1.1.0 URL. At this point we find a conflict for the iniconfig URL and error. ## Closing Best reviewed commit-by-commit, the tests are verbose. The git tests are slow, but they make the best example for different URL types i could find. Part of #3927. This PR does not handle `Locals` yet.
## Introduction We support forking the dependency resolution to support conflicting registry requirements for different platforms, say on package range is required for an older python version while a newer is required for newer python versions, or dependencies that are different per platform. We need to extend this support to direct URL requirements. ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'" ] ``` This did not work because `Urls` was built on the assumption that there is a single allowed URL per package. We collect all allowed URL ahead of resolution by following direct URL dependencies (including path dependencies) transitively, i.e. a registry distribution can't require a URL. ## Registry and URL requirement in the same package Consider the following two cases: requirements.in: ```text werkzeug==2.0.0 werkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl ``` pyproject.toml: ```toml dependencies = [ "iniconfig == 1.1.1 ; python_version < '3.12'", "iniconfig @ git+https://github.com/pytest-dev/iniconfig@93f5930e668c0d1ddf4597e38dd0dea4e2665e7a ; python_version >= '3.12'", ] ``` In the first case, we want the URL to override the registry dependency, in the second case we want to fork and have one branch use the registry and the other the URL. We have to know about this in `PubGrubRequirement::from_requirement`, but we only fork after the current method. Instead, we can check if the registry and the URL requirement for the same name are disjoint: If they are, they must end up in different branches on forking, if they aren't, we know the URL overrides the registry. An alternative implementation would be splitting in the lookahead resolver so that all URLs are already tagged by the markers of their fork. This would require duplicating the forking logic while making sure that we follow the exact same precedence rules, which is why i didn't do it. ## Determining the URL of a Requirement For a registry requirement: * If there is a URL override: Use that override, done. * If there is a fork-specific URL: Use that, done. * If there is another requirement in the same package with a URL requirement find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. For a URL requirement: * If there is a URL override: Use that override, done. * Find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. ## URL conflicts pyproject.toml (invalid): ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", ] ``` On the fork state, we keep `ForkUrls` that check for conflicts after forking, rejecting the third case because we added two packages of the same name with different URLs. We need to flatten out the requirements before transformation into pubgrub requirements to get the full list of other requirements which may contain a URL, which was changed in a previous PR: #4430. ## Complex Example a: ```toml dependencies = [ # Force a split "anyio==4.3.0 ; python_version >= '3.12'", "anyio==4.2.0 ; python_version < '3.12'", # Include URLs transitively "b" ] ``` b: ```toml dependencies = [ # Only one is used in each split. "b1 ; python_version < '3.12'", "b2 ; python_version >= '3.12'", "b3 ; python_version >= '3.12'", ] ``` b1: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", ] ``` b2: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", ] ``` b3: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", ] ``` In this example, all packages are url requirements (directory requirements) and the root package is `a`. We first split on `a`, `b` being in each split. In the first fork, we reach `b1`, the fork URLs are empty, we insert the iniconfig 1.1.1 URL, and then we skip over `b2` and `b3` since the mark is disjoint with the fork markers. In the second fork, we skip over `b1`, visit `b2`, insert the iniconfig 2.0.0 URL into the again empty fork URLs, then visit `b3` and try to insert the iniconfig 1.1.0 URL. At this point we find a conflict for the iniconfig URL and error. ## Closing Best reviewed commit-by-commit, the tests are verbose. The git tests are slow, but they make the best example for different URL types i could find. Part of #3927. This PR does not handle `Locals` yet.
## Introduction We support forking the dependency resolution to support conflicting registry requirements for different platforms, say on package range is required for an older python version while a newer is required for newer python versions, or dependencies that are different per platform. We need to extend this support to direct URL requirements. ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'" ] ``` This did not work because `Urls` was built on the assumption that there is a single allowed URL per package. We collect all allowed URL ahead of resolution by following direct URL dependencies (including path dependencies) transitively, i.e. a registry distribution can't require a URL. ## Registry and URL requirement in the same package Consider the following two cases: requirements.in: ```text werkzeug==2.0.0 werkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl ``` pyproject.toml: ```toml dependencies = [ "iniconfig == 1.1.1 ; python_version < '3.12'", "iniconfig @ git+https://github.com/pytest-dev/iniconfig@93f5930e668c0d1ddf4597e38dd0dea4e2665e7a ; python_version >= '3.12'", ] ``` In the first case, we want the URL to override the registry dependency, in the second case we want to fork and have one branch use the registry and the other the URL. We have to know about this in `PubGrubRequirement::from_requirement`, but we only fork after the current method. Instead, we can check if the registry and the URL requirement for the same name are disjoint: If they are, they must end up in different branches on forking, if they aren't, we know the URL overrides the registry. An alternative implementation would be splitting in the lookahead resolver so that all URLs are already tagged by the markers of their fork. This would require duplicating the forking logic while making sure that we follow the exact same precedence rules, which is why i didn't do it. ## Determining the URL of a Requirement For a registry requirement: * If there is a URL override: Use that override, done. * If there is a fork-specific URL: Use that, done. * If there is another requirement in the same package with a URL requirement find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. For a URL requirement: * If there is a URL override: Use that override, done. * Find a matching URL in `Urls`, use it to canonicalize the current URL. * After forking: Check that this URL is the only URL in this fork. ## URL conflicts pyproject.toml (invalid): ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", ] ``` On the fork state, we keep `ForkUrls` that check for conflicts after forking, rejecting the third case because we added two packages of the same name with different URLs. We need to flatten out the requirements before transformation into pubgrub requirements to get the full list of other requirements which may contain a URL, which was changed in a previous PR: #4430. ## Complex Example a: ```toml dependencies = [ # Force a split "anyio==4.3.0 ; python_version >= '3.12'", "anyio==4.2.0 ; python_version < '3.12'", # Include URLs transitively "b" ] ``` b: ```toml dependencies = [ # Only one is used in each split. "b1 ; python_version < '3.12'", "b2 ; python_version >= '3.12'", "b3 ; python_version >= '3.12'", ] ``` b1: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", ] ``` b2: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", ] ``` b3: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", ] ``` In this example, all packages are url requirements (directory requirements) and the root package is `a`. We first split on `a`, `b` being in each split. In the first fork, we reach `b1`, the fork URLs are empty, we insert the iniconfig 1.1.1 URL, and then we skip over `b2` and `b3` since the mark is disjoint with the fork markers. In the second fork, we skip over `b1`, visit `b2`, insert the iniconfig 2.0.0 URL into the again empty fork URLs, then visit `b3` and try to insert the iniconfig 1.1.0 URL. At this point we find a conflict for the iniconfig URL and error. ## Closing Best reviewed commit-by-commit, the tests are verbose. The git tests are slow, but they make the best example for different URL types i could find. Part of #3927. This PR does not handle `Locals` yet.
Downstack PR: #4481 ## Introduction We support forking the dependency resolution to support conflicting registry requirements for different platforms, say on package range is required for an older python version while a newer is required for newer python versions, or dependencies that are different per platform. We need to extend this support to direct URL requirements. ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'" ] ``` This did not work because `Urls` was built on the assumption that there is a single allowed URL per package. We collect all allowed URL ahead of resolution by following direct URL dependencies (including path dependencies) transitively, i.e. a registry distribution can't require a URL. ## The same package can have Registry and URL requirements Consider the following two cases: requirements.in: ```text werkzeug==2.0.0 werkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl ``` pyproject.toml: ```toml dependencies = [ "iniconfig == 1.1.1 ; python_version < '3.12'", "iniconfig @ git+https://github.com/pytest-dev/iniconfig@93f5930e668c0d1ddf4597e38dd0dea4e2665e7a ; python_version >= '3.12'", ] ``` In the first case, we want the URL to override the registry dependency, in the second case we want to fork and have one branch use the registry and the other the URL. We have to know about this in `PubGrubRequirement::from_registry_requirement`, but we only fork after the current method. Consider the following case too: a: ``` c==1.0.0 b @ https://b.zip ``` b: ``` c @ https://c_new.zip ; python_version >= '3.12'", c @ https://c_old.zip ; python_version < '3.12'", ``` When we convert the requirements of `a`, we can't know the url of `c` yet. The solution is to remove the `Url` from `PubGrubPackage`: The `Url` is redundant with `PackageName`, there can be only one url per package name per fork. We now do the following: We track the urls from requirements in `PubGrubDependency`. After forking, we call `add_package_version_dependencies` where we apply override URLs, check if the URL is allowed and check if the url is unique in this fork. When we request a distribution, we ask the fork urls for the real URL. Since we prioritize url dependencies over registry dependencies and skip packages with `Urls` entries in pre-visiting, we know that when fetching a package, we know if it has a url or not. ## URL conflicts pyproject.toml (invalid): ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl ; python_version < '3.12'", "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl ; python_version >= '3.12'", ] ``` On the fork state, we keep `ForkUrls` that check for conflicts after forking, rejecting the third case because we added two packages of the same name with different URLs. We need to flatten out the requirements before transformation into pubgrub requirements to get the full list of other requirements which may contain a URL, which was changed in a previous PR: #4430. ## Complex Example a: ```toml dependencies = [ # Force a split "anyio==4.3.0 ; python_version >= '3.12'", "anyio==4.2.0 ; python_version < '3.12'", # Include URLs transitively "b" ] ``` b: ```toml dependencies = [ # Only one is used in each split. "b1 ; python_version < '3.12'", "b2 ; python_version >= '3.12'", "b3 ; python_version >= '3.12'", ] ``` b1: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", ] ``` b2: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", ] ``` b3: ```toml dependencies = [ "iniconfig @ https://files.pythonhosted.org/packages/44/39/e96292c7f7068e58877f476908c5974dc76c37c623f1fa332fe4ed6dfbec/iniconfig-1.1.0.tar.gz", ] ``` In this example, all packages are url requirements (directory requirements) and the root package is `a`. We first split on `a`, `b` being in each split. In the first fork, we reach `b1`, the fork URLs are empty, we insert the iniconfig 1.1.1 URL, and then we skip over `b2` and `b3` since the mark is disjoint with the fork markers. In the second fork, we skip over `b1`, visit `b2`, insert the iniconfig 2.0.0 URL into the again empty fork URLs, then visit `b3` and try to insert the iniconfig 1.1.0 URL. At this point we find a conflict for the iniconfig URL and error. ## Closing The git tests are slow, but they make the best example for different URL types i could find. Part of #3927. This PR does not handle `Locals` or pre-releases yet.
Basically, it is suspected that there are some parts of our resolver that implicitly rely on the fact that there is only one version of a package available in resolution. This often leads to things like indexing on just package name. But we probably need to index on version (and possibly extra name?).
This was first brought up by @charliermarsh in #3831 (review)
The text was updated successfully, but these errors were encountered: