Skip to content
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.

Sign up for GitHub

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

Jump to bottom

delete --host command and message #10145

Merged
merged 3 commits into from
Dec 14, 2021
Merged

delete --host command and message #10145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56b8ae7
delete --host command and message
heisen-li Dec 1, 2021
64c69df
fix some bugs
heisen-li Dec 1, 2021
cf621fd
modify test
heisen-li Dec 2, 2021
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
6 changes: 0 additions & 6 deletions src/bin/cargo/commands/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ pub fn cli() -> App {
)
.arg(opt("quiet", "No output printed to stdout").short("q"))
.arg(Arg::with_name("token"))
// --host is deprecated (use --registry instead)
.arg(
opt("host", "Host to set the token for")
.value_name("HOST")
.hidden(true),
)
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.after_help("Run `cargo help login` for more detailed information.\n")
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/publish.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {

let registry = args.registry(config)?;
let ws = args.workspace(config)?;
let index = args.index(config)?;
let index = args.index()?;

ops::publish(
&ws,
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn cli() -> App {

pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
let registry = args.registry(config)?;
let index = args.index(config)?;
let index = args.index()?;
let limit = args.value_of_u32("limit")?;
let limit = min(100, limit.unwrap_or(10));
let query: Vec<&str> = args.values_of("query").unwrap_or_default().collect();
Expand Down
23 changes: 2 additions & 21 deletions src/cargo/util/command_prelude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,27 +613,8 @@ pub trait ArgMatchesExt {
}
}

fn index(&self, config: &Config) -> CargoResult<Option<String>> {
// TODO: deprecated. Remove once it has been decided `--host` can be removed
// We may instead want to repurpose the host flag, as mentioned in issue
// rust-lang/cargo#4208.
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. 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 index = match self._value_of("host") {
Some(host) => {
config.shell().warn(&msg)?;
Some(host.to_string())
}
None => self._value_of("index").map(|s| s.to_string()),
};
fn index(&self) -> CargoResult<Option<String>> {
let index = self._value_of("index").map(|s| s.to_string());
Ok(index)
}

Expand Down
86 changes: 1 addition & 85 deletions tests/testsuite/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
//! Tests for the `cargo login` command.

use cargo::core::Shell;
use cargo::util::config::Config;
use cargo_test_support::install::cargo_home;
use cargo_test_support::registry::{self, registry_url};
use cargo_test_support::registry;
use cargo_test_support::{cargo_process, paths, t};
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
Expand All @@ -18,11 +16,6 @@ fn setup_new_credentials() {
setup_new_credentials_at(config);
}

fn setup_new_credentials_toml() {
let config = cargo_home().join("credentials.toml");
setup_new_credentials_at(config);
}

fn setup_new_credentials_at(config: PathBuf) {
t!(fs::create_dir_all(config.parent().unwrap()));
t!(fs::write(
Expand Down Expand Up @@ -66,83 +59,6 @@ fn check_token(expected_token: &str, registry: Option<&str>) -> bool {
}
}

#[cargo_test]
fn login_with_old_credentials() {
registry::init();

cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.run();

// Ensure that we get the new token for the registry
assert!(check_token(TOKEN, None));
Copy link
Member

Choose a reason for hiding this comment

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

I believe these tests should largely all get updated to --index unless they're explicitly testing that --host produces a deprecated error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your reply. Actually, --registry is used to replace --host in here. The test case of cargo login --registry is already available below. There seems to be no need for modification here.

Copy link
Member

Choose a reason for hiding this comment

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

If --registry is the replacement for --host (I forget what all these flags do) then we can't delete the warning because the warning says "Please use '--index' instead.". If --index works then these tests should use --index.

Copy link
Contributor Author

@heisen-li heisen-li Dec 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for replying, alex. As I understand it, in the cargo login subcommand, use --registry instead of --host;
https://github.com/rust-lang/cargo/blob/master/src/bin/cargo/commands/login.rs#L13-L18
In the cargo publish and cargo search subcommand, use --index instead of --host.
#4209 and #4267
The --host command is now redundant, and both the cargo login --registry and cargo publish --index tests exist.

Please use '--index' instead.

This refers to the use of --index instead of --host in cargo search and cargo publish, not login, and there are already test cases.Therefore, in these two commands --host related information is redundant.

In the login command, there is a test case for --registry.

}

#[cargo_test]
fn login_with_new_credentials() {
registry::init();
setup_new_credentials();

cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.run();

// Ensure that we get the new token for the registry
assert!(check_token(TOKEN, None));
}

#[cargo_test]
fn credentials_work_with_extension() {
registry::init();
setup_new_credentials_toml();

cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.run();

// Ensure that we get the new token for the registry
assert!(check_token(TOKEN, None));
}

#[cargo_test]
fn login_with_old_and_new_credentials() {
setup_new_credentials();
login_with_old_credentials();
}

#[cargo_test]
fn login_without_credentials() {
registry::init();
cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.run();

// Ensure that we get the new token for the registry
assert!(check_token(TOKEN, None));
}

#[cargo_test]
fn new_credentials_is_used_instead_old() {
registry::init();
setup_new_credentials();

cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.run();

let mut config = Config::new(Shell::new(), cargo_home(), cargo_home());
let _ = config.values();
let _ = config.load_credentials();

let token = config.get_string("registry.token").unwrap().map(|p| p.val);
assert_eq!(token.unwrap(), TOKEN);
}

#[cargo_test]
fn registry_credentials() {
registry::alt_init();
Expand Down
74 changes: 2 additions & 72 deletions tests/testsuite/publish.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use cargo_test_support::git::{self, repo};
use cargo_test_support::paths;
use cargo_test_support::registry::{self, registry_path, registry_url, Package};
use cargo_test_support::registry::{self, registry_url, Package};
use cargo_test_support::{basic_manifest, no_such_file_err_msg, project, publish};
use std::fs;

Expand Down Expand Up @@ -185,57 +185,8 @@ See [..]
validate_upload_foo();
}

// TODO: Deprecated
// remove once it has been decided --host can be removed
#[cargo_test]
fn simple_with_host() {
registry::init();

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("publish --no-verify --token sekrit --host")
.arg(registry_url().to_string())
.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. 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] `{reg}` index
[WARNING] manifest has no documentation, [..]
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[UPLOADING] foo v0.0.1 ([CWD])
",
reg = registry_path().to_str().unwrap()
))
.run();

validate_upload_foo();
}

// TODO: Deprecated
// remove once it has been decided --host can be removed
#[cargo_test]
fn simple_with_index_and_host() {
fn simple_with_index() {
registry::init();

let p = project()
Expand All @@ -255,27 +206,6 @@ fn simple_with_index_and_host() {

p.cargo("publish --no-verify --token sekrit --index")
.arg(registry_url().to_string())
.arg("--host")
.arg(registry_url().to_string())
.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. 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] `{reg}` index
[WARNING] manifest has no documentation, [..]
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[UPLOADING] foo v0.0.1 ([CWD])
",
reg = registry_path().to_str().unwrap()
))
.run();

validate_upload_foo();
Expand Down
54 changes: 0 additions & 54 deletions tests/testsuite/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,60 +181,6 @@ fn simple() {
.run();
}

// TODO: Deprecated
// remove once it has been decided '--host' can be safely removed
#[cargo_test]
fn simple_with_host() {
setup();

cargo_process("search postgres --host")
.arg(registry_url().to_string())
.with_stderr(
"\
[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. 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] `[CWD]/registry` index
",
)
.with_stdout_contains(SEARCH_RESULTS)
.run();
}

// TODO: Deprecated
// remove once it has been decided '--host' can be safely removed
#[cargo_test]
fn simple_with_index_and_host() {
setup();

cargo_process("search postgres --index")
.arg(registry_url().to_string())
.arg("--host")
.arg(registry_url().to_string())
.with_stderr(
"\
[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. 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] `[CWD]/registry` index
",
)
.with_stdout_contains(SEARCH_RESULTS)
.run();
}

#[cargo_test]
fn multiple_query_params() {
setup();
Expand Down