search

frewsxcv / rust-crates-index

Rust library for retrieving and interacting with the crates.io index
https://docs.rs/crates-index/
Apache License 2.0
72 stars 37 forks source link

Checkout `HEAD` instead of `master` #53

Closed jtgeibel closed 3 years ago

jtgeibel commented 3 years ago

In cargo, HEAD is updated instead of master. This mismatch means that the two refs can drift, depending on when cargo and this library last fetched changes.

I believe this change will fix sunng87/cargo-release#224. In that bug, it is first confirmed (by fetching master) that a recently published crate is now available in the public index. Once the dependency is published, it should be safe to publish new crates which depend on it. However, when the next crate is published the local build fails because cargo is using a less recent HEAD, which was not updated by crates-index.

The change brings this implementation into alignment with src/bare_index.rs.

kornelski commented 3 years ago

Thanks

pksunkara commented 3 years ago

I am encountering the reverse situation. I am using the latest crates-index which updates the HEAD to check that the crate is published, but rust-analyzer (log) uses stable cargo which apparently is still failing.

pksunkara commented 3 years ago

Also, looks like cargo-release is having the same problem, https://github.com/sunng87/cargo-release/issues/224#issuecomment-790395825

jtgeibel commented 3 years ago

Yes, this PR didn't actually solve all the differences between how cargo sees the index and how this crate sees the index. My current theory is that it has to do with the .cache directory. It looks like the current BareIndex has support for reading the .cache directory, but I suspect we might also need to write there so that the next cargo invocation sees a fresh cache aligning with any changes that we've externally pulled down to HEAD. Or possibly update metadata somewhere so that cargo knows the cache is stale. I don't know how cargo currently manages this cache.

pksunkara commented 3 years ago

Yeah, I am also seeing that cargo is updating the origin/master tag.

pksunkara commented 3 years ago

@jtgeibel I found the issue. cargo actually reads the crates from origin/master in here leading to here and not origin/HEAD before cargo version 0.53.0 (or master). I think in 0.53.0, it's changed to read from origin/HEAD in here

For us to be backward compatible to work with all cargo versions, the fix should be also fetching the origin/master ref.

@kornelski Or if you can publish a new patch with recent path() getter commit, we should be able to fix it on our side.

jtgeibel commented 3 years ago

@pksunkara I'm not super familiar with the cargo codebase, but I believe the code you've linked to is related to dependency resolution, not interactions with the registry index.

The next stable release (0.53) will include a change such that if your Cargo.toml includes a git dependency, then instead of assuming the user wants the origin/master branch, it will default to origin/HEAD unless a branch is explicitly defined. My understanding is that this change in dependency resolution is so that for projects which have switched to main as their default branch, it will no longer be necessary to explicitly define branch = "main" in Cargo.toml and instead the default branch will be inferred from origin/HEAD.

The registry index code is located in /src/cargo/sources/registry/remote.rs, where GitReference::DefaultBranch defaults to HEAD.

kornelski commented 3 years ago

It's unclear to me what needs to be changed here. As far as I can tell reading HEAD is the correct way.

pksunkara commented 3 years ago

No it is not. I have built and run the old cargo locally with a lot of debugging and logging.

Cargo after updating the index reads the sha using this git resolve which returns the origin/master. I have manually checked this by not updating the origin/master refspec and logging the sha which gave me an old sha instead of latest origin/HEAD sha.

This sha is used to retrieve the crate info from git, which is why we are seeing the publish errors.

@kornelski the fix is to also fetch the origin/master (refs/heads/master:refs/remotes/origin/master) refspec after this statement.

pksunkara commented 3 years ago

The registry index code is located in /src/cargo/sources/registry/remote.rs, where GitReference::DefaultBranch defaults to HEAD.

You are linking to the latest 0.53.0 release which is okay. But people would still use lot of old cargo versions which reads origin/master.

If we want to say that this library only supports rust 1.53.0 onwards, that's okay with me. I would just request a new patch release so that I can build a work around on my tool (which uses this lib).