[Spike] Soroban RPC Dependencies / Repo Refactor

jcx120 commented 1 year ago

The reasoning for the repo refactors goes several ways - simplifying the build process, providing better code-level link between the artifact and the source code, reducing development mistakes and "moving" the cross-repo dependencies into code.

Acceptance Criteria:

Provide a draft with alternate repo layout ideas which provide compile dependency as needed(rust, go, other languages) between the repositories, without sacrificing any of the automation, clarity and history accuracy. Account for how PR's and releases will work under the new layout, highlight any benefits, drawbacks, concerns related to the layout.

A few seed thoughts:

consider rename tosoroban-tooling repo which has current day cli, but open for other tools later, align external dependencies against this.
for rust, instead of importing crates, use gitmodule to compile against explicit source code
move soroban-rpc out of monorepo to it's own repo, consider 12 factor app principles for reference, such as one codebase per app

Include:

How to's on release, test, dependencies etc
obtain team review/consensus

leighmcculloch commented 1 year ago

Something we need to think about with doing this is will PRs opened against the CLI need to the RPC to always be updated to match the same dependency graph? If the answer is yes, we may find the RPC's dependency on stellar-core will significantly slow down development on the CLI, because it is currently much easier and faster to upgrade the env that the CLI is using, than it is to do same in stellar-core.

leighmcculloch commented 1 year ago

Something else to think about is that today we have a standard process that applies to all our Rust repos for releasing them. The process does some non-trivial things to ensure that packages are released in a good and consistent state to crates.io. It would be ideal to retain that shared process. This is only relevant if the CLI continues to be written in Rust.

leighmcculloch commented 1 year ago

instead of importing crates, use gitmodule to compile against explicit source code

It's unclear to me what this means. Is the idea to avoid using the Rust/Cargo crate system? That seems difficult. Is the idea to use path instead of version/git references for the dependencies? If so, a couple things to think about:

Cargo doesn't support nested workspaces, without some hackery. We've done that hackery in one repo (rs-soroban-env), so when you come to this, recommending asking for help.
Path dependencies have to be removed to publish the crate to crates.io, so any path dependencies are only usable during development. If you hope to use path dependencies for final builds, we won't be able to publish to crates.io.
The current release process makes an assumption that any path dependencies are a part of the same workspace. It does this during the verification/dry-run workflow. We'd need to think about how to support that.

tsachiherman commented 1 year ago

instead of importing crates, use gitmodule to compile against explicit source code

It's unclear to me what this means. Is the idea to avoid using the Rust/Cargo crate system? That seems difficult. Is the idea to use path instead of version/git references for the dependencies? If so, a couple things to think about:

Cargo doesn't support nested workspaces, without some hackery. We've done that hackery in one repo (rs-soroban-env), so when you come to this, recommending asking for help.

Path dependencies have to be removed to publish the crate to crates.io, so any path dependencies are only usable during development. If you hope to use path dependencies for final builds, we won't be able to publish to crates.io.

The current release process makes an assumption that any path dependencies are a part of the same workspace. It does this during the verification/dry-run workflow. We'd need to think about how to support that.

Sounds like we could benefit from spiking that one.

sreuland commented 1 year ago

I updated the description and converted this to a spike after this was triaged in sprint grooming discussion.

paulbellamy commented 1 year ago

sreuland commented 1 year ago

@tsachiherman , has this evolved past a spike into implementation, it looks like https://github.com/stellar/soroban-cli/pull/251 is per this ticket? If so, wondering if we can change the title to reflect it's now more than spike, keep the ticket open correct, as there is still the final cutover step to remove soroban rpc from go repo. can we add a sub-task to acceptance criteria to request the quickstart/jenkins changes needed for new soroban rpc home as well? I can assist on that task.

tsachiherman commented 1 year ago

How's about closing this one, and using https://github.com/stellar/go/issues/4679 as the "hub" for the rest of the child-tasks ? clearly, 4679 would need to be updated.

sreuland commented 1 year ago

ok, can we split up the 5 points from here between these to keep accurate, assuming zero-sum, i.e. put 2 pts here and then 3pts on #4679 and move that one to in-progress?

tsachiherman commented 1 year ago

That's sounds like a good idea. I'm not particular exactly on how much time we have left, but that's a great starting point.

stellar / go

[Spike] Soroban RPC Dependencies / Repo Refactor #4672