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.

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

avoid duplicate definition of crates when pulling from git #1784

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baloo
Copy link
Member

@baloo baloo commented Mar 7, 2025

This allows to pull a git dependency from outside this workspace without having to also pull all the transitive dependencies from within this workspace.
Otherwise, the transitive dependency gets duplicated and you end up with objects not implementing trait error that are hard to debug.

For example, if you pull aead = { git = "https://.../traits.git" } you end up with two definitions of crypto-common, one from crates.io and one from git.
This causes issues because the objects you then pass to the aead traits do not implements the required traits from the crypto-common crates.

@newpavlov
Copy link
Member

I am not sure I like this approach. It also may create issues with crate publishing. Do you have an example of it being used elsewhere?

@baloo
Copy link
Member Author

baloo commented Mar 7, 2025

I am not sure I like this approach. It also may create issues with crate publishing. Do you have an example of it being used elsewhere?

We've been using that scheme in formats.git for a while now, I'm not aware of any issues.

This triggered in RustCrypto/AEADs#662
you should be able to reproduce with this patch:

diff --git a/Cargo.toml b/Cargo.toml
index eaef01b488..4b7440538d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -19,7 +19,6 @@
 aes-gcm     = { path = "./aes-gcm" }

 aead          = { git = "https://github.com/RustCrypto/traits.git" }
-crypto-common = { git = "https://github.com/RustCrypto/traits.git" }

 aes = { git = "https://github.com/RustCrypto/block-ciphers.git" }

@baloo baloo force-pushed the baloo/duplicate-definitions branch from 7db3024 to eeab1c9 Compare March 7, 2025 18:23
This allows to pull a git dependency from outside this workspace
without having to also pull all the transitive dependencies from
within this workspace.
Otherwise, the transitive dependency gets duplicated and you end
up with objects not implementing trait error that are hard to debug.

For example, if you pull aead = { git = "https://.../traits.git" }
you end up with two definitions of crypto-common, one from crates.io
and one from git.
This causes issues because the objects you then pass to the aead traits
do not implements the required traits from the crypto-common crates.
@baloo baloo force-pushed the baloo/duplicate-definitions branch from eeab1c9 to 8c19714 Compare March 7, 2025 18:34
@newpavlov
Copy link
Member

This triggered in RustCrypto/AEADs#662 you should be able to reproduce with this patch:

I don't think that having this additional line during transitory stages is problematic. We still need to patch a bunch of other dependencies and I think it makes it easier to track which dependencies we need to (pre-)publish for the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants