-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Infer multi-file binaries like src/bin/server/main.rs
by convention
#4214
Changes from 2 commits
10661f3
5d12c5b
1a08d2c
8833f3d
f08a9d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,7 @@ impl Layout { | |
|
||
try_add_file(&mut bins, root_path.join("src").join("main.rs")); | ||
try_add_files(&mut bins, root_path.join("src").join("bin")); | ||
try_add_mains_from_dirs(&mut bins, root_path.join("src").join("bin")); | ||
|
||
try_add_files(&mut examples, root_path.join("examples")); | ||
|
||
|
@@ -74,6 +75,25 @@ fn try_add_file(files: &mut Vec<PathBuf>, file: PathBuf) { | |
files.push(file); | ||
} | ||
} | ||
|
||
// Add directories form src/bin which contain main.rs file | ||
fn try_add_mains_from_dirs(files: &mut Vec<PathBuf>, root: PathBuf) { | ||
if let Ok(new) = fs::read_dir(&root) { | ||
let new: Vec<PathBuf> = new.filter_map(|i| i.ok()) | ||
// Filter only directories | ||
.filter(|i| { | ||
i.file_type().map(|f| f.is_dir()).unwrap_or(false) | ||
// Convert DirEntry into PathBuf and append "main.rs" | ||
}).map(|i| { | ||
i.path().join("main.rs") | ||
// Filter only directories where main.rs is present | ||
}).filter(|f| { | ||
f.as_path().exists() | ||
}).collect(); | ||
files.extend(new); | ||
} | ||
} | ||
|
||
fn try_add_files(files: &mut Vec<PathBuf>, root: PathBuf) { | ||
if let Ok(new) = fs::read_dir(&root) { | ||
files.extend(new.filter_map(|dir| { | ||
|
@@ -505,7 +525,28 @@ fn inferred_bin_targets(name: &str, layout: &Layout) -> Vec<TomlTarget> { | |
*bin == layout.root.join("src").join("main.rs") { | ||
Some(name.to_string()) | ||
} else { | ||
bin.file_stem().and_then(|s| s.to_str()).map(|f| f.to_string()) | ||
// bin is either a source file or a directory with main.rs inside. | ||
if bin.ends_with("main.rs") { | ||
if let Some(parent) = bin.parent() { | ||
// if the path ends with main.rs it is probably a directory, but it can also be | ||
// a file directly inside src/bin | ||
if parent.ends_with("src/bin") { | ||
// This would always return name "main" | ||
// Fixme: Is this what we want? based on what @matklad said, I don't think so | ||
// bin.file_stem().and_then(|s| s.to_str()).map(|f| f.to_string()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is preexisting behavior, so we should leave it as is to preserve backwards compatibility. And I am not actually sure that this is wrong in the first place, it's just curious :) |
||
|
||
// This seems to be the right solution based on the inferred_bin_paths function | ||
Some(name.to_string()) | ||
} else { | ||
parent.file_stem().and_then(|s| s.to_str()).map(|f| f.to_string()) | ||
} | ||
} else { | ||
None | ||
} | ||
} else { | ||
// regular case, just a file in the bin directory | ||
bin.file_stem().and_then(|s| s.to_str()).map(|f| f.to_string()) | ||
} | ||
}; | ||
|
||
name.map(|name| { | ||
|
@@ -1444,7 +1485,7 @@ fn inferred_bin_path(bin: &TomlBinTarget, | |
package_root: &Path, | ||
bin_len: usize) -> PathBuf { | ||
// here we have a single bin, so it may be located in src/main.rs, src/foo.rs, | ||
// srb/bin/foo.rs or src/bin/main.rs | ||
// src/bin/foo.rs, src/bin/foo/main.rs or src/bin/main.rs | ||
if bin_len == 1 { | ||
let path = Path::new("src").join(&format!("main.rs")); | ||
if package_root.join(&path).exists() { | ||
|
@@ -1463,6 +1504,12 @@ fn inferred_bin_path(bin: &TomlBinTarget, | |
return path.to_path_buf() | ||
} | ||
|
||
// check for the case where src/bin/foo/main.rs is present | ||
let path = Path::new("src").join("bin").join(bin.name()).join(&format!("main.rs")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last bit could be |
||
if package_root.join(&path).exists() { | ||
return path.to_path_buf() | ||
} | ||
|
||
return Path::new("src").join("bin").join(&format!("main.rs")).to_path_buf() | ||
} | ||
|
||
|
@@ -1472,6 +1519,12 @@ fn inferred_bin_path(bin: &TomlBinTarget, | |
return path.to_path_buf() | ||
} | ||
|
||
// we can also have src/bin/foo/main.rs, but the former one is preferred | ||
let path = Path::new("src").join("bin").join(bin.name()).join(&format!("main.rs")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
if package_root.join(&path).exists() { | ||
return path.to_path_buf() | ||
} | ||
|
||
if !has_lib { | ||
let path = Path::new("src").join(&format!("{}.rs", bin.name())); | ||
if package_root.join(&path).exists() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3271,3 +3271,64 @@ fn no_bin_in_src_with_lib() { | |
execs().with_status(101) | ||
.with_stderr_contains(r#"[ERROR] couldn't read "[..]main.rs"[..]"#)); | ||
} | ||
|
||
|
||
#[test] | ||
fn dirs_in_bin_dir_with_main_rs() { | ||
let p = project("foo") | ||
.file("Cargo.toml", r#" | ||
[package] | ||
name = "foo" | ||
version = "0.1.0" | ||
authors = [] | ||
"#) | ||
.file("src/main.rs", "fn main() {}") | ||
.file("src/bin/bar.rs", "fn main() {}") | ||
.file("src/bin/bar2.rs", "fn main() {}") | ||
.file("src/bin/bar3/main.rs", "fn main() {}") | ||
.file("src/bin/bar4/main.rs", "fn main() {}"); | ||
|
||
assert_that(p.cargo_process("build"), execs().with_status(0)); | ||
assert_that(&p.bin("foo"), existing_file()); | ||
assert_that(&p.bin("bar"), existing_file()); | ||
assert_that(&p.bin("bar2"), existing_file()); | ||
assert_that(&p.bin("bar3"), existing_file()); | ||
assert_that(&p.bin("bar4"), existing_file()); | ||
} | ||
|
||
#[test] | ||
fn dir_and_file_with_same_name_in_bin() { | ||
// this should fail, because we have two binaries with the same name | ||
let p = project("bar") | ||
.file("Cargo.toml", r#" | ||
[package] | ||
name = "bar" | ||
version = "0.1.0" | ||
authors = [] | ||
"#) | ||
.file("src/main.rs", "fn main() {}") | ||
.file("src/bin/foo.rs", "fn main() {}") | ||
.file("src/bin/foo/main.rs", "fn main() {}"); | ||
|
||
// TODO: This should output the error from toml.rs:756 | ||
assert_that(p.cargo_process("build"), execs().with_status(101)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right! You could use |
||
} | ||
|
||
#[test] | ||
fn inferred_path_in_src_bin_foo() { | ||
let p = project("foo") | ||
.file("Cargo.toml", r#" | ||
[package] | ||
name = "foo" | ||
version = "0.1.0" | ||
authors = [] | ||
|
||
[[bin]] | ||
name = "bar" | ||
# Note, no `path` key! | ||
"#) | ||
.file("src/bin/bar/main.rs", "fn main() {}"); | ||
|
||
assert_that(p.cargo_process("build"), execs().with_status(0)); | ||
assert_that(&p.bin("bar"), existing_file()); | ||
} |
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.
Let's change this to
bin.ends_with("main.rs") && !bin.ends_with("src/bin/main.rs")
so that we oneif
inside.