Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,17 @@ fn to_example_targets(
let mut result = Vec::new();
for toml in targets {
let path = package_root.join(&toml.path.as_ref().expect("previously normalized").0);
let target_name = name_or_panic(&toml);

validate_target_path_as_source_file(&path, target_name, TARGET_KIND_HUMAN_EXAMPLE)?;
Comment on lines +448 to +450
Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned on #10173, we shouldn't limit ourselves to examples


let crate_types = match toml.crate_types() {
Some(kinds) => kinds.iter().map(|s| s.into()).collect(),
None => Vec::new(),
};

let mut target = Target::example_target(
name_or_panic(&toml),
target_name,
crate_types,
path,
toml.required_features.clone(),
Expand Down Expand Up @@ -928,6 +932,54 @@ fn validate_unique_build_scripts(scripts: &[String]) -> CargoResult<()> {
Ok(())
}

// Will check if `path` specified for a target is a valid source file
fn validate_target_path_as_source_file(
path: &Path,
target_name: &str,
target_kind: &str,
) -> CargoResult<()> {
if !path.exists() {
anyhow::bail!(
"can't find {} `{}` at path `{}`",
target_kind,
target_name,
path.display()
);
}
Comment on lines +941 to +948
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have a test for this case as well. This might also represent a breaking change.


// If the path is likely a crate, then suggest setting the path to the entrypoint
if path.is_dir() {
anyhow::bail!(
"path `{}` for {} `{}` is a directory, but a source file was expected.\n\
Consider setting the path to the intended entrypoint file instead: `{}/.../main.rs`",
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we talked the directory to report concrete lib.rs and main.rss they could point to?

Copy link
Contributor

Choose a reason for hiding this comment

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

A message like this should start with help: and start with a lowercase letter per rustc's style guide

Copy link
Contributor

Choose a reason for hiding this comment

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

While Consider isn't a question, I feel like it falls into a similar category as questions in the rustc style guide

path.display(),
target_kind,
target_name,
path.display(),
);
}

// Validate if the path has a .rs extension
if let Some(extension) = path.extension() {
if extension != "rs" {
anyhow::bail!(
"{} `{}` has path `{}` which does not have a `.rs` extension",
target_kind,
target_name,
path.display()
);
}
} else {
anyhow::bail!(
"{} `{}` has path `{}` which has no file extension (expected `.rs`)",
target_kind,
target_name,
path.display()
);
}
Comment on lines +962 to +979
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens today when you have an extension-less file or a non-.rs extension? If it works, then we need to have the conversation about what the intended behavior should be, whether that is a bug or not (and should likely be moved out of this PR to not block on that). If not, we should have tests to compare with to see how the error changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

today it works if the example and its path is explicitly mentioned in Cargo.toml. It however does not work if there is an extensionless file, say fuzz in the examples directory and i try cargo check --example fuzz which on hindsight makes sense.

Ok(())
}

fn configure(
toml: &TomlTarget,
target: &mut Target,
Expand Down
39 changes: 39 additions & 0 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3755,3 +3755,42 @@ please set bin.path in Cargo.toml
"#]])
.run();
}

#[cargo_test]
fn example_target_path_validation() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2024"

[[example]]
name = "fuzz"
path = "examples/fuzz"
"#,
)
.file("src/main.rs", "")
.file("examples/fuzz/src/main.rs", "")
.file(
"examples/fuzz/Cargo.toml",
r#"
[package]
name = "fuzz"
version = "0.1.0"
edition = "2024"
"#,
)
.build();
p.cargo("check --example fuzz")
.with_status(101)
.with_stderr_data(str![[r#"[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`

Caused by:
path `[ROOT]/foo/examples/fuzz` for example `fuzz` is a directory, but a source file was expected.
Consider setting the path to the intended entrypoint file instead: `[ROOT]/foo/examples/fuzz/.../main.rs`

"#]]).run();
}
Loading