-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Validate target source paths before compilation with clearer errors #16338
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
Validate target source paths before compilation with clearer errors #16338
Conversation
6e604ae to
3200b5b
Compare
|
Feel free to edit your commits for how they should be merged. |
e52ea15 to
5e9e6cb
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Thanks for the review! Also could you rerun the CI workflow because it failed with this error: https://github.com/rust-lang/cargo/actions/runs/19952419092/job/57215065207#annotation:2:61
|
FYI it would be a big help for reviewing if the commits were cleaned up, especially if passing tests are added in their own commit (showing the old behavior) and then the feature work happens in the follow up that updates the tests so they still pass. |
|
Sure, i'll clean up the commit history |
5e9e6cb to
b949d9a
Compare
b949d9a to
585b8a9
Compare
src/cargo/ops/cargo_compile/mod.rs
Outdated
| None | ||
| } | ||
| } | ||
| _ => None, |
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.
Can we explicitly enumerate all TargetKinds so the compiler can remind us to re-evaluate this if a new variant is added?
|
Thanks, this is looking great! Just adjusting the |
585b8a9 to
f203f09
Compare
What does this PR try to resolve?
Closes #10173
This PR adds early validation of target source paths with clearer error messages.
For context, I tried to solve this issue before ( #16329 ) but ended up making an oversight, I would recommend reading this to understand the issues faced in resolving this.
So what changed in this PR?
TargetUnitsare validated early on in the compilation pipeline when theBuildContextis generated here. This is right after command line arguments/manifest targets are parsed and just before the build tasks are spawned.main.rs/lib.rsdepending on the Target kind: In this case ahelp:message is emitted.Doing this also means that Cargo validates each root target's path that is requested to be compiled before spawning any build tasks.
This PR enforces the invariant: No compilation occurs before validating every required target's source path.
I'm not sure if this should be expected behaviour and I'd love to hear from others on this.