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

improve error reporting when feature not found in activated_features #14647

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

elchukc
Copy link
Contributor

@elchukc elchukc commented Oct 5, 2024

Pulls the error message refactor out of #14593 (originally #13207) to improve error reporting when we fail to get the list of activated features enabled for the given package. It now fully lists the activated_features hashmap keys too.

From the original author:

I moved activated_features_int into activated_features and activated_features_unverified as I was concerned of the performance cost of generating the full error when its not a fatal error and may occur many times.

Old vs new error message:

- activated_features for invalid package: features did not find PackageId { name: "bindep", version: "0.0.0", source: "[ROOT]/foo/bindep" } NormalOrDev
+ did not find features for (PackageId { name: "bindep", version: "0.0.0", source: "[ROOT]/foo/bindep" }, NormalOrDev) within activated_features:
+ [
+     (
+         PackageId {
+             name: "bindep",
+             version: "0.0.0",
+             source: "[ROOT]/foo/bindep",
+         },
+         ArtifactDep(
+             CompileTarget {
+                 name: "[ALT_TARGET]",
+             },
+         ),
+     ),
+     (
+         PackageId {
+             name: "foo",
+             version: "0.0.0",
+             source: "[ROOT]/foo",
+         },
+         NormalOrDev,
+     ),
+ ]

r? weihanglo

@rustbot
Copy link
Collaborator

rustbot commented Oct 5, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-features2 Area: issues specifically related to the v2 feature resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 5, 2024
@elchukc elchukc force-pushed the refactor_pkg_activated_features branch from af8b11f to 4623a71 Compare October 5, 2024 19:45
@@ -319,8 +319,32 @@ impl ResolvedFeatures {
pkg_id: PackageId,
features_for: FeaturesFor,
) -> Vec<InternedString> {
self.activated_features_int(pkg_id, features_for)
Copy link
Member

Choose a reason for hiding this comment

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

What about reusing and calling the unverified variant in this function. If none then panic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@elchukc elchukc force-pushed the refactor_pkg_activated_features branch from 4623a71 to 1623c41 Compare October 5, 2024 21:11
@weihanglo
Copy link
Member

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 5, 2024

📌 Commit 1623c41 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 5, 2024
@bors
Copy link
Contributor

bors commented Oct 5, 2024

⌛ Testing commit 1623c41 with merge ab361f2...

@bors
Copy link
Contributor

bors commented Oct 5, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing ab361f2 to master...

@bors bors merged commit ab361f2 into rust-lang:master Oct 5, 2024
22 checks passed
@elchukc elchukc deleted the refactor_pkg_activated_features branch October 5, 2024 22:31
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 9, 2024
Update cargo

8 commits in ad074abe3a18ce8444c06f962ceecfd056acfc73..15fbd2f607d4defc87053b8b76bf5038f2483cf4
2024-10-04 18:18:15 +0000 to 2024-10-08 21:08:11 +0000
- initial version of checksum based freshness (rust-lang/cargo#14137)
- feat: Add custom completer for completing registry name (rust-lang/cargo#14656)
- Document build-plan as being deprecated (rust-lang/cargo#14657)
- fix(complete): Don't complete files for any value (rust-lang/cargo#14653)
- Add more SAT resolver tests (rust-lang/cargo#14614)
- fix: avoid inserting duplicate `dylib_path_envvar` when calling `cargo run` recursively (rust-lang/cargo#14464)
- chore(deps): bump gix-path from 0.10.9 to 0.10.11 (rust-lang/cargo#14489)
- improve error reporting when feature not found in `activated_features` (rust-lang/cargo#14647)

---

This also adds three license exceptions to Cargo.

* arrayref — BSD-2-Clause
* blake3 — CC0-1.0 OR Apache-2.0 OR Apache-2.0 WITH LLVM-exception
* constant_time_eq — CC0-1.0 OR MIT-0 OR Apache-2.0

These exceptions were added to rustc in rust-lang#126930, so should be fine for Cargo as well.
@rustbot rustbot added this to the 1.83.0 milestone Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-features2 Area: issues specifically related to the v2 feature resolver S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants