ringrtc icon indicating copy to clipboard operation
ringrtc copied to clipboard

ringrtc tracks libsignal on git, introduces duplicate dependency

Open rubdos opened this issue 1 year ago • 8 comments

ringrtc depends (currently) on:

zkgroup = { git = "https://github.com/signalapp/libsignal", tag = "v0.37.0" }

Signal Android depends (currently) on:

version("libsignal-client", "0.51.1")

On applications that use FFI, this doesn't really matter, because ringrtc and libsignal get pulled in independently. However, Rust applications that pull in ringrtc and libsignal in the same Rust dependency tree yield a conflict on cargo doc:

error: There are multiple `zkgroup` packages in your project, and the specification `zkgroup` is ambiguous.
Please re-run this command with one of the following specifications:
  https://github.com/signalapp/libsignal#[email protected]
  https://github.com/signalapp/libsignal#[email protected]

Keeping the ringrtc git tag version spec in sync with apps is, in my opinion, only a band-aid here. Releasing on crates.io (https://github.com/signalapp/libsignal/issues/490) would probably resolve this correctly, because it allows to track the versions of the actual crates, instead of depending on a git tag. I'll submit a PR for bumping libsignal on this repo now.

I'm not sure whether I should post this issue here or on libsignal itself. It might make more sense over there...

rubdos avatar Jun 26 '24 10:06 rubdos

Teeeechnically, since we haven't been bumping the version for the zkgroup crate and changes usually aren't breaking, I would expect you to be able to [patch] around this situation. But it is kind of annoying, yeah, and I haven't actually tried it.

jrose-signal avatar Jul 08 '24 21:07 jrose-signal

would expect you to be able to [patch] around this situation

I think the [patch] section does not allow disambiguating which zkgroup reference to patch, although in my case "patch-em-all" would be fine. We can currently just use 0.51.1; on the next libsignal-protocol bump I'll test this out.

rubdos avatar Jul 09 '24 09:07 rubdos

Another effect:

error: failed to resolve patches for `https://github.com/signalapp/libsignal/`

Caused by:
  patch for `zkgroup` in `https://github.com/signalapp/libsignal/` points to the same source, but patches must point to different sources

There's a trick though:

[patch."https://github.com/signalapp/libsignal/"]
# The source and target refs are the same URL. If we add another / in there,
# cargo sees this as a valid patch.  Very ugly workaround.
# https://github.com/rust-lang/cargo/issues/5478#issuecomment-1055365283
zkgroup = { git = "https://github.com/signalapp//libsignal", tag = "v0.56.1" }

... but that duplicates all transitive dependencies, yielding to other conflicts. Not ideal for now...

rubdos avatar Sep 09 '24 14:09 rubdos

Definitely not ideal. As a workaround, though, you could patch your own dependencies the same way, though, correct?

jrose-signal avatar Sep 12 '24 00:09 jrose-signal

Definitely not ideal. As a workaround, though, you could patch your own dependencies the same way, though, correct?

Yes, correct indeed, that's what I went with for now:

[patch."https://github.com/signalapp/libsignal/"]
# The source and target refs are the same URL. If we add another / in there,
# cargo sees this as a valid patch.  Very ugly workaround.
# https://github.com/rust-lang/cargo/issues/5478#issuecomment-1055365283
zkgroup = { git = "https://github.com/signalapp//libsignal", tag = "v0.56.1" }
libsignal-core = { git = "https://github.com/signalapp//libsignal", tag = "v0.56.1" }
signal-crypto = { git = "https://github.com/signalapp//libsignal", tag = "v0.56.1" }

rubdos avatar Sep 12 '24 06:09 rubdos

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 18 '24 07:12 stale[bot]

Marking this as acknowledged but we haven't yet come up with anything better.

jrose-signal avatar Dec 18 '24 18:12 jrose-signal

Marking this as acknowledged but we haven't yet come up with anything better.

thank you, much appreciated.

rubdos avatar Dec 19 '24 09:12 rubdos