Skip to content

Commit

Permalink
Auto merge of #13164 - epage:empty, r=weihanglo
Browse files Browse the repository at this point in the history
fix: Fill in more empty name holes

### What does this PR try to resolve?

This is a follow up to #13152 and expands on work done in #12928.

This is also part of #12801 as I am refactoring how we do validation so that it parts of it can move into the schema, removing the last dependency the schema has on the rest of cargo.

### How should we test and review this PR?

This prevents empty registry names which should be fine for the same reason as preventing empty feature names in #12928, that this was a regression due to changes in toml parsers

### Additional information
  • Loading branch information
bors committed Dec 13, 2023
2 parents 8412d30 + a383185 commit 078a65b
Show file tree
Hide file tree
Showing 18 changed files with 220 additions and 11 deletions.
4 changes: 4 additions & 0 deletions src/cargo/util/restricted_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ pub fn is_conflicting_artifact_name(name: &str) -> bool {
/// elsewhere. `cargo new` has a few restrictions, such as checking for
/// reserved names. crates.io has even more restrictions.
pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<()> {
if name.is_empty() {
bail!("{what} cannot be empty");
}

let mut chars = name.chars();
if let Some(ch) = chars.next() {
if ch.is_digit(10) {
Expand Down
4 changes: 0 additions & 4 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,6 @@ pub fn to_real_manifest(
};

let package_name = package.name.trim();
if package_name.is_empty() {
bail!("package name cannot be an empty string")
}

validate_package_name(package_name, "package name", "")?;

let resolved_path = package_root.join("Cargo.toml");
Expand Down
6 changes: 0 additions & 6 deletions src/cargo/util_schemas/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ impl PackageIdSpec {
Some(version) => Some(version.parse::<PartialVersion>()?),
None => None,
};
if name.is_empty() {
bail!("package ID specification must have a name: `{spec}`");
}
validate_package_name(name, "pkgid", "")?;
Ok(PackageIdSpec {
name: String::from(name),
Expand Down Expand Up @@ -185,9 +182,6 @@ impl PackageIdSpec {
None => (String::from(path_name), None),
}
};
if name.is_empty() {
bail!("package ID specification must have a name: `{url}`");
}
validate_package_name(name.as_str(), "pkgid", "")?;
Ok(PackageIdSpec {
name,
Expand Down
76 changes: 76 additions & 0 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1546,3 +1546,79 @@ or use environment variable CARGO_REGISTRY_TOKEN",
)
.run();
}

#[cargo_test]
fn config_empty_registry_name() {
let _ = RegistryBuilder::new()
.no_configure_token()
.alternative()
.build();
let p = project()
.file("src/lib.rs", "")
.file(
".cargo/config.toml",
"[registry.'']
",
)
.build();

p.cargo("publish")
.arg("--registry")
.arg("")
.with_status(101)
.with_stderr(
"\
[ERROR] registry name cannot be empty",
)
.run();
}

#[cargo_test]
fn empty_registry_flag() {
let p = project().file("src/lib.rs", "").build();

p.cargo("publish")
.arg("--registry")
.arg("")
.with_status(101)
.with_stderr(
"\
[ERROR] registry name cannot be empty",
)
.run();
}

#[cargo_test]
fn empty_dependency_registry() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
[dependencies]
bar = { version = "0.1.0", registry = "" }
"#,
)
.file(
"src/lib.rs",
"
extern crate bar;
pub fn f() { bar::bar(); }
",
)
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
Caused by:
registry name cannot be empty",
)
.run();
}
2 changes: 1 addition & 1 deletion tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ fn cargo_compile_with_empty_package_name() {
[ERROR] failed to parse manifest at `[..]`
Caused by:
package name cannot be an empty string
package name cannot be empty
",
)
.run();
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/empty_dep_name/in
24 changes: 24 additions & 0 deletions tests/testsuite/cargo_add/empty_dep_name/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

use cargo_test_support::curr_dir;

#[cargo_test]
fn case() {
cargo_test_support::registry::init();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("@1.2.3")
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_add/empty_dep_name/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/empty_dep_name/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: dependency name cannot be empty
Empty file.
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod dev;
mod dev_build_conflict;
mod dev_prefer_existing_version;
mod dry_run;
mod empty_dep_name;
mod empty_dep_table;
mod features;
mod features_activated_over_limit;
Expand Down
Empty file.
22 changes: 22 additions & 0 deletions tests/testsuite/cargo_new/empty_name/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("new")
.args(["foo", "--name", ""])
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
Empty file.
1 change: 1 addition & 0 deletions tests/testsuite/cargo_new/empty_name/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: package name cannot be empty
Empty file.
1 change: 1 addition & 0 deletions tests/testsuite/cargo_new/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod add_members_to_workspace_with_absolute_package_path;
mod add_members_to_workspace_with_empty_members;
mod add_members_to_workspace_with_exclude_list;
mod add_members_to_workspace_with_members_glob;
mod empty_name;
mod help;
mod inherit_workspace_lints;
mod inherit_workspace_package_table;
Expand Down
83 changes: 83 additions & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2700,3 +2700,86 @@ perhaps a crate was updated and forgotten to be re-vendored?"#,
)
.run();
}

#[cargo_test]
fn from_config_empty() {
Package::new("bar", "0.1.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
[dependencies]
bar = "0.1.0"
"#,
)
.file(
".cargo/config.toml",
r#"
[patch.'']
bar = { path = 'bar' }
"#,
)
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1"))
.file("bar/src/lib.rs", r#""#)
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] [patch] entry `` should be a URL or registry name
Caused by:
invalid url ``: relative URL without a base
",
)
.run();
}

#[cargo_test]
fn from_manifest_empty() {
Package::new("bar", "0.1.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
[dependencies]
bar = "0.1.0"
[patch.'']
bar = { path = 'bar' }
"#,
)
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1"))
.file("bar/src/lib.rs", r#""#)
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
Caused by:
[patch] entry `` should be a URL or registry name
Caused by:
invalid url ``: relative URL without a base
",
)
.run();
}

0 comments on commit 078a65b

Please sign in to comment.