Skip to content

Fix extracting extras from markers with many extras#2792

Merged
gaborbernat merged 2 commits intotox-dev:mainfrom
93578237:2791
Dec 31, 2022
Merged

Fix extracting extras from markers with many extras#2792
gaborbernat merged 2 commits intotox-dev:mainfrom
93578237:2791

Conversation

@93578237
Copy link
Copy Markdown
Contributor

Thanks for contribution

Please, make sure you address all the checklists (for details on how see
development documentation)!

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

@93578237
Copy link
Copy Markdown
Contributor Author

93578237 commented Dec 30, 2022

What should be done with requirements like 'sphinx>=3; extra == "extras1" and extra == "extras2"'

requires = [
        Requirement('filelock<4.0.0,>=3.9.0; extra == "extras1" or extra == "extras2"'),
        Requirement('sphinx>=3; extra == "extras1" and extra == "extras2"')
    ]
result = dependencies_with_extras(requires, {"extras1"}, "")

What result is expected?

@93578237
Copy link
Copy Markdown
Contributor Author

93578237 commented Dec 30, 2022

Maybe its a bug in the poetry build backend, because setuptools makes different METADATA Requires-Dists

Provides-Extra: extras1
Requires-Dist: filelock (<4.0.0,>=3.9.0) ; extra == 'extras1'
Provides-Extra: extras2
Requires-Dist: filelock (<4.0.0,>=3.9.0) ; extra == 'extras2'

@gaborbernat
Copy link
Copy Markdown
Member

What result is expected?

I'd say only filelock to show up, as sphinx needs both extra enabled.

Maybe its a bug in the poetry build backend, because setuptools makes different METADATA Requires-Dists

Worth reporting it to them, but I think is valid to expect if something generates such to behave as my previous paragraph here describes. Can you add a test for it?



def test_load_dependency_poetry_many_extra() -> None:
requires = [Requirement('filelock<4.0.0,>=3.9.0; extra == "extras1" or extra == "extras2"')]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of naming it poetry let's just say it extra_or 👍 And lets add an extra_and variant please?

93578237 and others added 2 commits December 30, 2022 16:22
Signed-off-by: Bernát Gábor <gaborjbernat@gmail.com>
Copy link
Copy Markdown
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's ship as is and do the and handling as a follow-up.

@gaborbernat gaborbernat merged commit 196b20d into tox-dev:main Dec 31, 2022
@93578237
Copy link
Copy Markdown
Contributor Author

93578237 commented Jan 1, 2023

@gaborbernat How many logic markers should tox handle? One?

descope bot referenced this pull request in descope/django-descope Jan 14, 2023
This PR contains the following updates:

| Package | Type | Update | Change | Pending |
|---|---|---|---|---|
| [tox](https://togithub.com/tox-dev/tox)
([changelog](https://tox.wiki/en/latest/changelog.html)) | dev | patch |
`4.1.1` -> `4.1.2` | `4.2.8` (+9) |

---

### Release Notes

<details>
<summary>tox-dev/tox</summary>

### [`v4.1.2`](https://togithub.com/tox-dev/tox/releases/tag/4.1.2)

[Compare Source](https://togithub.com/tox-dev/tox/compare/4.1.1...4.1.2)

#### What's Changed

- Fix --skip-missing-interpreters by
[@&#8203;q0w](https://togithub.com/q0w) in
[https://github.com/tox-dev/tox/pull/2793](https://togithub.com/tox-dev/tox/pull/2793)
- Show installed packages after setup in CI envs by
[@&#8203;gaborbernat](https://togithub.com/gaborbernat) in
[https://github.com/tox-dev/tox/pull/2794](https://togithub.com/tox-dev/tox/pull/2794)
- Fix extracting extras from markers with many extras by
[@&#8203;q0w](https://togithub.com/q0w) in
[https://github.com/tox-dev/tox/pull/2792](https://togithub.com/tox-dev/tox/pull/2792)

**Full Changelog**: tox-dev/tox@4.1.1...4.1.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC45OS4yIiwidXBkYXRlZEluVmVyIjoiMzQuOTkuMiJ9-->

Co-authored-by: descope[bot] <descope[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants