From 3f8fc0e66a3bd1dbd002f2cfda166752d30a8277 Mon Sep 17 00:00:00 2001 From: Alexey Chernyshov Date: Sat, 29 May 2021 21:24:27 +0300 Subject: [PATCH] respect user choice of lib/bin over heuristics --- src/bin/cargo/commands/init.rs | 4 +- src/bin/cargo/commands/new.rs | 4 +- src/cargo/ops/cargo_new.rs | 85 ++++++++++++++++++++++++++-------- tests/testsuite/init.rs | 65 ++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 23 deletions(-) diff --git a/src/bin/cargo/commands/init.rs b/src/bin/cargo/commands/init.rs index a9245c000c3..f3f1440d15b 100644 --- a/src/bin/cargo/commands/init.rs +++ b/src/bin/cargo/commands/init.rs @@ -14,9 +14,9 @@ pub fn cli() -> App { pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let opts = args.new_options(config)?; - ops::init(&opts, config)?; + let project_kind = ops::init(&opts, config)?; config .shell() - .status("Created", format!("{} package", opts.kind))?; + .status("Created", format!("{} package", project_kind))?; Ok(()) } diff --git a/src/bin/cargo/commands/new.rs b/src/bin/cargo/commands/new.rs index a1c64439981..8900cd35628 100644 --- a/src/bin/cargo/commands/new.rs +++ b/src/bin/cargo/commands/new.rs @@ -15,7 +15,7 @@ pub fn cli() -> App { pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let opts = args.new_options(config)?; - ops::new(&opts, config)?; + let project_kind = ops::new(&opts, config)?; let path = args.value_of("path").unwrap(); let package_name = if let Some(name) = args.value_of("name") { name @@ -24,7 +24,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { }; config.shell().status( "Created", - format!("{} `{}` package", opts.kind, package_name), + format!("{} `{}` package", project_kind, package_name), )?; Ok(()) } diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index f4bf9fdb26a..7116e4b0f81 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -62,12 +62,17 @@ pub struct NewOptions { pub enum NewProjectKind { Bin, Lib, + Auto, } impl NewProjectKind { fn is_bin(self) -> bool { self == NewProjectKind::Bin } + + fn is_auto(self) -> bool { + self == NewProjectKind::Auto + } } impl fmt::Display for NewProjectKind { @@ -75,6 +80,7 @@ impl fmt::Display for NewProjectKind { match *self { NewProjectKind::Bin => "binary (application)", NewProjectKind::Lib => "library", + NewProjectKind::Auto => "auto-select type", } .fmt(f) } @@ -109,8 +115,8 @@ impl NewOptions { let kind = match (bin, lib) { (true, true) => anyhow::bail!("can't specify both lib and binary outputs"), (false, true) => NewProjectKind::Lib, - // default to bin - (_, false) => NewProjectKind::Bin, + (true, false) => NewProjectKind::Bin, + (false, false) => NewProjectKind::Auto, }; let opts = NewOptions { @@ -389,7 +395,26 @@ fn plan_new_source_file(bin: bool, package_name: String) -> SourceFileInformatio } } -pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> { +fn calculate_new_project_kind( + requested_kind: NewProjectKind, + found_files: &Vec, +) -> NewProjectKind { + let bin_file = found_files.iter().find(|x| x.bin); + + let kind_from_files = if !found_files.is_empty() && bin_file.is_none() { + NewProjectKind::Lib + } else { + NewProjectKind::Bin + }; + + if requested_kind.is_auto() { + return kind_from_files; + } + + requested_kind +} + +pub fn new(opts: &NewOptions, config: &Config) -> CargoResult { let path = &opts.path; if path.exists() { anyhow::bail!( @@ -399,20 +424,22 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> { ) } + let kind = match opts.kind { + NewProjectKind::Bin => NewProjectKind::Bin, + NewProjectKind::Auto => NewProjectKind::Bin, + _ => NewProjectKind::Lib, + }; + let is_bin = kind.is_bin(); + let name = get_name(path, opts)?; - check_name( - name, - opts.name.is_none(), - opts.kind.is_bin(), - &mut config.shell(), - )?; + check_name(name, opts.name.is_none(), is_bin, &mut config.shell())?; let mkopts = MkOptions { version_control: opts.version_control, path, name, source_files: vec![plan_new_source_file(opts.kind.is_bin(), name.to_string())], - bin: opts.kind.is_bin(), + bin: is_bin, edition: opts.edition.as_deref(), registry: opts.registry.as_deref(), }; @@ -424,10 +451,10 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> { path.display() ) })?; - Ok(()) + Ok(kind) } -pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> { +pub fn init(opts: &NewOptions, config: &Config) -> CargoResult { // This is here just as a random location to exercise the internal error handling. if std::env::var_os("__CARGO_TEST_INTERNAL_ERROR").is_some() { return Err(crate::util::internal("internal error test")); @@ -445,14 +472,34 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> { detect_source_paths_and_types(path, name, &mut src_paths_types)?; + let kind = calculate_new_project_kind(opts.kind, &src_paths_types); + let has_bin = kind.is_bin(); + if src_paths_types.is_empty() { - src_paths_types.push(plan_new_source_file(opts.kind.is_bin(), name.to_string())); - } else { - // --bin option may be ignored if lib.rs or src/lib.rs present - // Maybe when doing `cargo init --bin` inside a library package stub, - // user may mean "initialize for library, but also add binary target" + src_paths_types.push(plan_new_source_file(has_bin, name.to_string())); + } else if src_paths_types.len() == 1 && !src_paths_types.iter().any(|x| x.bin == has_bin) { + // we've found the only file and it's not the type user wants. Change the type and warn + let file_type = if src_paths_types[0].bin { + NewProjectKind::Bin.to_string() + } else { + NewProjectKind::Lib.to_string() + }; + config.shell().warn(format!( + "file '{}' seems to be a {} file", + src_paths_types[0].relative_path, file_type + ))?; + src_paths_types[0].bin = has_bin + } else if src_paths_types.len() > 1 && !has_bin { + // We have found both lib and bin files and the user would like us to treat both as libs + anyhow::bail!( + "cannot have a package with \ + multiple libraries, \ + found both `{}` and `{}`", + src_paths_types[0].relative_path, + src_paths_types[1].relative_path + ) } - let has_bin = src_paths_types.iter().any(|x| x.bin); + check_name(name, opts.name.is_none(), has_bin, &mut config.shell())?; let mut version_control = opts.version_control; @@ -508,7 +555,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> { path.display() ) })?; - Ok(()) + Ok(kind) } /// IgnoreList diff --git a/tests/testsuite/init.rs b/tests/testsuite/init.rs index 4379f0530aa..85920d9ac50 100644 --- a/tests/testsuite/init.rs +++ b/tests/testsuite/init.rs @@ -598,3 +598,68 @@ mod tests { "# ); } + +#[cargo_test] +fn creates_binary_when_instructed_and_has_lib_file_no_warning() { + let path = paths::root().join("foo"); + fs::create_dir(&path).unwrap(); + fs::write(path.join("foo.rs"), "fn not_main() {}").unwrap(); + cargo_process("init --bin") + .cwd(&path) + .with_stderr( + "[WARNING] file 'foo.rs' seems to be a library file\n[CREATED] binary (application) package" + ) + .run(); + + let cargo_toml = fs::read_to_string(path.join("Cargo.toml")).unwrap(); + assert!(cargo_toml.contains("[[bin]]")); + assert!(!cargo_toml.contains("[lib]")); +} + +#[cargo_test] +fn creates_library_when_instructed_and_has_bin_file() { + let path = paths::root().join("foo"); + fs::create_dir(&path).unwrap(); + fs::write(path.join("foo.rs"), "fn main() {}").unwrap(); + cargo_process("init --lib") + .cwd(&path) + .with_stderr( + "[WARNING] file 'foo.rs' seems to be a binary (application) file\n[CREATED] library package" + ) + .run(); + + let cargo_toml = fs::read_to_string(path.join("Cargo.toml")).unwrap(); + assert!(!cargo_toml.contains("[[bin]]")); + assert!(cargo_toml.contains("[lib]")); +} + +#[cargo_test] +fn creates_binary_when_both_binlib_present() { + let path = paths::root().join("foo"); + fs::create_dir(&path).unwrap(); + fs::write(path.join("foo.rs"), "fn main() {}").unwrap(); + fs::write(path.join("lib.rs"), "fn notmain() {}").unwrap(); + cargo_process("init --bin") + .cwd(&path) + .with_stderr("[CREATED] binary (application) package") + .run(); + + let cargo_toml = fs::read_to_string(path.join("Cargo.toml")).unwrap(); + assert!(cargo_toml.contains("[[bin]]")); + assert!(cargo_toml.contains("[lib]")); +} + +#[cargo_test] +fn cant_create_library_when_both_binlib_present() { + let path = paths::root().join("foo"); + fs::create_dir(&path).unwrap(); + fs::write(path.join("foo.rs"), "fn main() {}").unwrap(); + fs::write(path.join("lib.rs"), "fn notmain() {}").unwrap(); + cargo_process("init --lib") + .cwd(&path) + .with_status(101) + .with_stderr( + "[ERROR] cannot have a package with multiple libraries, found both `foo.rs` and `lib.rs`" + ) + .run(); +}