nabijaczleweli / cargo-update

A cargo subcommand for checking and applying updates to installed executables
MIT License
1.24k stars 42 forks source link

`find_git_db_repo` not always work #135

Closed DCjanus closed 4 years ago

DCjanus commented 4 years ago

Example:

$ cargo install --git https://github.com/greshake/i3status-rust
Installed package `i3status-rs v0.14.1 (https://github.com/greshake/i3status-rust#0d39c9b8)` (executable 
`i3status-rs`)
$ ls ~/.cargo/git/db
i3status-rust-6c011c22e9b90d52

which means, in this function cratename is i3status-rs rather than i3status-rust. And this function return None always.

DCjanus commented 4 years ago

By the way, directories in $HOME/.cargo/git/db is in the format cratename-hash, maybe we should copy some code from cargo and generate same hash for same package.

starts_with might not be a good idea.

I'd like to help this, but might took a long time.

nabijaczleweli commented 4 years ago

FWIW, this doesn't actually break the update, just slightly pessimises by doing a fresh clone into --temp-dir:

nabijaczleweli@tarta:~/uwu/i3status-rust$ cargo install-update -lag
    Updating registry 'https://github.com/rust-lang/crates.io-index'

Package     Installed  Latest   Needs update
cargo-deb   v1.26.0    v1.26.0  No
cargo-tree  v0.29.0    v0.29.0  No
checksums   v0.6.0     v0.6.0   No
termimage   v1.1.0     v1.1.0   No
treesize    v0.5.0     v0.5.0   No

Package      Installed                                 Latest                                    Needs update
i3status-rs  000000000000000000000000f3a7a00015c65fba  8ec217ae0d5488136009a575f3a7a00015c65fba  Yes

So as to the bare starts_with(): you're right, that might actually cause problems (though I think cargo checks out by SHA anyway so it might also not?).

I tracked the location down to https://github.com/rust-lang/cargo/blob/74f2b400d2be43da798f99f94957d359bc223988/src/cargo/sources/git/source.rs#L118, where self.ident is the result of

fn ident(id: &SourceId) -> String {
    let ident = id
        .canonical_url()
        .raw_canonicalized_url()
        .path_segments()
        .and_then(|s| s.rev().next())
        .unwrap_or("");

    let ident = if ident == "" { "_empty" } else { ident };

    format!("{}-{}", ident, short_hash(id.canonical_url()))
}

Which should be easy enough to integrate.

nabijaczleweli commented 4 years ago

@DCjanus can you try from current HEAD (i.e. with https://github.com/nabijaczleweli/cargo-update/commit/567a9418faa94140a2ea25a81cea098bce5625b8)?

I've verified this with the i3status-rs case you provided (very helpfully, thank you!) and it works on my machine, but a second confirmation (that /tmp/cargo-update stays clean) would be great.

DCjanus commented 4 years ago

Example:

$ cargo install --git https://github.com/upstream/example.git --rev 123456
$ cargo install --git https://github.com/my_fork/example.git --rev 234567 
$ cargo install-update --all --git

Note: commit 123456 is parent of 234567

In this case, without cargo_hash, cargo-update would tell me, installed is 234567, latest is 123456, which is obviously wrong.

This should be fixed in https://github.com/nabijaczleweli/cargo-update/commit/567a9418faa94140a2ea25a81cea098bce5625b8. Thanks for your help and your great tool.

nabijaczleweli commented 4 years ago

Cool. Released in v4.0.0.