-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fail in case nothing to run was found #43145
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
src/bootstrap/step.rs
Outdated
.map(|priority| (rule, priority)) | ||
} | ||
}).collect(); | ||
|
||
if rules.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this is good, but I sort of expected we'd go a step further and check that every path matched with a rule... I might be wrong, but I think that should always be the case. cc @alexcrichton -- can you confirm we should match all paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, seems fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of paths and a lot of them aren't doing anything, so I'm sure this is a good idea...
prepare:
$(Q)$(BOOTSTRAP) build nonexistent/path/to/trigger/cargo/metadata Please either special-case " |
2bab72a
to
0cf8f85
Compare
Fixed |
Ping in here. |
Alright, I'll accept that stopgap for now, though we should look into a better solution. It's fine for the time being I think. @bors r+ rollup |
📌 Commit 0cf8f85 has been approved by |
…ng, r=Mark-Simulacrum fail in case nothing to run was found Fixes rust-lang#43121. r? @Mark-Simulacrum
☔ The latest upstream changes (presumably #43246) made this pull request unmergeable. Please resolve the merge conflicts. |
Bors? You okay? @bors r- |
Fixes #43121.
r? @Mark-Simulacrum