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

all: dividing go-cloud into more than one module #886

Closed
jba opened this issue Dec 4, 2018 · 10 comments
Closed

all: dividing go-cloud into more than one module #886

jba opened this issue Dec 4, 2018 · 10 comments
Assignees
Labels
enhancement New feature or request P1

Comments

@jba
Copy link
Contributor

jba commented Dec 4, 2018

Currently, this repo consists of a single module. At some point, we will want to take some packages beta (that is, v1), while leaving others at v0. That implies that we'll have at least two modules.

This issue tracks the decisions involved in using more than one module. It assumes we have a "vanity" import path, as indeed we soon will (gocloud.dev/...). With a vanity path, we have the option using more than one repo. For example, the import path gocloud.dev/blob could point to a repo just for blob (say, github.com/google/go-cloud-blob) while the import path gocloud.dev/pubsub could continue to refer to github.com/google/go-cloud/pubsub.

The disadvantages of using more than one repo are:

  • Administrative overhead to obtain each additional repo (a one-time cost)
  • Development overhead in maintaining multiple repos
  • The inability to make commits atomically across repos

The advantage is that we would avoid the problems with multi-module repos. There are two such problems:

  1. Developing code in a multi-module repo is tricky. The main problem is that git works in terms of commits—your working directory is always "at" some single commit—but in your program, each module in your repo may be at a different version (and therefore at a different commit). At present, there is no clean, supported way to manage this. See cmd/go: support simultaneous edits of interdependent modules golang/go#27542 for discussion.

  2. If one module is in a subdirectory of another, a "requires" cycle is needed. That complicates testing, and also, go mod tidy currently removes the cycle. See cmd/go: ensure that 'go mod tidy' and go get -u do not introduce ambiguous imports golang/go#27899.

Note that the first of these is a problem even if the modules are siblings.

We should monitor those two issues for improvements to the situation, and we should defer any decision on multiple modules as long as possible in the hope that things will get better.

@vangent vangent added this to the Unplanned milestone Dec 5, 2018
@tv42
Copy link

tv42 commented Feb 21, 2019

Another argument would be having fewer dependencies pulled in. At least with Go modules on go1.12rc1, the list of packages that had their metadata downloaded was very long, and the delay on my current LTE internet connection was long enough that I seriously considered not using this library and hardcoding the exact blob provider I plan to use.

@vangent vangent removed this from the Unplanned milestone Mar 4, 2019
@vangent vangent added the P1 label Mar 4, 2019
@eliben eliben added the enhancement New feature or request label Mar 4, 2019
@eliben
Copy link
Member

eliben commented Mar 8, 2019

I will take a stab at evaluating this, seeing if we can split off some of the heaviest dependencies first.

@eliben eliben self-assigned this Mar 8, 2019
@tamalsaha
Copy link

In the process, if you can divide this into multiple repos, that will be really great! For example, I am primarily interested in the blob store pkg. That will make it simpler for me or anyone else, I suppose.

@eliben
Copy link
Member

eliben commented Mar 8, 2019

@tamalsaha I'm curious to hear about your use case. Why would multiple repos simplify things for you? Are you worried about the dependencies pulled when go get-ing the CDK, or something else?

@tamalsaha
Copy link

Are you worried about the dependencies pulled when go get-ing the CDK, or something else?

  • Yes.

@eliben
Copy link
Member

eliben commented Mar 8, 2019

Are you worried about the dependencies pulled when go get-ing the CDK, or something else?

* Yes.

This is precisely what splitting into multiple modules attempts to address. AFAIU you shouldn't much care if it comes from the same github repo, as long as you only pull the dependencies you want.

@tamalsaha
Copy link

tamalsaha commented Mar 8, 2019

Yeah. Though I read this from @FiloSottile https://twitter.com/FiloSottile/status/1096165600890707973

@FiloSottile
Copy link
Contributor

FiloSottile commented Mar 8, 2019

My opinions on multi-module repositories are mostly informed by the opinions of @ianthehat, @bcmills and @jayconrod. Multi-repo (what's proposed here) is probably stricly better than multi-module repos.

Another argument would be having fewer dependencies pulled in.

This part will be mostly solved by proxies in Go 1.13, because only the go.mod file will be fetched for dependencies that you don't actually need to build.

eliben added a commit that referenced this issue May 13, 2019
…#2057)

Updates #2048 - moves the go.mod file from samples/appengine to samples.

Note the replace gocloud.dev => ../ added to the go.mod. Without this, running go test ./... in samples/ results in many errors like:

can't load package: package gocloud.dev/samples/server: unknown import path "gocloud.dev/samples/server": ambiguous import: found gocloud.dev/samples/server in multiple modules:
	gocloud.dev/samples (/home/eliben/eli/go-cloud/samples/server)
	gocloud.dev v0.13.0 (/home/eliben/eli/go/pkg/mod/[email protected]/samples/server)

The symptom and solution is explained by bcmills in ugorji/go#279 (which refers also to golang/go#27899). The new go.mod points to gocloud.dev v0.13, which also provides these packages - so the go command is confused - it sees the same package(s) provided by two different modules.

The ugorji/go solution was to use a pseudo-version pointing at an existing commit in the core module which removes the packages - this removes the ambiguity. In our case, there is no existing commit yet - so I'm using a replace line. The replace line should be unnecessary when we release a new CDK version.

This has interesting implications for #886 - we'll likely have to do the same when we split out providers to their own modules and retain replace lines until a new release.
eliben added a commit that referenced this issue May 28, 2019
eliben added a commit that referenced this issue May 30, 2019
BREAKING_CHANGE_OK

Bump go.mod requires to future 0.15 version

Updates #886
@eliben
Copy link
Member

eliben commented May 30, 2019

Fixed with #2189

@eliben eliben closed this as completed May 30, 2019
@vangent
Copy link
Contributor

vangent commented May 30, 2019

\o/ thanks for all the hard work on this Eli!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P1
Projects
None yet
Development

No branches or pull requests

6 participants