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

cargo uninstall with a sparse registry removes "sparse+" from the urls in .cargo.toml and .cargo2.json breaking install/uninstall #11751

Closed
jsumner opened this issue Feb 21, 2023 · 1 comment · Fixed by #11756
Closed

cargo uninstall with a sparse registry removes "sparse+" from the urls in .cargo.toml and .cargo2.json breaking install/uninstall #11751

jsumner opened this issue Feb 21, 2023 · 1 comment · Fixed by #11756
Assignees
@arlosi
Labels
A-sparse-registry Area: http sparse registries C-bug Category: bug Command-uninstall P-high Priority: High regression-from-stable-to-beta Regression in beta that previously worked in stable.

Comments

@jsumner
Copy link

jsumner commented Feb 21, 2023 •
edited by rustbot
Loading

Problem

When running "cargo uninstall" the "sparse" prefix is removed from the urls in .crates.toml and .crates2.json files. Breaking the file and stopping cargo install/uninstall from working.

The url in both .crates.toml and .crates2.json both require a prefix. Typically this is "registry", however when using a sparse registry it should be "sparse". Some of the code expects this and reading the files now fails.

Install works fine against sparse registries - it is the act of uninstalling is what breaks the files. Other cargo commands seem fine against a sparse repo as well.

Steps

  1. cargo install yaml2json
  2. cargo install grex
  3. edit ~/.cargo/.crates.toml and change url for grex from registry+... to sparse+...
    From: grex 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = ["grex"]
    To: grex 1.4.1 (sparse+https://github.com/rust-lang/crates.io-index)" = ["grex"]
  4. cargo uninstall yaml2json
  5. check ~/.cargo/crates.toml - grex url is now http... missing the sparse at the start.
    Output: grex 1.4.1 (https://github.com/rust-lang/crates.io-index)" = ["grex"]

This reproduces without having the actually have a sparse registry. If you have a sparse registry you can skip step 3 as that's just a hack to mimic.

Possible Solution(s)

src/cargo/core/source/source_id.rs - line 724 writes the url as "{url}", however I think it should be writing as "sparse+{url}" based on what the rest of the function is doing.

A workaround for anyone hitting the same problem is to add the "sparse+" back into the crates file (need to do both).

Notes

I've made the change in a test environment and it seems to do the trick, however I don't know what other knock on effects this change may have.

Version

cargo 1.67.1 (8ecd4f20a 2023-01-10)
release: 1.67.1
commit-hash: 8ecd4f20a9efb626975ac18a016d480dc7183d9b
commit-date: 2023-01-10
host: x86_64-unknown-linux-gnu
libgit2: 1.5.0 (sys:0.16.0 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:OpenSSL/1.1.1q)
os: Arch Linux [64-bit]
@jsumner jsumner added the C-bug Category: bug label Feb 21, 2023
@ehuss ehuss added P-high Priority: High Command-uninstall A-sparse-registry Area: http sparse registries labels Feb 21, 2023
@arlosi
Copy link
Contributor

arlosi commented Feb 22, 2023

@rustbot claim

@arlosi arlosi mentioned this issue Feb 22, 2023
Merged
@weihanglo weihanglo added the regression-from-stable-to-beta Regression in beta that previously worked in stable. label Feb 23, 2023
@bors bors closed this as completed in 964b730 Feb 26, 2023
weihanglo pushed a commit to weihanglo/cargo that referenced this issue Feb 26, 2023
@bors @weihanglo
Auto merge of rust-lang#11756 - arlosi:sparse-uninstall, r=ehuss
c3d6601 
Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml

The URL associated with a `SourceId` for a sparse registry includes the `sparse+` prefix in the URL to differentiate from git (non-sparse) registries.

`SourceId::from_url` was not including the `sparse+` prefix of sparse registry URLs on construction, which caused roundtrips of `as_url` and `from_url` to fail by losing the prefix.

Fixes rust-lang#11751 by:
* Including the prefix in the URL
* Adding a test that verifies round-trip behavior for sparse `SourceId`s
* Modifying `CanonicalUrl` so it no longer considers `sparse+` and non-`sparse+` URLs to be equivalent
weihanglo pushed a commit to weihanglo/cargo that referenced this issue Feb 26, 2023
@bors @weihanglo
Auto merge of rust-lang#11756 - arlosi:sparse-uninstall, r=ehuss
bedb524 
Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml

The URL associated with a `SourceId` for a sparse registry includes the `sparse+` prefix in the URL to differentiate from git (non-sparse) registries.

`SourceId::from_url` was not including the `sparse+` prefix of sparse registry URLs on construction, which caused roundtrips of `as_url` and `from_url` to fail by losing the prefix.

Fixes rust-lang#11751 by:
* Including the prefix in the URL
* Adding a test that verifies round-trip behavior for sparse `SourceId`s
* Modifying `CanonicalUrl` so it no longer considers `sparse+` and non-`sparse+` URLs to be equivalent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arlosi arlosi

Labels
A-sparse-registry Area: http sparse registries C-bug Category: bug Command-uninstall P-high Priority: High regression-from-stable-to-beta Regression in beta that previously worked in stable.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml arlosi/cargo
4 participants
@ehuss @arlosi @jsumner @weihanglo