From ce9e47ac81bf942558337878724bd58c541f38e0 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Thu, 22 Jun 2017 15:32:30 -0400 Subject: [PATCH 1/3] replace instances of flag --host with --index in publish, still supports --host however deprecation warning is issued --- src/bin/publish.rs | 24 +++++++++++-- tests/publish.rs | 84 ++++++++++++++++++++++++++++++++++++++++------ tests/registry.rs | 2 +- 3 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/bin/publish.rs b/src/bin/publish.rs index 3f0af0baaf7..36b7806d9f0 100644 --- a/src/bin/publish.rs +++ b/src/bin/publish.rs @@ -5,6 +5,7 @@ use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(Deserialize)] pub struct Options { + flag_index: Option, flag_host: Option, flag_token: Option, flag_manifest_path: Option, @@ -27,7 +28,8 @@ Usage: Options: -h, --help Print this message - --host HOST Host to upload the package to + --index INDEX Host to upload the package to + --host HOST DEPRECATED --token TOKEN Token to use when uploading --no-verify Don't verify package tarball before publish --allow-dirty Allow publishing with a dirty source directory @@ -48,8 +50,12 @@ pub fn execute(options: Options, config: &Config) -> CliResult { &options.flag_color, options.flag_frozen, options.flag_locked)?; + + + let Options { flag_token: token, + flag_index: index, flag_host: host, flag_manifest_path, flag_no_verify: no_verify, @@ -59,12 +65,26 @@ pub fn execute(options: Options, config: &Config) -> CliResult { .. } = options; + + let msg = "The flag '--host' is no longer valid. + +Previous versions of Cargo accepted this flag, but it is being +deprecated. The flag is being renamed to 'index', as the flag +wants the location of the index to which to publish. Please +use '--index' instead. + +This will soon become a hard error, so it's either recommended +to update to a fixed version or contact the upstream maintainer +about this warning."; + let root = find_root_manifest_for_wd(flag_manifest_path.clone(), config.cwd())?; let ws = Workspace::new(&root, config)?; ops::publish(&ws, &ops::PublishOpts { config: config, token: token, - index: host, + index: + if host.clone().is_none() || host.clone().unwrap().is_empty() { index } + else { config.shell().warn(&msg)?; host }, verify: !no_verify, allow_dirty: allow_dirty, jobs: jobs, diff --git a/tests/publish.rs b/tests/publish.rs index 968281f6219..1713dcb8f41 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -44,6 +44,60 @@ fn setup() { fn simple() { setup(); + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#) + .file("src/main.rs", "fn main() {}"); + + assert_that(p.cargo_process("publish").arg("--no-verify") + .arg("--index").arg(registry().to_string()), + execs().with_status(0).with_stderr(&format!("\ +[UPDATING] registry `{reg}` +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] foo v0.0.1 ({dir}) +[UPLOADING] foo v0.0.1 ({dir}) +", + dir = p.url(), + reg = registry()))); + + let mut f = File::open(&upload_path().join("api/v1/crates/new")).unwrap(); + // Skip the metadata payload and the size of the tarball + let mut sz = [0; 4]; + assert_eq!(f.read(&mut sz).unwrap(), 4); + let sz = ((sz[0] as u32) << 0) | + ((sz[1] as u32) << 8) | + ((sz[2] as u32) << 16) | + ((sz[3] as u32) << 24); + f.seek(SeekFrom::Current(sz as i64 + 4)).unwrap(); + + // Verify the tarball + let mut rdr = GzDecoder::new(f).unwrap(); + assert_eq!(rdr.header().filename().unwrap(), "foo-0.0.1.crate".as_bytes()); + let mut contents = Vec::new(); + rdr.read_to_end(&mut contents).unwrap(); + let mut ar = Archive::new(&contents[..]); + for file in ar.entries().unwrap() { + let file = file.unwrap(); + let fname = file.header().path_bytes(); + let fname = &*fname; + assert!(fname == b"foo-0.0.1/Cargo.toml" || + fname == b"foo-0.0.1/Cargo.toml.orig" || + fname == b"foo-0.0.1/src/main.rs", + "unexpected filename: {:?}", file.header().path()); + } +} + +#[test] +fn simple_with_host() { + setup(); + let p = project("foo") .file("Cargo.toml", r#" [project] @@ -58,6 +112,16 @@ fn simple() { assert_that(p.cargo_process("publish").arg("--no-verify") .arg("--host").arg(registry().to_string()), execs().with_status(0).with_stderr(&format!("\ +[WARNING] The flag '--host' is no longer valid. + +Previous versions of Cargo accepted this flag, but it is being +deprecated. The flag is being renamed to 'index', as the flag +wants the location of the index to which to publish. Please +use '--index' instead. + +This will soon become a hard error, so it's either recommended +to update to a fixed version or contact the upstream maintainer +about this warning. [UPDATING] registry `{reg}` [WARNING] manifest has no documentation, [..] See [..] @@ -113,7 +177,7 @@ fn git_deps() { .file("src/main.rs", "fn main() {}"); assert_that(p.cargo_process("publish").arg("-v").arg("--no-verify") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101).with_stderr("\ [UPDATING] registry [..] [ERROR] crates cannot be published to crates.io with dependencies sourced from \ @@ -150,7 +214,7 @@ fn path_dependency_no_version() { .file("bar/src/lib.rs", ""); assert_that(p.cargo_process("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101).with_stderr("\ [UPDATING] registry [..] [ERROR] all path dependencies must have a version specified when publishing. @@ -175,7 +239,7 @@ fn unpublishable_crate() { .file("src/main.rs", "fn main() {}"); assert_that(p.cargo_process("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101).with_stderr("\ [ERROR] some crates cannot be published. `foo` is marked as unpublishable @@ -205,7 +269,7 @@ fn dont_publish_dirty() { .build(); assert_that(p.cargo("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101).with_stderr("\ [UPDATING] registry `[..]` error: 1 files in the working directory contain changes that were not yet \ @@ -240,7 +304,7 @@ fn publish_clean() { .build(); assert_that(p.cargo("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(0)); } @@ -268,7 +332,7 @@ fn publish_in_sub_repo() { .build(); assert_that(p.cargo("publish").cwd(p.root().join("bar")) - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(0)); } @@ -297,7 +361,7 @@ fn publish_when_ignored() { .build(); assert_that(p.cargo("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(0)); } @@ -324,7 +388,7 @@ fn ignore_when_crate_ignored() { "#) .nocommit_file("bar/src/main.rs", "fn main() {}"); assert_that(p.cargo("publish").cwd(p.root().join("bar")) - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(0)); } @@ -350,7 +414,7 @@ fn new_crate_rejected() { "#) .nocommit_file("src/main.rs", "fn main() {}"); assert_that(p.cargo("publish") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101)); } @@ -370,7 +434,7 @@ fn dry_run() { .file("src/main.rs", "fn main() {}"); assert_that(p.cargo_process("publish").arg("--dry-run") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(0).with_stderr(&format!("\ [UPDATING] registry `[..]` [WARNING] manifest has no documentation, [..] diff --git a/tests/registry.rs b/tests/registry.rs index b336827abbc..e7bc06e4e4d 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -616,7 +616,7 @@ fn bad_license_file() { "#); assert_that(p.cargo_process("publish") .arg("-v") - .arg("--host").arg(registry().to_string()), + .arg("--index").arg(registry().to_string()), execs().with_status(101) .with_stderr_contains("\ [ERROR] the license file `foo` does not exist")); From fba459d2839b3b63cfaa9d229283feeb654901c4 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Thu, 22 Jun 2017 15:47:07 -0400 Subject: [PATCH 2/3] Add test for if user ever decides to use both --index and --host --- tests/publish.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/publish.rs b/tests/publish.rs index 1713dcb8f41..d2b3e748a57 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -158,6 +158,71 @@ See [..] } } +#[test] +fn simple_with_index_and_host() { + setup(); + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#) + .file("src/main.rs", "fn main() {}"); + + assert_that(p.cargo_process("publish").arg("--no-verify") + .arg("--index").arg(registry().to_string()) + .arg("--host").arg(registry().to_string()), + execs().with_status(0).with_stderr(&format!("\ +[WARNING] The flag '--host' is no longer valid. + +Previous versions of Cargo accepted this flag, but it is being +deprecated. The flag is being renamed to 'index', as the flag +wants the location of the index to which to publish. Please +use '--index' instead. + +This will soon become a hard error, so it's either recommended +to update to a fixed version or contact the upstream maintainer +about this warning. +[UPDATING] registry `{reg}` +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] foo v0.0.1 ({dir}) +[UPLOADING] foo v0.0.1 ({dir}) +", + dir = p.url(), + reg = registry()))); + + let mut f = File::open(&upload_path().join("api/v1/crates/new")).unwrap(); + // Skip the metadata payload and the size of the tarball + let mut sz = [0; 4]; + assert_eq!(f.read(&mut sz).unwrap(), 4); + let sz = ((sz[0] as u32) << 0) | + ((sz[1] as u32) << 8) | + ((sz[2] as u32) << 16) | + ((sz[3] as u32) << 24); + f.seek(SeekFrom::Current(sz as i64 + 4)).unwrap(); + + // Verify the tarball + let mut rdr = GzDecoder::new(f).unwrap(); + assert_eq!(rdr.header().filename().unwrap(), "foo-0.0.1.crate".as_bytes()); + let mut contents = Vec::new(); + rdr.read_to_end(&mut contents).unwrap(); + let mut ar = Archive::new(&contents[..]); + for file in ar.entries().unwrap() { + let file = file.unwrap(); + let fname = file.header().path_bytes(); + let fname = &*fname; + assert!(fname == b"foo-0.0.1/Cargo.toml" || + fname == b"foo-0.0.1/Cargo.toml.orig" || + fname == b"foo-0.0.1/src/main.rs", + "unexpected filename: {:?}", file.header().path()); + } +} + #[test] fn git_deps() { setup(); From 78e64de9c7335ff83cedf00467fbb14dac300400 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Thu, 22 Jun 2017 16:34:57 -0400 Subject: [PATCH 3/3] Added comments indicating which parts should be removed at appropriate time --- src/bin/publish.rs | 15 ++++++++++----- tests/publish.rs | 4 ++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/bin/publish.rs b/src/bin/publish.rs index 36b7806d9f0..507496b46c2 100644 --- a/src/bin/publish.rs +++ b/src/bin/publish.rs @@ -6,7 +6,7 @@ use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(Deserialize)] pub struct Options { flag_index: Option, - flag_host: Option, + flag_host: Option, // TODO: Deprecated, remove flag_token: Option, flag_manifest_path: Option, flag_verbose: u32, @@ -28,8 +28,8 @@ Usage: Options: -h, --help Print this message - --index INDEX Host to upload the package to - --host HOST DEPRECATED + --index INDEX Registry index to upload the package to + --host HOST DEPRECATED, renamed to '--index' --token TOKEN Token to use when uploading --no-verify Don't verify package tarball before publish --allow-dirty Allow publishing with a dirty source directory @@ -56,7 +56,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult { let Options { flag_token: token, flag_index: index, - flag_host: host, + flag_host: host, // TODO: Deprecated, remove flag_manifest_path, flag_no_verify: no_verify, flag_allow_dirty: allow_dirty, @@ -66,6 +66,11 @@ pub fn execute(options: Options, config: &Config) -> CliResult { } = options; + // TODO: Deprecated + // remove once it has been decided --host can be removed + // We may instead want to repurpose the host flag, as + // mentioned in this issue + // https://github.com/rust-lang/cargo/issues/4208 let msg = "The flag '--host' is no longer valid. Previous versions of Cargo accepted this flag, but it is being @@ -84,7 +89,7 @@ about this warning."; token: token, index: if host.clone().is_none() || host.clone().unwrap().is_empty() { index } - else { config.shell().warn(&msg)?; host }, + else { config.shell().warn(&msg)?; host }, // TODO: Deprecated, remove verify: !no_verify, allow_dirty: allow_dirty, jobs: jobs, diff --git a/tests/publish.rs b/tests/publish.rs index d2b3e748a57..166b021c7be 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -94,6 +94,8 @@ See [..] } } +// TODO: Deprecated +// remove once it has been decided --host can be removed #[test] fn simple_with_host() { setup(); @@ -158,6 +160,8 @@ See [..] } } +// TODO: Deprecated +// remove once it has been decided --host can be removed #[test] fn simple_with_index_and_host() { setup();