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

Repo split #2639

Closed
7 tasks
hu55a1n1 opened this issue Sep 14, 2022 · 16 comments
Closed
7 tasks

Repo split #2639

hu55a1n1 opened this issue Sep 14, 2022 · 16 comments
Assignees
Labels
O: code-hygiene Objective: cause to improve code hygiene
Milestone

Comments

@hu55a1n1
Copy link
Member

hu55a1n1 commented Sep 14, 2022

Summary

It has been proposed that we split the ibc-rs monorepo into two repos - one for the proto & module crates and the other for the relayer crate (and its associated crates).
Additionally, the relayer code must not depend on the ibc crate to have clear boundaries/separation and allow the modules to evolve independently.

Problem Statement

The monorepo approach is restrictive and becoming difficult to maintain. (See #1437 (comment))
Moreover, having the relayer crate depend on the ibc crate makes it difficult to modify one without touching the other, and have resulted in code bloat and unclear boundaries - for e.g. the modules code contains relayer specific code.

Proposal

Using the ideas described in #2541 and #1437, the repo hosting the relayer crates (the relayer repo) would look like so ->

<relayer>
├── CHANGELOG.md
├── CONTRIBUTING.md
├── Cargo.lock
├── Cargo.toml
│
├── crates
│	├── ibc-relayer
│	├── ibc-relayer-cli
│	├── ibc-relayer-rest
│	├── ibc-telemetry
│	└── ibc-chain-registry 
│
├── etc...

And the modules repo would look like so ->

<modules>
├── CHANGELOG.md
├── CONTRIBUTING.md
├── Cargo.toml
│
├── crates
│	├── ibc
│	└── proto
│		├── ibc-proto
│		└── ibc-proto-compiler 
│
├── etc...

Acceptance Criteria

The ibc-rs repo is split into two repos as proposed above, ideally preserving history.

TODOs

  • Rename the default branches of both repos from master to main
  • Split gm out of the repository

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@hu55a1n1 hu55a1n1 added the O: code-hygiene Objective: cause to improve code hygiene label Sep 14, 2022
@hu55a1n1 hu55a1n1 self-assigned this Sep 14, 2022
@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Sep 15, 2022

Detailed proposal

Below is a more detailed proposal and an (opinionated) action plan ->

  • Phase 1 - Split the repositories into two with a passing CI.
  • Phase 2 - GitHub related settings and crate metadata separation.
  • Phase 3 - Copy module code into relayer crate so that it no longer depends on the modules.

Phase 1

IMO, the best way to extract a directory in a git repo into its own repo is to use git-filter-repo - this is also the officially recommended tool for this purpose both by git and GitHub.

Creating the modules repo

The following commands clone the current monorepo and only keep the desired directories while (almost) preserving the
relevant history ->

git clone https://github.com/informalsystems/ibc-rs.git
cd ibc-rs/

git filter-repo --path-rename modules:crates/ibc \
				--path-rename proto:crates/proto/ibc-proto \
				--path-rename proto-compiler:crates/proto/ibc-proto-compiler \
				--path ci/no-std-check/ \
				--path .github/workflows/no-std.yaml \
#			    ...

This leaves us with a repo structure like so ->

<modules>
├── crates
│	├── ibc
│	└── proto
│		├── ibc-proto
│		└── ibc-proto-compiler 
│
├── ci
│	└── no-std-check
│
├── .github
│	└── workflows
│	    └── no-std.yaml
├── ...

Note that we're also moving the crates into a crates directory.
Many of the remaining files will have to be modified to remove relayer related stuff.

Creating the relayer repo

This is more tricky and there are multiple ways to go about it ->

  1. Rename the current ibc-rs repo and delete everything that is module related. This is my preferred approach.
  2. Use filter-repo with --invert-paths to keep only relayer related code and create a new repo with it.
  3. Use filter-repo with --invert-paths to keep only relayer related code and push to the current ibc-rs repo.

The benefit of option 1 is that we get to keep the current ibc-rs repo on GitHub along with all issues, PRs, etc. and
users can continue to contribute to it with without major local rebases or re-clones because we aren't rewriting
history. The drawback here is that the relayer repo will continue to inherit the history of the module related code.
With a fresh clone, the repo size is 39MB and after deleting module related code we get 37MB. With options 2 & 3
however, we get 33MB. So we do save ~4MB, but I don't think the tradefoffs are worth it.

Options 2 and 3 are similar to what was described in the previous section, so here we would be rewriting history. Both
approaches have significant overhead on the part of devs/users to continue to contribute to/use the codebase.
The git-filter-repo
manual has a detailed discussion on this approach ->

You’ll need to carefully synchronize with everyone who has cloned the repository, and will also need to carefully
synchronize with everything (e.g. CI systems) that has cloned it. Every single clone will either need to be thrown
away and re-cloned, or need to take all the steps outlined in item 4 as well as follow the necessary steps from
"RECOVERING FROM UPSTREAM REBASE" section of git-rebase(1). If you miss fixing any clones, you’ll risk mixing old and
new history and end up with an even worse mess to clean up.

Option 1 essentially means running something like ->

git rm -rf modules proto proto-compiler ci/no-std-check/ .github/workflows/no-std.yaml # ...

We then move the crates into a crate directory and remove all references to module-related code until we end up with a
directory structure like so ->

<relayer>
├── CHANGELOG.md
├── CONTRIBUTING.md
├── Cargo.lock
├── Cargo.toml
│
├── crates
│	├── ibc-relayer
│	├── ibc-relayer-cli
│	├── ibc-relayer-rest
│	├── ibc-telemetry
│	└── ibc-chain-registry 
│
├── etc...

Note: At this point the relayer crates are depending on up-stream versions of the modules crates. The plan is to address this in phase 3.

Phase 2

This involves the following steps -

  • Create a GitHub modules repo.
  • Apply branch-protection and other GitHub settings.
  • Setup CI, issue/PR templates (copy from relayer repo), etc.
  • Transfer modules related issues/discussions from relayer repo to modules repo.

Naming

I would personally prefer to name the modules repo as ibc-rs because I think this consistent with how repositories are
named in the interchain ecosystem, for e.g. ibc is the spec repo, ibc-go is the go implementation. For the relayer
repo I like hermes because that is the central product/bin provided by that repo and everything else there is
supporting the bin.

That said, with the current proposal this naming implies that the current ibc-rs repo would be
renamed to hermes and there would be a new repo ibc-rs that would host the modules. While reusing repo names is
allowed b Github, it might be confusing for users, so maybe we should think about a different name for the modules
repo (ibc-rust?).

ADRs

While most ADRs address specific concerns and can be moved to their respective repos, some of them are relevant for both
the relayer and modules/proto. I would we keep such ADRs in the relayer repo.

CHANGELOG

I would suggest we do not modify the CHANGELOG.md and .changelog entries in the relayer repo and start with a fresh
changelog in the modules repo.

docs/spec

The docs/spec directory contains some docs and TLA+ specs related to both the relayer and modules. It is unclear if
these specs are up-to-date and being used. I would suggest we keep them in the relayer repo for now.

Protobuf

There have been recent discussions on the way the protobuf crate is structured, versioning of raw/domain types, usage
of buf, etc. I would suggest we move the ibc-proto crate as is to the modules repo and discuss this further on
@mzabaluev's upcoming ADR.

References

@mzabaluev
Copy link
Contributor

I think the history should be preserved unchanged in both repositories after the split (without making it an official fork on GitHub). The repo size of 39 MB is not something I would worry about.

This way, anybody with an outstanding branch can rebase or merge against either or both the new repositories.

@mzabaluev
Copy link
Contributor

I would also take exception to ibc-proto-compiler and move it under tools, like in tendermint-rs.

@hu55a1n1
Copy link
Member Author

I think the history should be preserved unchanged in both repositories after the split (without making it an official fork on GitHub).

With my current proposal, the history will be preserved unchanged for the relayer repo. The module repo will also preserve the history but only for the modules, proto & proto-compiler crates, because we're using git-filter-repo. I like to use git-filter-repo for the modules repo because it reduces the repo size to ~8.5MB, the history is cleaner and a rebase should still be possible in most cases. Here's what the resulting module repo history would look like -> https://github.com/hu55a1n1/ibc-rust/commits/master

I would also take exception to ibc-proto-compiler and move it under tools, like in tendermint-rs.

Do you mean we move the ibc-proto-compiler to crates/tools/ibc-proto-compiler or tools/ibc-proto-compiler?

@hu55a1n1 hu55a1n1 mentioned this issue Sep 19, 2022
7 tasks
@mzabaluev
Copy link
Contributor

mzabaluev commented Sep 19, 2022

Do you mean we move the ibc-proto-compiler to crates/tools/ibc-proto-compiler or tools/ibc-proto-compiler?

To tools/ibc-proto-compiler. Also, all library crates would be on the same level under crates.
The logic here is that all production code is hosted under crates, and everything auxiliary that is used to build or maintain the production code goes under tools.

In tendermint-rs, tools is also an independent Cargo workspace with its own Cargo.lock that is version-controlled; I think it makes sense particularly for ibc-proto-compiler, which needs a reproducibility anchor for the code that is generated for ibc-proto.

@plafer
Copy link
Contributor

plafer commented Sep 19, 2022

Two minor thoughts:

  1. I think we should use ibc-rs for the modules, and have a statement in the README that redirects to the hermes repo and explains the transition that happened. In the long run, we'll be happy to have "the correct name" for the repo
  2. Wouldn't it be better to have proto be its own repository? I find it cleaner for people who look for "ibc-rs" to just fall right on the "handlers repo" rather than a monorepo that also includes a proto repo.

@mzabaluev
Copy link
Contributor

Wouldn't it be better to have proto be its own repository?

I'm generally against splitting crates that are tightly coupled, like handlers and ibc-proto need to be. This leads to increased developer logistics when changes are introduced. Having to synchronize a change across two repositories (ibc-rs and hermes) is already a burden, but at least there are good reasons for this split, including the tentative idea to get rid of code reuse between modules and hermes.

@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Sep 21, 2022

As discussed with @plafer and @romac, we decided to keep the proto crates in a different repository for now since both the relayer and modules currently depend on the raw types. With the split this means each repo will have it's own version(s) of the domain types.

We seem to have consensus on hermes as the relayer repo's name, but we're still undecided on the modules repo's name as that topic seems more nuanced than expected. ibc-rs seems like the favourite (followed by ibc-rust) but we first need to answer the following questions before we can take a decision on this ->

  • What happens when someone pushes to a remote that has been renamed?
  • What if the remote's old name has been reused?
  • Are there long-lived forks/branches of hermes? (looks like there are ~5 projects that have recently updated forks)
  • Name of forks wont change, so could this be a problem?
  • What about links to ibc-rs in external repos?

@adizere
Copy link
Member

adizere commented Sep 21, 2022

An alternative is to create a repo cosmos/ibc-rs to host the modules in Rust. Context I can provide here to inform the decision is that this alternative would work towards incentivizing external contribution to modules, especially since that part of our work is more outside oriented. Drawback is we lose the "Informal brand".

I think hosting the Hermes repo under informalsystems/hermes (renamed and redirected automatically from informalsystems/ibc-rs) makes a lot of sense and I have no input there.

@hu55a1n1
Copy link
Member Author

Regarding the naming of the proto repo, I see that we already have an archived ibc-proto repository. (https://github.com/informalsystems/ibc-proto) I would prefer to reuse it but just wanted to ask if anyone has other ideas.

@romac
Copy link
Member

romac commented Sep 22, 2022

Yeah let's re-use that repo 👍🏻

@romac
Copy link
Member

romac commented Sep 22, 2022

While we're at it, we should use this opportunity to rename the default branch from master to main as per the new Git conventions: https://github.com/github/renaming

@greg-szabo
Copy link
Member

I would use this event to break out gm from under IBC so it can start becoming its own tool.

hu55a1n1 added a commit to cosmos/ibc-proto-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Add workspace Cargo.toml

Fix github workflow

Add dependabot.yml

Move ibc-proto crate to the root instead of using a workspace

Add license file

Fix links to point to cosmos/ibc-rs and cosmos/ibc-proto-rs

Fix workflow triggers to use main branch

Update MSRV

Add sync-protobuf.sh script
hu55a1n1 added a commit to cosmos/ibc-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory
hu55a1n1 added a commit to cosmos/ibc-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory

Bump tendermint-rs dependencies to v0.25.0
hu55a1n1 added a commit to cosmos/ibc-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory

Bump tendermint-rs dependencies to v0.25.0

Use cosmos/ibc-proto-rs patch in ci/no-std-check

Fix clippy errors
hu55a1n1 added a commit to cosmos/ibc-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory

Bump tendermint-rs dependencies to v0.25.0

Use cosmos/ibc-proto-rs patch in ci/no-std-check

Fix clippy errors

Undo no-std-check use deletions
hu55a1n1 added a commit to cosmos/ibc-rs that referenced this issue Sep 29, 2022
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory

Bump tendermint-rs dependencies to v0.25.0

Use cosmos/ibc-proto-rs patch in ci/no-std-check

Fix clippy errors

Undo no-std-check use deletions

Fix Cargo.tomls
@adizere
Copy link
Member

adizere commented Oct 28, 2022

I would use this event to break out gm from under IBC so it can start becoming its own tool.

Might also be a good idea to rename from gm = "gaia manager" to something more broad, e.g., "local ibc/interchain testnet manager" or something along those lines. Not sure what acronym would that yield, lite comes to mind. CC @harveenSingh

@harveenSingh
Copy link
Contributor

Good idea but when we use the tool it's gm for example: gm start. Are you suggesting to change that as well?
Adding @greg-szabo

@adizere
Copy link
Member

adizere commented Oct 31, 2022

Good idea but when we use the tool it's gm for example: gm start. Are you suggesting to change that as well?
Adding @greg-szabo

Yes, basically rename the binary. @harveenSingh can you please create a separate issue in the gm repo to track renaming gm and continue this discussion? Thanks!
Curious what @greg-szabo thinks about renaming gm.

I will close this as the repo split is done.

@adizere adizere closed this as completed Oct 31, 2022
@adizere adizere added this to the v1.1 milestone Oct 31, 2022
seanchen1991 pushed a commit to informalsystems/cosmwasm-ibc that referenced this issue Aug 13, 2024
Add Cargo.lock to .gitignore

Update all GitHub workflows

Update Cargo.tomls to point to upstream ibc-proto

Add root level README.md

Add CONTRIBUTING.md

Rename branch master -> main in CONTRIBUTING.md

Add ADR dir

Fix links to point to cosmos/ibc-rs

Add license file

Fix workflow triggers to use main branch

Fix links

Fix workflow trigger paths

Remove references to hermes in docs

Update README.md

Removed Requirements section. I don't think it adds much value, and it would always fall behind (e.g. it was saying the latest Rust version is 1.60).
Update README.md

Remove the term "IBC module"
Update README.md
Update README.md
Update README.md
Fix dependabot.yml ibc crate directory

Bump tendermint-rs dependencies to v0.25.0

Use cosmos/ibc-proto-rs patch in ci/no-std-check

Fix clippy errors

Undo no-std-check use deletions

Fix Cargo.tomls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: code-hygiene Objective: cause to improve code hygiene
Projects
None yet
Development

No branches or pull requests

7 participants