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

Fix support for pyproject.toml dependencies with multiple selectors #597

Merged

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Dec 29, 2024

Description

It is common for the same dependency to have multiple selectors, like the following:

    - pandas >=1.5.3,<2.2  # [py<39]
    - pandas >=2.1.2,<2.2  # [py>=39]

This merge attempts to fully support this capability.

In several places in the code where dependencies are merged, care is now taken to allow multiple versions of the same package as long as they come from the same source. This happens when merging toml and setup dependencies into sdist dependencies, and again when merging sdist dependencies with pypi dependencies. It also takes place when removing redundancies later in the process of building a recipe.

In addition, because some strategies (namely parsing of poetry dependencies) add selectors early in the process, care has been taken not to drop selectors that may mistakenly be identified as tags or comments.

Finally, a fix has been added to identify that a package is_arch when it has selectors, just as it is identified as is_arch when "extras" are detected.

Four new pypi tests have been added and new testing has been added to the existing test_run_requirements_sdist test to ensure that multiple selectors for the same package are handled correctly.

We want to prioritize toml deps over setup deps.  We don't want
to remove redundant packages in toml deps because these can
have corresponding, different selectors.
We want to prioritize sdist deps over pypi deps.  We don't want
to remove redundant packages in sdist deps because these can
have corresponding, different selectors.

The progressbar has become a bit more complicated because we
iterate separately over sdist and pypi dependencies.
While `is_arch` gets set to true if there are extras, we also
want to do so if selectors have already been defined.
When removing duplicate packages, we want to consider different
selectors as unique.  Thus, we only remove dependencies if they
have the same package name *and* the same selector.
Previously, selectors were getting dropped as tags and comments
These tests are designed to make sure multiple selectors for the
same package are preserved correctly at every stage of the process
of merging dependencies from different sources.

When parsing poetry dependencies, we introduce selectors early
in the process, whereas for other approaches, we retain "extras"
(separated by semicolons) for much of the merging process. Packages
employing each strategy are used in the added tests.
@xylar xylar requested a review from a team as a code owner December 29, 2024 14:56
@xylar
Copy link
Contributor Author

xylar commented Dec 29, 2024

@marcelotrevisani, as always, I would very much appreciate your input and review.

@lorepirri, this work builds on yours in #592. I would very much appreciate your feedback as well.

I have put several days into learning my way around grayskull (and souschef), but I have to return to my normal work soon so I don't know how much time I'll be able to put into this in the next while. That's just to say that, if I'm slow to respond, don't take that as a lack of interest.

Copy link
Member

@marcelotrevisani marcelotrevisani left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks again! :)

@marcelotrevisani
Copy link
Member

@marcelotrevisani, as always, I would very much appreciate your input and review.

@lorepirri, this work builds on yours in #592. I would very much appreciate your feedback as well.

I have put several days into learning my way around grayskull (and souschef), but I have to return to my normal work soon so I don't know how much time I'll be able to put into this in the next while. That's just to say that, if I'm slow to respond, don't take that as a lack of interest.

the modification looks sensible to me, I believe we can integrate it, thanks!

Don't worry, I am not taking that as a lack of interest at all, I am very glad to the insights and inputs you are bringing to gray skull!

@marcelotrevisani marcelotrevisani merged commit f3e910f into conda:main Dec 30, 2024
8 checks passed
@xylar xylar deleted the fix-toml-deps-with-multiple-selectors branch December 30, 2024 19:07
@xylar
Copy link
Contributor Author

xylar commented Dec 30, 2024

@marcelotrevisani, wonderful, thanks so much. I'd like to follow up at some point when I have time with some updates to --strict-conda-forge but I think that's also a project for when I have a few free days.

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 this pull request may close these issues.

2 participants