can't import v2: need semantic import versioning

[ukai]

with require github.com/bazelbuild/remote-apis v2.0.0 in go.mod go claims

go: finding github.com/bazelbuild/remote-apis v2.0.0
go: finding github.com/bazelbuild/remote-apis v2.0.0
go: errors parsing go.mod:
/workspace/go.mod:10: require github.com/bazelbuild/remote-apis: version "v2.0.0" invalid: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v2

with require github.com/bazelbuild/remote-apis/v2 v2.0.0

go: github.com/bazelbuild/remote-apis/[email protected]: go.mod has non-.../v2 module path "github.com/bazelbuild/remote-apis" (and .../v2/go.mod does not exist) at revision v2.0.0

I think remote-apis's go.mod should say module github.com/bazelbuild/remote-apis/v2.

cf: https://github.com/golang/go/wiki/Modules#semantic-import-versioning

[ola-rozenfeld]

Oh, right, I forgot to update the go.mod/go.sum! I should try migrating it to Go 1.13 as well.

Hmm, if I declare this to be v2.0.0 in the mod file, according to https://github.com/golang/go/wiki/Modules#semantic-import-versioning this means that from now on this will require a whole different package name for the import, one that includes the version, "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2/v2". But that's weird, because we split versions by directories already. Let me try and figure this out.

[ola-rozenfeld]

After reading more of https://github.com/golang/go/wiki/Modules, I'm not sure we did this right to begin with. Our current versioning plan for the repo is very Bazel-centric: we have a directory under build/bazel/remote/v2, so that we depend on version-specific targets in Bazel (and later, we plan to put an incompatible build/bazel/remote/v3 there, etc).

But for Go build, it seems that if we want to maintain multiple backwards-incompatible versions together in this repo, we will have to either:

• do it on separate branches, or
• have separate v2 and v3 directories under root with separate go.mod and go.sum files for each.

I'd very much like to avoid the separate branches; but I'm also a bit stumped as to how to pull off the second option without screwing up Bazel -- I'd like all Bazel imports to have the version as the suffix, not the prefix, and, more importantly, I'd like to not break any existing clients code, if possible.

Changing the go.mod to have module github.com/bazelbuild/remote-apis/v2 won't work, I think: this would mean that all clients would now have to import "github.com/bazelbuild/remote-apis/v2/build/bazel/remote/execution/v2", right?

WDYT?

[ukai]

I think changing go.mod to have module github.com/bazelbuild/remote-apis/v2 and change go import path from "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" to "github.com/bazelbuild/remote-apis/v2/build/bazel/remote/execution/v2" is the correct way to fix this issue. It also needs to fix import path "github.com/bazelbuild/remote-apis/build/bazel/semver" in https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.pb.go to "github.com/bazelbuild/remote-apis/v2/build/bazel/semver". (fix importpath in go_proto_library?)

If new version doesn't break the existing clients,it should not have been set it as v2.x.x? New major version means it would break existing clients.

[sstriker]

@EdSchouten, @peterebden, any thoughts on this issue?

[EdSchouten]

The remote-APIs repo contains multiple protocols that each have a separate versioning scheme. Could you therefore argue that we shouldn’t specify a version at the top level (that is currently seemingly based on REv2) in the first place?

Maybe we should kick it out entirely.

[peterebden]

I agree, adding it at the top level produces ugly import paths with v2 repeated (or even worse, v2/.../v1 in the case of the Asset API), which seems like a sign that something is a bit wrong. It's a breaking change for consumers too (although I imagine we can all cope if we have to).

The 'correct' solution might be to have a go.mod for each of the protocols since they're versioned differently. Then the /v2 part of the path would appear in the right place. I'm not certain but I think that might also be breaking for consumers though.

[peterebden]

I've had a play with my suggestion above. I still think theoretically it should be able to work but in practice still seemed to get hung up about the repo root in the same way, even though there shouldn't have been a module there any more (wondering if maybe this is a mod cache issue, but cleaning didn't seem to help).

My feeling is that this is probably too finicky to try to solve, and I agree with Ed that the top-level version is a bit irrelevant given versioning of the individual protocols. Feel like we should probably do nothing here since there is a workaround of using untagged versions.