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

docs: add ADR 053 Go Module Refactoring #11799

Merged
merged 5 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/architecture/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,4 @@ When writing ADRs, follow the same best practices for writing RFCs. When writing

- [ADR 044: Guidelines for Updating Protobuf Definitions](./adr-044-protobuf-updates-guidelines.md)
- [ADR 047: Extend Upgrade Plan](./adr-047-extend-upgrade-plan.md)
- [ADR 053: Go Module Refactoring](./adr-053-go-module-refactoring.md)
109 changes: 109 additions & 0 deletions docs/architecture/adr-053-go-module-refactoring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# ADR 053: Go Module Refactoring

## Changelog

* 2022-04-27: First Draft

## Status

PROPOSED

## Abstract

The current SDK is built as a single monolithic go module. This ADR describes
how we refactor the SDK into smaller independently versioned go modules
for ease of maintenance.

## Context

Go modules impose certain requirements on software projects with respect to
stable version numbers (anything above 0.x) in that [any API breaking changes
necessitate a major version](https://go.dev/doc/modules/release-workflow#breaking)
increase which technically creates a new go module
(with a v2, v3, etc. suffix).

[Keeping modules API compatible](https://go.dev/blog/module-compatibility) in
this way requires a fair amount of fair thought and discipline.

The Cosmos SDK is a fairly large project which originated before go modules
came into existence and has always been under a v0.x release even though
it has been used in production for years now, not because it isn't production
quality software, but rather because the API compatibility guarantees required
by go modules are fairly complex to adhere to with such a large project.
Up to now, it has generally been deemed more important to be able to break the
API if needed rather than require all users update all package import paths
to accommodate breaking changes causing v2, v3, etc. releases. This is in
addition to the other complexities related to protobuf generated code that will
be addressed in a separate ADR.

Nevertheless, the desire for semantic versioning has been [strong in the
community](https://github.com/cosmos/cosmos-sdk/discussions/10162) and the
single go module release process has made it very hard to
release small changes to isolated features in a timely manner. Release cycles
often exceed six months which means small improvements done in a day or
two get bottle-necked by everything else in the monolithic release cycle.

## Decision

To improve the current situation, the SDK is being refactored into multiple
go modules within the current repository. There has been a [fair amount of
debate](https://github.com/cosmos/cosmos-sdk/discussions/10582#discussioncomment-1813377)
as to how to do this, with some developers arguing for larger vs smaller
module scopes. There are pros and cons to both approaches (which will be
discussed below in the [Consequences](#consequences) section), but the
approach being adopted is the following:
* a go module should generally be scoped to a specific coherent set of
functionality (such as math, errors, store, etc.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth mentioning that each SDK module will be its own go module too?

* when code is removed from the core SDK and moved to a new module path, every
effort should be made to avoid API breaking changes in the existing code using
aliases and wrapper types (as done in https://github.com/cosmos/cosmos-sdk/pull/10779
and https://github.com/cosmos/cosmos-sdk/pull/11788)
* new go modules should be moved to a standalone domain (`cosmossdk.io`) before
being tagged as `v1.0.0` to accommodate the possibility that they may be
better served by a standalone repository in the future
* all go modules should follow the guidelines in https://go.dev/blog/module-compatibility
before `v1.0.0` is tagged and should make use of `internal` packages to limit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For x/* go modules, there some probably more guidelines, right? Some unanswered questions I have, which I think could go into this ADR:

  • do state-machine breaking changes require a major bump?
  • client-breaking changes?
  • CLI?

the exposed API surface
* the new go module's API may deviate from the existing code where there are
clear improvements to be made or to remove legacy dependencies (for instance on
amino or gogo proto), as long the old package attempts
to avoid API breakage with aliases and wrappers
* care should be taken when simply trying to turn an existing package into a
new go module: https://github.com/golang/go/wiki/Modules#is-it-possible-to-add-a-module-to-a-multi-module-repository.
In general, it seems safer to just create a new module path (appending v2, v3, etc.
if necessary), rather than trying to make an old package a new module.
Comment on lines +73 to +74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is a bit confusing to me. It seems for errors, we did create a new module from an old package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing that's not clear for me is: when bumping a major version, do we:

  • rename in go.mod cosmossdk.io/foo -> cosmossdk.io/foo/v2, and tag foo/v2.0.0
  • or no rename in go.mod, simply tag foo/v2.0.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that go.mod module directive has to be updated, but moving code to subdirectory is optional:

If the module is released at major version 2 or higher, the module path must end with a major version suffix like /v2. This may or may not be part of the subdirectory name. For example, the module with path golang.org/x/repo/sub/v2 could be in the /sub or /sub/v2 subdirectory of the repository golang.org/x/repo.

ref: https://go.dev/ref/mod


## Consequences

### Backwards Compatibility

If the above guidelines are followed to use aliases or wrapper types pointing
in existing APIs that point back to the new go modules, there should be no or
very limited breaking changes to existing APIs.

### Positive

* standalone pieces of software will reach `v1.0.0` sooner
* new features to specific functionality will be released sooner

### Negative
Copy link
Contributor

@amaury1093 amaury1093 Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently SDK modules depend on other modules' public APIs. With independent go module versioning, we might need to have compatibility tables like staking@v5 is compatible with bank@{v3,v4} but not with bank@v2 etc, is that correct?

It seems this can be elegantly solved by ADR-033 and the api/ folder (no go API compatibility, on proto comptability), but until then, is that a concern to you too @aaronc ?


* there will be more go module versions to update in the SDK itself and
per-project, although most of these will hopefully be indirect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another negative one is that I think we need tooling for publishing go modules. Similar to lerna for go modules.

Say foo@1.0.0 has a security fix, so we release foo@1.0.1. And we want a new patch version for each of {bar,baz,qux} go modules. Currently we would need to update deps manually (or using dependabot), and then tag these individually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a Tooling subsection would actually make sense, but can be left empty for this version of the ADR, since this ADR is still only proposed.

### Neutral

## Further Discussions

Further discussions are occurring in primarily in
https://github.com/cosmos/cosmos-sdk/discussions/10582 and within
the Cosmos SDK Framework Working Group.

## References

* https://go.dev/doc/modules/release-workflow
* https://go.dev/blog/module-compatibility
* https://github.com/cosmos/cosmos-sdk/discussions/10162
* https://github.com/cosmos/cosmos-sdk/discussions/10582
* https://github.com/cosmos/cosmos-sdk/pull/10779
* https://github.com/cosmos/cosmos-sdk/pull/11788