-
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
Fix overwriting alternate registry token #7708
Changes from 2 commits
6cbde6e
a718ed6
75d06de
2a4d1dc
3613999
b7bc069
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 |
---|---|---|
|
@@ -60,6 +60,17 @@ pub fn alt_api_url() -> Url { | |
Url::from_file_path(alt_api_path()).ok().unwrap() | ||
} | ||
|
||
fn generate_path(name: &str) -> PathBuf { | ||
paths::root().join(name) | ||
} | ||
fn generate_url(name: &str) -> Url { | ||
Url::from_file_path(generate_path(name)).ok().unwrap() | ||
} | ||
fn generate_dl_url(name: &str) -> String { | ||
let base = Url::from_file_path(generate_path(name)).ok().unwrap(); | ||
format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) | ||
} | ||
|
||
/// A builder for creating a new package in a registry. | ||
/// | ||
/// This uses "source replacement" using an automatically generated | ||
|
@@ -166,9 +177,13 @@ pub fn init() { | |
|
||
[registries.alternative] | ||
index = '{alt}' | ||
|
||
[registries.alternative2] | ||
index = '{alt2}' | ||
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. I think it would be best to not set this here, and only set it in the |
||
"#, | ||
reg = registry_url(), | ||
alt = alt_registry_url() | ||
alt = alt_registry_url(), | ||
alt2 = generate_url("alternative2-registry") | ||
) | ||
.as_bytes() | ||
)); | ||
|
@@ -214,6 +229,23 @@ pub fn init() { | |
fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap(); | ||
} | ||
|
||
pub fn init_alt2_registry() { | ||
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. Can this be written so that the other two registries reuse the same code instead of duplicating it? 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. I replaced this with a new function |
||
// Initialize an alternative2 registry. | ||
repo(&generate_path("alternative2-registry")) | ||
.file( | ||
"config.json", | ||
&format!( | ||
r#" | ||
{{"dl":"{}","api":"{}"}} | ||
"#, | ||
generate_dl_url("alt2_dl"), | ||
generate_url("alt2_api") | ||
), | ||
) | ||
.build(); | ||
fs::create_dir_all(generate_path("alt2_api").join("api/v1/crates")).unwrap(); | ||
} | ||
|
||
impl Package { | ||
/// Creates a new package builder. | ||
/// Call `publish()` to finalize and build the package. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ use cargo_test_support::{cargo_process, t}; | |
use toml; | ||
|
||
const TOKEN: &str = "test-token"; | ||
const TOKEN2: &str = "test-token2"; | ||
const ORIGINAL_TOKEN: &str = "api-token"; | ||
|
||
fn setup_new_credentials() { | ||
|
@@ -169,6 +170,7 @@ fn new_credentials_is_used_instead_old() { | |
#[cargo_test] | ||
fn registry_credentials() { | ||
registry::init(); | ||
registry::init_alt2_registry(); | ||
setup_new_credentials(); | ||
|
||
let reg = "alternative"; | ||
|
@@ -185,4 +187,18 @@ fn registry_credentials() { | |
|
||
// Also ensure that we get the new token for the registry | ||
assert!(check_token(TOKEN, Some(reg))); | ||
|
||
let reg2 = "alternative2"; | ||
cargo_process("login --registry") | ||
.arg(reg2) | ||
.arg(TOKEN2) | ||
.arg("-Zunstable-options") | ||
.masquerade_as_nightly_cargo() | ||
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. These two lines can be removed, I think we just forgot to remove them when stabilized. And remove the same 2 lines in the call above. |
||
.run(); | ||
|
||
// Ensure not overwriting 1st alternate registry token with | ||
// 2nd alternate registry token (see rust-lang/cargo#7701). | ||
assert!(check_token(ORIGINAL_TOKEN, None)); | ||
assert!(check_token(TOKEN, Some(reg))); | ||
assert!(check_token(TOKEN2, Some(reg2))); | ||
} |
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.
Can all the other functions (the
alt_*
functions) be changed to call these new functions instead of duplicating the code?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.
Yes, I fixed it like that. I also corrected
registry_path
function etc., but is this OK?