Skip to content

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.

@baloo
Copy link
Member Author

baloo commented Mar 10, 2025

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.

I won't fight over this. Your call.

@baloo baloo closed this Mar 10, 2025
@baloo baloo deleted the baloo/duplicate-definitions branch April 18, 2025 14:08
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