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

Catch PackageNotFoundError for DependencyNeedsBuildingError #5257

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbargull
Copy link
Member

Description

Partially reverts
7acbbf2

Fixes gh-5256.

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?

Partially reverts
conda@7acbbf2

Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Mar 25, 2024
@mbargull
Copy link
Member Author

Needs regression test and news entry.

except (
NoPackagesFoundError,
UnsatisfiableError,
PackagesNotFoundError,
Copy link
Member Author

Choose a reason for hiding this comment

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

@kenodegard, why did we had this change in 7acbbf2 ?
Re-adding PackagesNotFoundError fixes gh-5256. Am I missing something?

Copy link
Contributor

@kenodegard kenodegard Mar 26, 2024

Choose a reason for hiding this comment

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

I had added it in the prior commit (7539a73) as I was trying to debug a weird error (see 7acbbf2#r1517170310)

Since the fix had nothing to do with catching PackagesNotFoundError I removed it again

See the full diff for conda_buid.environ https://github.com/conda/conda-build/pull/5203/files#diff-3ba52da5dde963be464da17aa3a40b91541d9709735c97fd03b59d3a0fb54a78R32-R55 doesn't include PackagesNotFoundError before or after

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I landed at 7acbbf2 as the first commit to show the regression but didn't realize that was an intermediate change. Thanks, that's very helpful.
Since 4da3bce works, there must be something in ca792ee (with the get_install_actions name dance applied) that causes it, I think. Let me dig into those changes a bit..

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I guess 24.1 didn't show this because d7a9751 broke the "get_install_actions is in the last N entries on the stack" assumption of conda-libmamba-solver.
Meaning, conda-build<24.1 also didn't handle these errors correctly and in fact conda-build=24.1 broke conda-libmamba-solver setting context.offline...

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to adjust anything in conda-libmamba-solver?

Copy link

codspeed-hq bot commented Mar 25, 2024

CodSpeed Performance Report

Merging #5257 will not alter performance

Comparing mbargull:fix-5256 (06b806e) with main (15f3323)

Summary

✅ 3 untouched benchmarks

@@ -965,7 +966,11 @@ def get_install_actions(
with capture():
try:
precs = _install_actions(prefix, index, specs)["LINK"]
except (NoPackagesFoundError, UnsatisfiableError) as exc:
except (
NoPackagesFoundError,
Copy link
Member Author

Choose a reason for hiding this comment

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

We may want to use the non-aliased ResolvePackageNotFound here for clarification.
(conda.exceptions/conda.exports sets NoPackagesFound = NoPackagesFoundError = ResolvePackageNotFound.)

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.

PackagesNotFound not converted to DependencyNeedsBuildingError
4 participants