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

Poetry 1.2+ incorrectly appears to validate 'platform_release' markers against PEP 440 #7418

Closed
4 tasks done
j-baker opened this issue Jan 26, 2023 · 8 comments · Fixed by python-poetry/poetry-core#722
Closed
4 tasks done
Labels
area/core Related to the poetry-core library kind/bug Something isn't working as expected

Comments

@j-baker
Copy link

j-baker commented Jan 26, 2023

  • Poetry version: 1.3.2
  • Python version: 3.11.1
  • OS version and name: macOS 13.1
  • pyproject.toml: not publicly available pyproject.toml, but doesn't seem relevant.
  • 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

We have a package with a marker on a platform_constraint. I'm not fully spun up on what it's doing, but I think it's downloading different versions of a dependency depending on which embedded platform Poetry is being run on.

I believe that this is a regression between Poetry 1.1.15 and Poetry 1.2 because Poetry 1.2 stopped supporting Python 3.6 and this project was stuck on that until recently.

The constraint looks something like:

some-package = [
    {version="1.0.0", optional = true, source = "source", markers = "platform_release == '1.0.0-platformname'}
    {version="1.2.1", optional = true, source = "source", markers = "platform_release == '1.0.0-otherplatformname'}
]

What I'm seeing is that PEP440 constraint validation is failing on the platform_release, and clearly that's not a valid PEP440 constraint. It's also not meant to be, though - PEP508 describes that a platform_release might be something like 3.14.1-x86_64-linode39. From my read of the code, I have a hunch that... platform_releases are considered to be 'version like' because they could be of the form 13.0 or the like, and at some point, someone started validating version like constraints against PEP 440. But I'm not familiar with this codebase so it's just a hunch.

  Stack trace:

  20  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/application.py:327 in run
       325│
       326│             try:
     → 327│                 exit_code = self._run(io)
       328│             except BrokenPipeError:
       329│                 # If we are piped to another process, it may close early and send a

  19  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/console/application.py:190 in _run
       188│         self._load_plugins(io)
       189│
     → 190│         exit_code: int = super()._run(io)
       191│         return exit_code
       192│

  18  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/application.py:431 in _run
       429│             io.input.interactive(interactive)
       430│
     → 431│         exit_code = self._run_command(command, io)
       432│         self._running_command = None
       433│

  17  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/application.py:473 in _run_command
       471│
       472│         if error is not None:
     → 473│             raise error
       474│
       475│         return terminate_event.exit_code

  16  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/application.py:457 in _run_command
       455│
       456│             if command_event.command_should_run():
     → 457│                 exit_code = command.run(io)
       458│             else:
       459│                 exit_code = ConsoleCommandEvent.RETURN_CODE_DISABLED

  15  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/commands/base_command.py:119 in run
       117│         io.input.validate()
       118│
     → 119│         status_code = self.execute(io)
       120│
       121│         if status_code is None:

  14  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/cleo/commands/command.py:62 in execute
        60│
        61│         try:
     →  62│             return self.handle()
        63│         except KeyboardInterrupt:
        64│             return 1

  13  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/console/commands/version.py:87 in handle
        85│             else:
        86│                 self.line(
     →  87│                     f"{self.poetry.package.name}"
        88│                     f" {self.poetry.package.pretty_version}"
        89│                 )

  12  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/console/commands/command.py:23 in poetry
        21│     def poetry(self) -> Poetry:
        22│         if self._poetry is None:
     →  23│             return self.get_application().poetry
        24│
        25│         return self._poetry

  11  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/console/application.py:129 in poetry
       127│             project_path = self._io.input.option("directory")
       128│
     → 129│         self._poetry = Factory().create_poetry(
       130│             cwd=project_path,
       131│             io=self._io,

  10  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/factory.py:55 in create_poetry
        53│             io = NullIO()
        54│
     →  55│         base_poetry = super().create_poetry(cwd=cwd, with_groups=with_groups)
        56│
        57│         locker = Locker(

   9  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/factory.py:65 in create_poetry
        63│         assert isinstance(version, str)
        64│         package = self.get_package(name, version)
     →  65│         package = self.configure_package(
        66│             package, local_config, poetry_file.parent, with_groups=with_groups
        67│         )

   8  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/factory.py:159 in configure_package
       157│
       158│         if "dependencies" in config:
     → 159│             cls._add_package_group_dependencies(
       160│                 package=package, group=MAIN_GROUP, dependencies=config["dependencies"]
       161│             )

   7  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/factory.py:105 in _add_package_group_dependencies
       103│
       104│                 group.add_dependency(
     → 105│                     cls.create_dependency(
       106│                         name,
       107│                         _constraint,

   6  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/factory.py:347 in create_dependency
       345│                 )
       346│
     → 347│             marker = parse_marker(markers) if markers else AnyMarker()
       348│
       349│             if python_versions:

   5  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/version/markers.py:791 in parse_marker
       789│     parsed = _parser.parse(marker)
       790│
     → 791│     markers = _compact_markers(parsed.children)
       792│
       793│     return markers

   4  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/version/markers.py:809 in _compact_markers
       807│         if token.data == "marker":
       808│             groups[-1] = MultiMarker.of(
     → 809│                 groups[-1], _compact_markers(token.children, tree_prefix=tree_prefix)
       810│             )
       811│         elif token.data == f"{tree_prefix}item":

   3  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/version/markers.py:821 in _compact_markers
       819│             value = value[1:-1]
       820│             groups[-1] = MultiMarker.of(
     → 821│                 groups[-1], SingleMarker(str(name), f"{op}{value}")
       822│             )
       823│         elif token.data == f"{tree_prefix}BOOL_OP" and token.children[0] == "or":

   2  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/version/markers.py:232 in __init__
       230│                 self._constraint = self._parser(glue.join(versions))
       231│             else:
     → 232│                 self._constraint = self._parser(constraint_string)
       233│         else:
       234│             # if we have a in/not in operator we split the constraint

   1  ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/constraints/version/parser.py:36 in parse_constraint
        34│                 constraint_objects.append(parse_single_constraint(constraint))
        35│         else:
     →  36│             constraint_objects.append(parse_single_constraint(and_constraints[0]))
        37│
        38│         if len(constraint_objects) == 1:

  ParseConstraintError

  Could not parse version constraint: ==1.0.0-ourversion

  at ~/.local/pipx/venvs/poetry/lib/python3.11/site-packages/poetry/core/constraints/version/parser.py:167 in parse_single_constraint
      163│         if op == "!=":
      164│             return VersionUnion(VersionRange(max=version), VersionRange(min=version))
      165│         return version
      166│
    → 167│     raise ParseConstraintError(f"Could not parse version constraint: {constraint}")
      168│

@j-baker j-baker added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Jan 26, 2023
@dimbleby
Copy link
Contributor

@j-baker
Copy link
Author

j-baker commented Jan 26, 2023

Yes, I found that snippet! The problem I saw was that there are actually tests on specifically this element being depended on as a version and supporting version constraints, and my assumption is that if I remove it from the version_like list, these tests will break.
If we start treating this marker as 'not-a-version', is that a problem for users in practice? I could imagine that on e.g. OSX where it genuinely may be a version it could lead to problems. I thought it was likely that this was more of an unintended consequence of another refactor for this reason.

@j-baker
Copy link
Author

j-baker commented Jan 26, 2023

it's possible however that it was just based on the author of the test picking any of the 3 as a 'version like marker'. do you have context?

@dimbleby
Copy link
Contributor

I expect it's a deliberate test but - you are telling us - of wrong behaviour. The PEP should be the final arbiter.

@ohiliazov
Copy link

ohiliazov commented Mar 9, 2023

I have the same issue with these markers:

torchvision = [
    { version = "0.10.0+4.9.253tegra", markers = "platform_release == '4.9.253-tegra'" },
    { version = "0.10.0+x86.cpu", markers = "platform_release != '4.9.253-tegra'" },
]

This prevents from upgrading from Poetry 1.1.15.

@tiagovrtr
Copy link

I have the same issue with these markers:

torchvision = [
    { version = "0.10.0+4.9.253tegra", markers = "platform_release == '4.9.253-tegra'" },
    { version = "0.10.0+x86.cpu", markers = "platform_release != '4.9.253-tegra'" },
]

This prevents from upgrading from Poetry 1.1.15.

I'm experiencing this issue with a very similar example for torch

@FlowMatric
Copy link

FlowMatric commented Oct 3, 2023

Adding another case here: I am experiencing this issue with a wheel that I am building (amzi-pyras-lib, unfortunately it's not public) and which itself has the following dependencies:

pydsstools = [
    { version = "^2.3", markers = "sys_platform == 'win32'" },
    { file = "lib/pydsstools-3.2.1.gd65443b.dirty-cp38-cp38-linux_x86_64.whl", markers = "python_version == '3.8' and sys_platform == 'linux'" },
    { file = "lib/pydsstools-3.2.1.gd65443b.dirty-cp39-cp39-linux_x86_64.whl", markers = "python_version == '3.9' and sys_platform == 'linux'" },
    { file = "lib/pydsstools-3.2.1.gd65443b.dirty-cp310-cp310-linux_x86_64.whl", markers = "python_version == '3.10' and sys_platform == 'linux'" },
]

Note that I am using poetry to build this wheel and also to test it via nox. The tests install it virtual envs without hickup1, but as soon as I add amzi-pyras-lib as a dependency to another project, the install fails like so:

$ poetry add --dry-run lib/amzi_pyras_lib-0.0.1-py3-none-any.whl 

Could not parse version constraint: 3.2.1.gd65443b.dirty

-vvv shows that parser.py fails at line 188 but since I am at Poetry v1.5.1 I am not surprised about this discrepancy to the original issue posting.

1) I just realized that nox runs poetry export --format=requirements.txt --with=bump --without-hashes and then installs into the .venv via pip

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 Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/core Related to the poetry-core library kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants