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

Add python (et al.) to used variants even if only referenced via special selectors #5139

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mbargull
Copy link
Member

@mbargull mbargull commented Jan 16, 2024

Description

Package-dependent special selectors like py and similar are hard-coded to be always available no matter if python etc. are actually recognized as used variant variables.

This is problematic since the values of those selectors are not well-defined in case the associated package is not pinned down as variant.
E.g., for a list of variants with different python versions, the py selector can be any of those versions and is in fact non-deterministically chosen in the current implementation.

We have instances in the ecosystem where a multi-output recipe does not have a dependency on python but some skip: true # [py ...] selector is used, as in

package:
  name: top-level
  version: 1.0
build:
  skip: true  # [py<39]
outputs:
  - name: sub-package
    requirements:
      host:
        - python

; e.g., see conda-forge/conda-smithy#1782 and the linked recipes in there.

There are multiple ways to argue about this:

  1. The recipe could be seen as deficient since it uses py without python at the top level.
  2. The py (etc.) selectors should only be defined if the corresponding packages are used.

Re. 1.: Yes, the recipe could be amended to have skip: true # [python and py<39] or move the skip line to each output.
Regardless, having an implicit non-deterministic behavior is something we should avoid in any case.

Re. 2.: If the selectors would not be defined, the rendering could gracefully fail with an error.
Changing the code to do this could be difficult since the rendering of the current recipe format is rather convoluted.
Plus, there could be recipes that expect those selectors to always be defined, i.e., it might be more disruptive regarding backwards compatibility to remove them.

The alternative, which is implemented here, is to pull in the packages into the "used variant variables" whenever such selectors are used.
This is also a breaking change (due to potentially introducing more variants), but less disruptive as the selectors' removals but possibly (e.g., for the top-level skip: true # [py<39] example above) even what the user expected in the first place.

N.B.: Personally, I'd consider those selectors as unnecessary tech-debt and hope the ecosystem can move off of their usage with the new recipe format which is currently worked on in the rattler-build project.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 16, 2024
@mbargull mbargull force-pushed the special-selector-variants branch 2 times, most recently from faeaf00 to ba7463e Compare January 16, 2024 13:32
@mbargull mbargull force-pushed the special-selector-variants branch 3 times, most recently from f5eab4d to d424837 Compare January 16, 2024 14:52
@mbargull mbargull marked this pull request as ready for review January 16, 2024 14:52
@mbargull mbargull force-pushed the special-selector-variants branch from d424837 to bfd39c5 Compare January 16, 2024 19:39
@mbargull
Copy link
Member Author

  • Add / update outdated documentation?

I didn't really see a place where selector<->variant interactions are addressed.
If anyone has suggestions for a doc addition, feel free to push to this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants