search

libp2p / rust-libp2p

The Rust Implementation of the libp2p networking stack.
https://libp2p.io
MIT License
4.43k stars 920 forks source link

identity: move to separate repository #4742

Open thomaseizinger opened 10 months ago

thomaseizinger commented 10 months ago

Description

I think we should move libp2p-identity to a separate repository. This requires a bit more effort because we need to duplicate some CI setup.

We purposely are very conservative when it comes to breaking changes in libp2p-identity because the entire libp2p Rust ecosystem depends on it. As a result, we want the API to very stable.

Having crates in a workspace makes breaking changes across crates easier. But if we don't want breaking changes anyway, moving it to another repository doesn't hurt much.

Motivation

Having libp2p-identity in the monorepo requires us to do use [patch.crates-io] for it: https://github.com/libp2p/rust-libp2p/blob/20e4b36c9a3b4a61dab7e853934b8b84d0a1734b/Cargo.toml#L123-L130

That is because multiaddr also depends on libp2p-identity which does not live in this monorepo. This makes releasing difficult because cargo semver-checks will rightfully fail every time we bump the libp2p-identity version.

Current Implementation

We are maintaining it within the mono-repo.

Are you planning to do it yourself in a pull request ?

Maybe

mxinden commented 9 months ago

Would removing it from the workspace work as well?

thomaseizinger commented 9 months ago

Would removing it from the workspace work as well?

Maybe but doesn't that just create more confusion? I.e. you would still not be able to make atomic changes across all crates but being in the same repo kind of suggests that you can.

kayabaNerve commented 9 months ago

Practically, your reasoning makes sense. Theoretically, I'm not a fan due to it scattering things around. I'd rather move multiaddr into the monorepo here (though multiaddr isn't a random lib yet part of its own eco so I won't claim that makes sense/is justifiable. Just sometimes annoying to have two ecos so co-dependent...).

If a new repository is to be created, I'd advocate it not be rust-libp2p-identity yet rust-libp2p-core (or similar, I know libp2p-core is technically taken), referring to a series of crates so stable so we don't end up with three repos/making libp2p-identity into another monorepo later.

thomaseizinger commented 9 months ago

If a new repository is to be created, I'd advocate it not be rust-libp2p-identity yet rust-libp2p-core (or similar, I know libp2p-core is technically taken), referring to a series of crates so stable so we don't end up with three repos/making libp2p-identity into another monorepo later.

We can always rename the repository if we find more things to stabilize. I don't see why we should optimise for that now.

In practice, the only crates that need to live in a separate repository are those that multiple other repos depend on. Technically, we could move it into multiaddr but I think that doesn't make sense semantically.

mxinden commented 9 months ago

Would removing it from the workspace work as well?

Maybe but doesn't that just create more confusion? I.e. you would still not be able to make atomic changes across all crates but being in the same repo kind of suggests that you can.

Say we want a change that touches both libp2p-identity and libp2p-kad, we could then have one pull request that alters both libp2p-identity and libp2p-kad. Isn't that an atomic change?

thomaseizinger commented 9 months ago

Would removing it from the workspace work as well?

Maybe but doesn't that just create more confusion? I.e. you would still not be able to make atomic changes across all crates but being in the same repo kind of suggests that you can.

Say we want a change that touches both libp2p-identity and libp2p-kad, we could then have one pull request that alters both libp2p-identity and libp2p-kad. Isn't that an atomic change?

Atomic changes are useful if you break interfaces. If the changes are backwards-compatible, there is no need for an atomic change, you can just make the change to libp2p-identity first, test it with a git-reference and then release it and update libp2p-kad later.

Because multiaddr lives outside of the repository, you cannot make an atomic breaking change to libp2p-identity in this repository as it would show up as two different versions and the code would not compile (assuming you bump the version in the repository accordingly, which is something that we've been doing so far).

mxinden commented 9 months ago

Because multiaddr lives outside of the repository, you cannot make an atomic breaking change to libp2p-identity in this repository as it would show up as two different versions and the code would not compile (assuming you bump the version in the repository accordingly, which is something that we've been doing so far).

Good argument. Thank you for bearing with me.

I have created https://github.com/libp2p/rust-libp2p-identity. @thomaseizinger can you push the crate there? Preference for maintaining git history for commit touching identity/, though fine without in case it is too involved.