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.

Sign up for GitHub

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

Jump to bottom

CI adjustments for conda-libmamba-solver as default #5059

Merged
merged 6 commits into from
Nov 17, 2023
Merged

CI adjustments for conda-libmamba-solver as default #5059

merged 6 commits into from
Nov 17, 2023

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Nov 7, 2023 •
edited
Loading

Description

conda 23.10.0 requires conda-libmamba-solver and has solver=libmamba as default, so conda-build's CI is going to be using it now.

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 Nov 7, 2023
mbargull
mbargull previously approved these changes Nov 10, 2023
View reviewed changes
Copy link
Member

@mbargull mbargull left a comment

Choose a reason for hiding this comment

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

LGTM

@mbargull
Copy link
Member

@jaimergp, this is still marked as a draft.
Any reason not to merge?

@jaimergp
Copy link
Contributor Author

Just the pre-commit and missing news. I'll commit them now. Thanks for the reminder!

mbargull
mbargull previously approved these changes Nov 10, 2023
View reviewed changes
jezdez
jezdez previously approved these changes Nov 13, 2023
View reviewed changes
@jaimergp
Copy link
Contributor Author

Errors are related to conda-libmamba-solver itself, probably to be fixed by conda/conda-libmamba-solver#365.

@jaimergp jaimergp mentioned this pull request Nov 14, 2023
Merged
@jaimergp jaimergp closed this Nov 16, 2023
@jaimergp jaimergp reopened this Nov 16, 2023
@jaimergp
Copy link
Contributor Author

Let's see how conda-libmamba-solver 23.11.1 does now :)

@kenodegard kenodegard dismissed stale reviews from mbargull and jezdez via 031dac4 November 17, 2023 05:00
@kenodegard
Copy link
Contributor

kenodegard commented Nov 17, 2023 •
edited
Loading

@jaimergp no idea why tests/cli/test_main_build.py::test_build_output_folder and tests/cli/test_main_build.py::test_build_source are failing (see https://github.com/conda/conda-build/actions/runs/6897842353/job/18770690271?pr=5059#step:8:3164), attempting to fix it by switching to tmp_path, see 031dac4

@jaimergp jaimergp mentioned this pull request Nov 17, 2023
Closed
2 tasks
@jaimergp
Copy link
Contributor Author

Me neither @kenodegard 🤷 Thanks for the help though! I'm going to be the solver-related issue tests/test_api_build.py::test_build_msvc_compiler[14.0] - KeyError: Dist(channel='local', dist_name='conda-build-test-build-msvc-compiler-1.0-0', name='conda-build-test-build-msvc-compiler', fmt='.tar.bz2', version='1.0', build_string='0', build_number=0, base_url=None, platform=None) in conda/conda-libmamba-solver#386. The other things might be artifacts of the solver now being faster, which means things need to be recreated more quickly than before? Idk. If feels like a race condition.

@mbargull
Copy link
Member

@jaimergp no idea why tests/cli/test_main_build.py::test_build_output_folder and tests/cli/test_main_build.py::test_build_source are failing (see https://github.com/conda/conda-build/actions/runs/6897842353/job/18770690271?pr=5059#step:8:3164), attempting to fix it by switching to tmp_path, see 031dac4

This is most likely fixed by gh-5068 .
What we have here is that the croot is being modified and/or removed by concurrently running tests which is why conda-index fails.

@mbargull mbargull mentioned this pull request Nov 17, 2023
Merged
3 tasks
@mbargull
Copy link
Member

It would be good if we could merge this PR and gh-5068 to remove some of the failing test clutter.
Further fixes should be done/investigated in subsequent PRs.
@jaimergp, mark as "Ready for review"?

@mbargull
Copy link
Member

@jaimergp no idea why tests/cli/test_main_build.py::test_build_output_folder and tests/cli/test_main_build.py::test_build_source are failing (see https://github.com/conda/conda-build/actions/runs/6897842353/job/18770690271?pr=5059#step:8:3164), attempting to fix it by switching to tmp_path, see 031dac4

This is most likely fixed by gh-5068 . What we have here is that the croot is being modified and/or removed by concurrently running tests which is why conda-index fails.

Ah, taking another look at those 2 tests, I see that they use --croot explicitly; so those might not necessarily be affected by gh-5068. (We should merge that in nonetheless.)

@jaimergp jaimergp marked this pull request as ready for review November 17, 2023 11:01
@jaimergp
Copy link
Contributor Author

Agreed @mbargull. Thanks!

@jezdez
Copy link
Member

jezdez commented Nov 17, 2023

@mbargull Just approved, but I wanted to clarify that it's a-ok to merge given the new build tools team effort. 🚀

@mbargull
Copy link
Member

@mbargull Just approved, but I wanted to clarify that it's a-ok to merge given the new build tools team effort. 🚀

❤️ Thanks.
(I've been out for so long, I'm only slowly getting the lay of the land of the current state of who/what/when in Conda land ;).)

dholth
dholth reviewed Nov 17, 2023
View reviewed changes
tests/test_api_build.py
@@ -123,6 +124,9 @@ def test_recipe_builds(
# ``source_setup_py_data_subdir`` reproduces the problem.
if recipe.name == "source_setup_py_data_subdir":
pytest.xfail("Issue related to #3754 on conda-build.")
elif recipe.name == "unicode_all_over" and context.solver == "libmamba":
pytest.xfail("Unicode package names not supported in libmamba.")
Copy link
Contributor

@dholth dholth Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's too bad.

Copy link
Member

Choose a reason for hiding this comment

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

libmamba doesn't support unicode package names IIUC, no need to test for it.

@kenodegard
Copy link
Contributor

Merged in #5068, any remaining changes necessary to get everything back to 🟢 should be added to this PR

@kenodegard kenodegard merged commit 05cc56a into conda:main Nov 17, 2023
24 checks passed
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Nov 17, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dholth dholth dholth left review comments

@jezdez jezdez jezdez approved these changes

@kenodegard kenodegard kenodegard approved these changes

@mbargull mbargull mbargull left review comments

Assignees
No one assigned
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
🔎 Review
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jaimergp @mbargull @kenodegard @jezdez @dholth @conda-bot