go: cmd/go: go mod init doesn't import nested module, tidy picks older version

To shed a little more light on the situation here the Consul repo contains 3 modules.

github.com/hashicorp/consul github.com/hashicorp/consul/api github.com/hashicorp/consul/sdk

Both the api and sdk modules are nested under the root module. One big purpose we use the nested modules for is to limit the dependencies necessary for the Consul API client (the api module). Consul itself has many large dependencies that we don’t want to require everyone who uses our API client to have to download and compile.

Our general strategy for modules regarding releases is:

1. Tag the sdk module if it had updates and then update the api and root modules dependencies on it.
2. Tag the api module if it had updates and then update the root modules dependency on it.
3. Tag the root module.

The versions of the sdk and api modules do not correspond to the Consul version. There will be plenty of releases where neither of those two modules have updates and we aren’t bumping the version. As of right now the api/v1.1.0 tag represents the latest API client and would be fully compatible with the v1.5.3 tag of the root module.

With regards to the nested module situation, it has been a source of pain for us since implementing it. So much so that we having been considering ways to automate away the nesting.

The root problem I see is that we want to be able to PR changes to our public facing API and the corresponding changes to the API client all at once. This means that both bits of code must live in the same repository (at least at the time of the PR). What we are trying to avoid is leaking all of the root modules dependencies to everyone who wants to pull in the API client.

One solution I have been thinking through is getting rid of the nested module in the same repo and instead at release time pushing the API client code to a secondary repository where the go.mod would live. This would:

1. Maintain the development workflow of 1 PR to touch both the public API and its API client
2. Allow a smaller set of dependencies for those pulling in just the API client
3. Work around a handful of issues with various Go commands really not working quite right with nested modules (requiring replace directives in the root modules go.mod to prevent pulling the nested modules from github instead of using whats we already have, preventing issues with go mod vendor attempting to vendor our own nested modules and there are others)
4. Be able to maintain better backwards compatibility with older versions of Go in the separate repo.

The downside is that many of our users would need to change the import path or now be required to pull in all of the root modules dependencies. I wrote a tool to automate fixing import paths though so the burden on our users would be minimal.

@rogpeppe: as far as I can tell, there are several conditions that must all be met in order for the version to actually regress:

1. The user code must have a legacy lockfile that specifies versions at the repo level.
2. The dependency repo at the commit named in the lockfile must contain a nested module.
   • And the user code must import a package from within the nested module.
3. At that commit, the module at the root of the repo must not require the nested module at an equivalent version (that is, a version for which the code within the nested module has the same behavior as at the named commit).
4. The latest version of the nested module must be before the named commit. a. The nested module must have a release (or pre-release) tag. b. The named commit must be after the latest tag.
   • This implies that the user code is relying on behavior without a guarantee of stability.

It certainly is possible to meet those conditions, because you presumably would not have filed this issue otherwise. But I doubt that they co-occur often in practice.

Perhaps we can do something more sophisticated here. We could walk the import graph, figure out what version or commit each package should have been required at, then try to reverse-engineer a go.mod file that produces the same build list.

In general, the module configuration converted from another dependency manager will often require adjustment anyway: for example, some dependency managers have operated at the package (rather than repo) granularity, and migrating those to modules ends up bumping the module version for all of those packages upward to the package with the highest requirement, which can end up pulling in breaking changes.

So it’s probably more useful to view the converted go.mod as a “roughed-in” configuration rather than a high-fidelity equivalent, and given that, addressing this issue seems like it would be a lot of work (and a lot of complexity) in order to address a transitional problem — and hopefully a rare one even then.

This seems like a problem with importing from glide.lock, rather than go mod tidy.

Before you run go mod tidy, only github.com/hashicorp/consul is required. github.com/hashicorp/consul/api is imported, but not required. Any build command will add a requirement on the latest version, which is is v1.1.0.

$ cat go.mod
module m

go 1.13

require github.com/hashicorp/consul v1.5.1

$ go list .
m

$ cat go.mod
module m

go 1.13

require (
	github.com/hashicorp/consul v1.5.1
	github.com/hashicorp/consul/api v1.1.0
)

go mod tidy will add the requirement on github.com/hashicorp/consul/api, but it will also remove any requirements on modules that aren’t transitively imported, so github.com/hashicorp/consul is removed.

When go mod init imports from another package manager, it does a pretty simple translation. I don’t think there’s any package manager that supports Go nested modules, so the glide.lock file will just say the repository is required at tag v1.5.2, including everything in the api subdirectory.

Perhaps we can do something more sophisticated here. We could walk the import graph, figure out what version or commit each package should have been required at, then try to reverse-engineer a go.mod file that produces the same build list. This is close to what go get does now, so it’s not infeasible (though I’m sure there are cases where MVS can’t produce the same result), but it’s a bit of work.

@rogpeppe

github.com/hashicorp/consul/api v1.1.1-0.20190522201912-40cec98468b8

…/api v1.5.1-… ?