Load credentials only when needed #7774

giraffate · 2020-01-07T14:14:36Z

Credentials are always loaded, even if these are not used. If
access to confidential files such as credentials is not given,
cargo build fails despite not using credentials.

Fixes #7624.

rust-highfive · 2020-01-07T14:14:40Z

r? @ehuss

(rust_highfive has picked a reviewer for you, use r? to override)

Credentials are always loaded, even if these are not used. If access to confidential files such as credentials is not given, `cargo build` fails despite not using credentials. Fixes rust-lang#7624.

ehuss

The credentials aren't being loaded for all commands that require credentials. I think yank, unyank, and owners are the subcommands that need authentication. It would probably be good to have tests for those, too.

ehuss · 2020-01-07T21:57:03Z

src/cargo/util/config/mod.rs

@@ -1006,7 +1004,28 @@ impl Config {
            }
        }

-        cfg.merge(value, true)?;
+        if let CV::Table(mut map, _) = value {


Why does this not use merge?

This function is modified not to receive cfg: &mut ConfigValue as an argument, so I think that merge can not be used. However, I may simply not know how to use merge in Config, so could you tell me if there is any good way?

Ah, I see. Would it be possible to just loop over the key/values of the map and merge/insert all of them instead of hard-coding the keys here?

Thanks for your advice! I fixed it.

ehuss

Thanks for adding the tests!

ehuss · 2020-01-13T18:07:38Z

tests/testsuite/yank.rs

+use cargo_test_support::registry::{self, api_path, registry_url};
+
+fn setup(name: &str, version: &str) {
+    fs::create_dir_all(&api_path().join(format!("api/v1/crates/{}/{}", name, version))).unwrap();


I think this can be simplified a little with:

let dir = api_path().join(format!("api/v1/crates/{}/{}", name, version)); dir.mkdir_p(); fs::write(dir.join("yank"), r#"{"ok": true}"#).unwrap();

mkdir_p needs use cargo_test_support::paths::CargoPathExt;

ehuss · 2020-01-13T18:10:59Z

tests/testsuite/owner.rs

+use cargo_test_support::registry::{self, api_path, registry_url};
+
+fn setup(name: &str) {
+    fs::create_dir_all(&api_path().join(format!("api/v1/crates/{}", name))).unwrap();


Similar to yank.

let dir = api_path().join(format!("api/v1/crates/{}", name)); dir.mkdir_p(); // ... fs::write(dir.join("owners"), content).unwrap();

ehuss · 2020-01-13T18:11:38Z

tests/testsuite/publish.rs

+    registry::init();
+
+    let credentials_toml = paths::home().join(".cargo/credentials.toml");
+    File::create(&credentials_toml)


fs::write here too.

ehuss · 2020-01-13T18:17:29Z

src/cargo/util/config/mod.rs

@@ -1006,7 +1004,16 @@ impl Config {
            }
        }

-        cfg.merge(value, true)?;
+        if let CV::Table(map, _) = value {


Can this maybe be simplified a little instead of removing and reinserting? Maybe something like this?

if let CV::Table(map, _) = value { let base_map = self.values_mut()?; for (k, v) in map { match base_map.entry(k) { Vacant(entry) => { entry.insert(v); } Occupied(mut entry) => { entry.get_mut().merge(v, true)? } } } }

Use `mkdir_p` and `fs::write`.

giraffate · 2020-01-14T13:58:35Z

Thanks for your review! I fixed it.

ehuss · 2020-01-15T00:30:59Z

Great, thanks!
@bors r+

bors · 2020-01-15T00:31:01Z

📌 Commit 438d005 has been approved by ehuss

bors · 2020-01-15T00:31:11Z

⌛ Testing commit 438d005 with merge 0a4ec29...

Load credentials only when needed Credentials are always loaded, even if these are not used. If access to confidential files such as credentials is not given, `cargo build` fails despite not using credentials. Fixes #7624.

bors · 2020-01-15T00:56:14Z

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing 0a4ec29 to master...

…chton Update cargo, books ## cargo 9 commits in ad3dbe10e1e654fb1f032a5dd9481d7cbaa00d65..f6449ba236db31995255ac5e4cad4ab88296a7c6 2020-01-13 21:37:15 +0000 to 2020-01-21 16:15:39 +0000 - Fix wrong directories in host_libdir. (rust-lang/cargo#7798) - Update humantime requirement from 1.2.0 to 2.0.0 (rust-lang/cargo#7815) - Fix doc_target test which no longer works on stable/beta. (rust-lang/cargo#7817) - Fix some erroneous em-dashes in man pages. (rust-lang/cargo#7814) - fix some clippy warnings (rust-lang/cargo#7808) - Don't assume iowait always increases on Linux (rust-lang/cargo#7803) - Add and update some doc comments. (rust-lang/cargo#7800) - Consistently use em-dash in environment documentation page. (rust-lang/cargo#7799) - Load credentials only when needed (rust-lang/cargo#7774) ## reference 3 commits in e115753..11e893f 2019-12-22 13:13:14 +0100 to 2020-01-18 21:24:08 +0100 - Small improvements to types/pointer.md (rust-lang/reference#726) - repr(transparent): mention align=1 requirement (rust-lang/reference#737) - Elaborate on how to use an extern static correctly (rust-lang/reference#736) ## book 4 commits in 5c5cfd2e94cd42632798d9bd3d1116133e128ac9..87dd6843678575f8dda962f239d14ef4be14b352 2019-12-16 09:27:21 -0600 to 2020-01-20 15:20:40 -0500 - Fix listing numbers (rust-lang/book#2227) - Move `async` and `await` keywords to 'Currently in Use' (rust-lang/book#2140) - More cleanup - remove unneeded files (rust-lang/book#2213) - Small cleanups extracted from the bigger pr i'm working on (rust-lang/book#2212) ## rust-by-example 1 commits in 1d59403cb5269c190cc52a95584ecc280345495a..1c2bd024d13f8011307e13386cf1fea2180352b5 2019-12-27 08:27:05 -0300 to 2020-01-20 12:18:36 -0300 - CamelCase -> UpperCamelCase (rust-lang/rust-by-example#1302) ## embedded-book 1 commits in 9493b7d4dc97eda439bd8780f05ad7b234cd1cd7..4d78994915af1bde9a95c04a8c27d8dca066232a 2019-12-27 20:05:00 +0000 to 2020-01-14 08:25:25 +0000 - Update .gitattributes (rust-embedded/book#221)

rust-highfive assigned ehuss Jan 7, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 7, 2020

Load credentials only when needed

4b70f14

Credentials are always loaded, even if these are not used. If access to confidential files such as credentials is not given, `cargo build` fails despite not using credentials. Fixes rust-lang#7624.

giraffate force-pushed the update_credentials branch from 654c949 to 4b70f14 Compare January 7, 2020 14:41

ehuss reviewed Jan 8, 2020

View reviewed changes

giraffate added 2 commits January 9, 2020 22:22

Load credentials and add tests for yank and owner commands

5e15286

Refactoring to loop over the key/values instead of hard-coding

3c673cb

ehuss reviewed Jan 13, 2020

View reviewed changes

giraffate added 2 commits January 14, 2020 16:15

Refactoring of creating files in tests

8076f57

Use `mkdir_p` and `fs::write`.

Refactoring to use Vacant/Occupied instead of remove/insert

438d005

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2020

bors merged commit 438d005 into rust-lang:master Jan 15, 2020

giraffate deleted the update_credentials branch January 15, 2020 00:57

ehuss mentioned this pull request Jan 21, 2020

Update cargo, books rust-lang/rust#68421

Merged

ehuss added this to the 1.42.0 milestone Feb 6, 2022

Eh2406 mentioned this pull request May 16, 2022

Implement RFC 3139: alternative registry authentication support #10592

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load credentials only when needed #7774

Load credentials only when needed #7774

giraffate commented Jan 7, 2020

rust-highfive commented Jan 7, 2020

ehuss left a comment

ehuss Jan 7, 2020

giraffate Jan 9, 2020

ehuss Jan 9, 2020

giraffate Jan 10, 2020

ehuss left a comment

ehuss Jan 13, 2020

ehuss Jan 13, 2020

ehuss Jan 13, 2020

ehuss Jan 13, 2020

giraffate commented Jan 14, 2020

ehuss commented Jan 15, 2020

bors commented Jan 15, 2020

bors commented Jan 15, 2020

bors commented Jan 15, 2020

Load credentials only when needed #7774

Load credentials only when needed #7774

Conversation

giraffate commented Jan 7, 2020

rust-highfive commented Jan 7, 2020

ehuss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehuss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giraffate commented Jan 14, 2020

ehuss commented Jan 15, 2020

bors commented Jan 15, 2020

bors commented Jan 15, 2020

bors commented Jan 15, 2020