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

Resolution fails due to encoding issue with version strings containing a + #143

Closed
fviernau opened this issue Sep 14, 2023 · 2 comments · Fixed by #144
Closed

Resolution fails due to encoding issue with version strings containing a + #143

fviernau opened this issue Sep 14, 2023 · 2 comments · Fixed by #144

Comments

@fviernau
Copy link
Contributor

fviernau commented Sep 14, 2023

The following requirements.txt can be resolved with pip, but not with python-inspector.

--extra-index-url https://download.pytorch.org/whl/cpu
torch==2.0.0+cpu

Outcome:

python-inspector -p 3.10  --operating-system linux --json-pdt x.json --analyze-setup-py-insecurely --requirement requirements.txt --verbose
Resolving dependencies...
direct_dependencies:
 DependentPackage(purl='pkg:pypi/[email protected]%2Bcpu', extracted_requirement='torch==2.0.0+cpu', scope='install')
environment: Environment(python_version='310', operating_system='linux')
repos:
 PypiSimpleRepository(index_url='https://pypi.org/simple', credentials=None)
Traceback (most recent call last):
  File "/home/frank/.local/lib/python3.10/site-packages/resolvelib/resolvers.py", line 397, in resolve
    self._add_to_criteria(self.state.criteria, r, parent=None)
  File "/home/frank/.local/lib/python3.10/site-packages/resolvelib/resolvers.py", line 174, in _add_to_criteria
    raise RequirementsConflicted(criterion)
resolvelib.resolvers.RequirementsConflicted: Requirements conflict: <Requirement('torch==2.0.0+cpu')>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/frank/.local/lib/python3.10/site-packages/python_inspector/resolve_cli.py", line 247, in resolve_dependencies
    resolution_result: Dict = resolver_api(
  File "/home/frank/.local/lib/python3.10/site-packages/python_inspector/api.py", line 263, in resolve_dependencies
    resolution, purls = resolve(
  File "/home/frank/.local/lib/python3.10/site-packages/python_inspector/api.py", line 322, in resolve
    resolved_dependencies, packages = get_resolved_dependencies(
  File "/home/frank/.local/lib/python3.10/site-packages/python_inspector/api.py", line 360, in get_resolved_dependencies
    resolver_results = resolver.resolve(requirements=requirements, max_rounds=max_rounds)
  File "/home/frank/.local/lib/python3.10/site-packages/resolvelib/resolvers.py", line 546, in resolve
    state = resolution.resolve(requirements, max_rounds=max_rounds)
  File "/home/frank/.local/lib/python3.10/site-packages/resolvelib/resolvers.py", line 399, in resolve
    raise ResolutionImpossible(e.criterion.information)
resolvelib.resolvers.ResolutionImpossible: [RequirementInformation(requirement=<Requirement('torch==2.0.0+cpu')>, parent=None)]

Observation:

After some debugging, I found that packages_from_links() in utils_pypi.py operates on packages which have the version encoded. In particular, the + is encoded as %2B. My guess is that the matching somehow then compares + with %2B and fails. I tried the following (workaround) requirements.txt, which can be successfully analyzed. I guess this proofs that it's an encoding bug:

--extra-index-url https://download.pytorch.org/whl/cpu
torch==2.0.0%2Bcpu
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 15, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoded, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 15, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 25, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 25, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 25, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to fviernau/python-inspector that referenced this issue Sep 25, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
@pombredanne
Copy link
Member

For reference, not sure if you have read this before, the + is for "local version identifiers" as explained here https://peps.python.org/pep-0440/#local-version-identifiers and here https://peps.python.org/pep-0440/#adding-local-version-identifiers

fviernau added a commit to fviernau/python-inspector that referenced this issue Oct 10, 2023
`Distribution.from_link()` derives the version string of a package from
the given (percent encoded) `Link.url`. That derivation lacks the
decoding, so the resulting version string may also contain percent
encoded characters in which case the dependency resolution fails.

Fix the resolution by URL adding the missing unquoting.

Fixes aboutcode-org#143.

Signed-off-by: Frank Viernau <[email protected]>
@pombredanne
Copy link
Member

@fviernau I have a fewthings to push to our branch shortly

pombredanne added a commit to fviernau/python-inspector that referenced this issue Oct 24, 2023
Move the resolution to the from_filename() method in subclasses

Reference: aboutcode-org#143
Signed-off-by: Philippe Ombredanne <[email protected]>
pombredanne added a commit to fviernau/python-inspector that referenced this issue Oct 24, 2023
These new test were missing originally and they excercise all the
corner cases of encoding.

Reference: aboutcode-org#143
Signed-off-by: Philippe Ombredanne <[email protected]>
pombredanne added a commit to fviernau/python-inspector that referenced this issue Oct 24, 2023
* Ensure that we honor the --generic-paths option when converting to
plain mapping.

* Avoid recursive imports by moving remove_test_data_dir_variable_prefix
to utils.py

* Simplifify tests to bypass the creation of an output file when not
needed

* Some tests are also updated to account for package version updates.

Reference: aboutcode-org#143
Signed-off-by: Philippe Ombredanne <[email protected]>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Oct 31, 2023
Amongst others, the new release has a fix for a version string encoding
related issue, see aboutcode-org/python-inspector#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Nov 1, 2023
Amongst others, the new release has a fix for a version string encoding
related issue, see aboutcode-org/python-inspector#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Nov 1, 2023
Amongst others, the new release has a fix for a version string encoding
related issue, see aboutcode-org/python-inspector#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Nov 1, 2023
Amongst others, the new release has a fix for a version string encoding
related issue, see aboutcode-org/python-inspector#143.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit to oss-review-toolkit/ort that referenced this issue Nov 1, 2023
Amongst others, the new release has a fix for a version string encoding
related issue, see aboutcode-org/python-inspector#143.

Signed-off-by: Frank Viernau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants