search

radicle-dev / radicle-link

The second iteration of the Radicle code collaboration protocol.
Other
423 stars 39 forks source link

fix git2-rs patch not getting used #676

Closed kim closed 3 years ago

kim commented 3 years ago

Cargo patch won't use the patched version if it can satisfy the dependency in other ways, e.g. via a newer version of the crate. Due to the wonders of caret dependencies, we thus need to pin git2-rs to the version used in the patch override.

kim commented 3 years ago

I don't see the cargo patch applied in bins nor upstream, is that deliberate? /cc @alexjg

alexjg commented 3 years ago

Not exactly deliberate. I was waiting to merge the changes in link and then forgot to put the changes in to upstream and bins :grimacing: . Will sort that now.

kim commented 3 years ago

It is probably safe to assume cargo will pick the most restrictive bound iff all bounds are semver-compatible. This is most likely a footgun due to the PoW rule (git2-rs’ version starts with a zero).

I’m thinking, however, that we might also get away with pinning the version for librad (that is, not use the cargo patch but a git dep) and live with duplicate versions in applications.

FintanH commented 3 years ago

It is probably safe to assume cargo will pick the most restrictive bound iff all bounds are semver-compatible. This is most likely a footgun due to the PoW rule (git2-rs’ version starts with a zero).

Ya, fair enough. That sounds right.

I’m thinking, however, that we might also get away with pinning the version for librad (that is, not use the cargo patch but a git dep) and live with duplicate versions in applications.

:cowboy_hat_face:

alexjg commented 3 years ago

I'll switch this over to use the git dep then, that would also mean we don't need to do anything in downstream crates right?

kim commented 3 years ago

switch this over to use the git dep then

I just tried this and the following happened: cargo treats a git dep as both the same crate and not the same crate!

That is, dependency resolution fails if there is some other dependency on the same crate, but from a different repository (that is, crates.io). Presumably, that's because it doesn't honor the version of the git dep, and it's not possible to force it to a version by stating it in the dependencies stanza.

Iow, we'd need to update all direct dependencies on git2 in this workspace to use the git rev, and then upstream still fails to resolve because radicle-surf has a normal version constraint.

What a clusterf**k.

alexjg commented 3 years ago

Ouch. So to get this working all downstreams will need to add the git dependency as well?

kim commented 3 years ago

Yeah pretty much. I think the cargo patch approach is probably better (only two places to update), and then hope we can move on to gitoxide quickly.