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

Use conda.base.constants.KNOWN_SUBDIRS for setting up selectors #5009

Merged
merged 15 commits into from
Oct 16, 2023

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Sep 22, 2023

Description

Refactor conda_build.metadata.get_selectors to use conda.base.constants.KNOWN_SUBDIRS instead of duplicating this list in conda_build. Furthermore, abstract the selector generation so theres less effort to add new OS/Archs in the future.

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 Sep 22, 2023
@isuruf isuruf marked this pull request as ready for review September 23, 2023 08:30
@isuruf
Copy link
Contributor Author

isuruf commented Sep 23, 2023

pre-commit.ci autofix

@isuruf isuruf requested a review from a team September 24, 2023 22:43
@JeanChristopheMorinPerso
Copy link
Contributor

I see #5024. I had a little fear that we could loose some selectors by accident since the get_selectors function doesn't have any unit test.

I think the last thing would probably be to document that the supported subdirs come from conda. We might also want to go over https://docs.conda.io/projects/conda-build/en/stable/resources/define-metadata.html#preprocessing-selectors and update and add the supported selectors?

@jaimergp
Copy link
Contributor

jaimergp commented Oct 9, 2023

update and add the supported selectors?

Yes, I think that table is missing wasi, emscriptem and other recently added architectures. I wonder how tricky would be to autogenerate it like it's done in conda/conda for some parts.

@kenodegard
Copy link
Contributor

@isuruf I've gone ahead and merged the latest changes from main into this branch since #5024 has been merged

conda_build/metadata.py Outdated Show resolved Hide resolved
tests/test_metadata.py Outdated Show resolved Hide resolved
tests/test_metadata.py Outdated Show resolved Hide resolved
Co-authored-by: Isuru Fernando <isuruf@gmail.com>
@kenodegard kenodegard merged commit d480c32 into conda:main Oct 16, 2023
25 checks passed
@jezdez jezdez added this to the 3.28.0 milestone Nov 13, 2023
@kenodegard kenodegard mentioned this pull request Dec 1, 2023
67 tasks
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
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants