Add dependency `registry` to `cargo metadata`. #6500

ehuss · 2018-12-30T17:21:45Z

This adds the registry field for dependencies for alternate registries in
cargo metadata.

This adds the `registry` field for dependencies for alternate registries in `cargo metadata`.

rust-highfive · 2018-12-30T17:21:48Z

r? @Eh2406

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

ehuss · 2018-12-30T17:22:33Z

Sorry if this is a little long. I can split this up into multiple PRs if desired. I'll try to walk through the changes:

DetailedTomlDependency::to_dependency: Only set the registry_id in a dependency if it is set in the manifest (don't unconditionally set crates.io when not set). This is needed to properly display null in the metadata when it is not set, or if path is also set.
transmit: Because of the above change, handle when a dependency is null.
DetailedDependency::into_dep: Properly handle the distinction between the index (where null=same registry) and a manifest (where null=crates.io).
RegistrySource::get_pkg: Remove the code that replaces the summary. This was causing a problem because the summary loaded from the index has registry values different from registry values in a manifest. I'm moderately confident this code is not needed anymore, because the dependency building code (now in unit_dependencies.rs) now solely uses the resolver to fetch dependency information. This was added in Ignore summaries in downloaded crates #3217. I checked that the offending crate (nuklear-backend-gfx 0.1.3) grabs the correct dependency, and of course the test in Ignore summaries in downloaded crates #3217 still passes.
testsuite changes:
- Package: Properly handle normal deps vs alternative deps. It looks like there was some weirdness because index registry values are not the same as manifest registry values.
- Consolidate the code for validating upload/publish and packaging.
- Make sure publish tests validate the uploaded JSON values (important for this PR to verify it didn't break anything — nothing was checking the json!).
- Extract out find_json_mismatch into a reusable function.

Eh2406 · 2018-12-31T15:27:24Z

I looked over the code and the description. It all looks good. I especially love the improvements to the tests. But this is not a topic I know much about, so I will wait on an r+ for others to give feedback.

alexcrichton

This looks great to me as well, thanks so much @ehuss! I only found one tiny nit, but otherwise r=me

Does this also close #6499?

alexcrichton · 2019-01-02T17:02:52Z

src/cargo/core/source/source_id.rs

@@ -247,6 +249,8 @@ impl SourceId {
    }

    /// Is this source from an alternative registry
+    /// DEPRECATED: This is not correct if the registry name is not known
+    /// (for example when loaded from an index).


To confirm, is this left in for clients of the cargo crate? Or does Cargo still use this a lot internally?

(if it's the former I think we can just delete this)

There is one usage in cargo. I believe it is currently correct due to where it is used, but I put it on my todo list to remove it in the future since it is easy to get wrong. Its removal isn't trivial, and was unrelated to this PR.

ehuss · 2019-01-02T17:53:47Z

#6499 could probably be closed. I was a little confused because the RFC mentioned supporting the ability to specify URLs in the registry field, but I could not find any discussion about it afterwards. Was it decided not to bother with that?

alexcrichton · 2019-01-02T19:24:28Z

@bors: r+

Ok!

For urls, we decided to have a dedicated field to discourage it, and the original RFC was accepted I believe with no urls specified anywhere in the manifest (explicitly). We then later added registry-index for published crates to have an "elaborated" view of the world. The registry-index field isn't really intended for public consumption, and registry should always be a human readable name

bors · 2019-01-02T19:24:29Z

📌 Commit 64042d8 has been approved by alexcrichton

bors · 2019-01-02T19:24:40Z

⌛ Testing commit 64042d8 with merge 21fe000...

Add dependency `registry` to `cargo metadata`. This adds the `registry` field for dependencies for alternate registries in `cargo metadata`.

bors · 2019-01-02T19:53:16Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 21fe000 to master...

Update cargo 24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e 2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000 - Display environment variables for rustc commands (rust-lang/cargo#6492) - Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515) - Add a high-level overview of how `fix` works. (rust-lang/cargo#6516) - Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500) - Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493) - serialize version directly (rust-lang/cargo#6512) - use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355) - Fix error message when resolving dependencies (rust-lang/cargo#6510) - use PathBuf in cargo metadata (rust-lang/cargo#6511) - Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506) - Update display of contents of Cargo.toml (rust-lang/cargo#6501) - Update display of contents of Cargo.toml (rust-lang/cargo#6502) - Fixup cargo install's help message (rust-lang/cargo#6495) - testsuite: Require failing commands to check output. (rust-lang/cargo#6497) - Delete unnecessary 'return' (rust-lang/cargo#6496) - Fix new unused patch warning. (rust-lang/cargo#6494) - Some minor documentation changes. (rust-lang/cargo#6481) - Add `links` to `cargo metadata`. (rust-lang/cargo#6480) - Salvaged semver work (rust-lang/cargo#6476) - Warn on unused patches. (rust-lang/cargo#6470) - don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473) - Rewrite `login` and registry cleanups. (rust-lang/cargo#6466) - [issue#6461] Fix cargo commands list (rust-lang/cargo#6462) - Restrict registry names to same style as package names. (rust-lang/cargo#6469)

Looks like rust-lang/cargo#6500 accidentally broke the previous logic, so let's load the checksums from elsewhere!

While not actually used in Cargo itself the `Summary::checksum` method of downloaded packages from the registry currently returns `None`. This was accidentally regressed in rust-lang#6500 and noticed when updating `cargo-vendor` (it took a long time to get around to updating that). The fix here is to override the `checksum` field of the summary for packages we return from the registry, and everything else should remain the same!

Ensure Summary::checksum works for registry crates While not actually used in Cargo itself the `Summary::checksum` method of downloaded packages from the registry currently returns `None`. This was accidentally regressed in #6500 and noticed when updating `cargo-vendor` (it took a long time to get around to updating that). The fix here is to override the `checksum` field of the summary for packages we return from the registry, and everything else should remain the same!

Add dependency registry to cargo metadata.

3d84d0a

This adds the `registry` field for dependencies for alternate registries in `cargo metadata`.

rust-highfive assigned Eh2406 Dec 30, 2018

Fix registry-index bug.

64042d8

ehuss mentioned this pull request Dec 31, 2018

Questions regarding registry-index. #6499

Closed

alexcrichton reviewed Jan 2, 2019

View reviewed changes

bors added a commit that referenced this pull request Jan 2, 2019

Auto merge of #6500 - ehuss:metadata-reg, r=alexcrichton

21fe000

Add dependency `registry` to `cargo metadata`. This adds the `registry` field for dependencies for alternate registries in `cargo metadata`.

bors merged commit 64042d8 into rust-lang:master Jan 2, 2019

ehuss mentioned this pull request Jan 3, 2019

Update cargo rust-lang/rust#57315

Merged

ehuss mentioned this pull request Jan 5, 2019

Add some more fields. oli-obk/cargo_metadata#61

Merged

alexcrichton added a commit to alexcrichton/cargo-vendor that referenced this pull request Apr 12, 2019

Fix loading checksums from Cargo

e3e5451

Looks like rust-lang/cargo#6500 accidentally broke the previous logic, so let's load the checksums from elsewhere!

alexcrichton mentioned this pull request Apr 12, 2019

Ensure Summary::checksum works for registry crates #6842

Merged

ehuss added this to the 1.33.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dependency `registry` to `cargo metadata`. #6500

Add dependency `registry` to `cargo metadata`. #6500

ehuss commented Dec 30, 2018

rust-highfive commented Dec 30, 2018

ehuss commented Dec 30, 2018

Eh2406 commented Dec 31, 2018

alexcrichton left a comment

alexcrichton Jan 2, 2019

ehuss Jan 2, 2019

ehuss commented Jan 2, 2019

alexcrichton commented Jan 2, 2019

bors commented Jan 2, 2019

bors commented Jan 2, 2019

bors commented Jan 2, 2019

Add dependency registry to cargo metadata. #6500

Add dependency registry to cargo metadata. #6500

Conversation

ehuss commented Dec 30, 2018

rust-highfive commented Dec 30, 2018

ehuss commented Dec 30, 2018

Eh2406 commented Dec 31, 2018

alexcrichton left a comment

Choose a reason for hiding this comment

alexcrichton Jan 2, 2019

Choose a reason for hiding this comment

ehuss Jan 2, 2019

Choose a reason for hiding this comment

ehuss commented Jan 2, 2019

alexcrichton commented Jan 2, 2019

bors commented Jan 2, 2019

bors commented Jan 2, 2019

bors commented Jan 2, 2019

Add dependency `registry` to `cargo metadata`. #6500

Add dependency `registry` to `cargo metadata`. #6500