Broken govendor/dep.. vendor support

[mieczkowski]

@mediocregopher I appreciate that you target to new modules feature in golang, but imports with /v3/ in path broke support for old GOPATH way (try using govendor/dep - it is not working anymore). Please consider some backward compatibility, because some people are not ready for changing dependencies managment system (and don't want to change library, because radix fits perfectly our needs :-) )

[yerden]

+1. dep is apparently broken.

init failed: unable to solve the dependency graph: Solving failure: No versions of github.com/mediocregopher/radix met constraints:
        3.1.0: Could not introduce github.com/mediocregopher/[email protected] due to multiple problematic subpackages:
        Subpackage github.com/mediocregopher/radix/v3/resp is missing. (Package is required by github.com/mediocregopher/radix.v3@master.)      Subpackage github.com/mediocregopher/radix/v3/resp/resp2 is missing. (Package is required by github.com/mediocregopher/radix.v3@master.)

[mediocregopher]

#50 already reported this. Unfortunately there's not much I can do at this point, it's too late to change the import paths of the package. It looks like dep has a PR open to fix this here: golang/dep#1963

but the author maybe just had a baby, judging by the comments. I'll leave this issue open, waiting on that one to get closed.

[mieczkowski]

@mediocregopher how about putting resp package into te main package? Problematic imports are gone and we have backwards compability ;-)

[yerden]

@mieczkowski I think, just as @mediocregopher stated, that would break builds for a lot of dependency packages. Hence, that would also break backwards compability.

[mediocregopher]

@mieczkowski the only way that would work is if I copied resp and its sub-package into the main one, I wouldn't be able to move them, which would be a giant pain for me as I'd now have a huge maintenance overhead for however long it takes for dep to get fixed.

Perhaps you could fork radix for now and do whatever needs doing, until that PR goes through? Or you could help contribute to that PR so it can get done faster. From the looks of it all it needs is tests and docs.

[awkejiang]

any updates on this issue ?

[mediocregopher]

As i mentioned here there's not really anything to be done. Once again, someone could try contributing to dep to get that PR through, that would be the most effective fix.

[cep21]

You can support both if you make a /v3 directory and move all the code to that directory. It will support both modules and dep. Will you accept a pull request for this?

[cep21]

I made an example of what it would look like to support dep and modules at the same time as #128 It's just a reference PR to see what it looks like.

Edit: This should also support glide.

[mediocregopher]

Hi @cep21, thanks so much for bringing this up! I'm going to continue the convo here rather than the PR.

I do remember reading that section of the docs a long time ago, I guess it just didn't stick in my brain >_< You brought up the aesthetics of having a top-level v3 directory, but honestly I don't think that's a big deal. I do have a couple of questions about other aspects of the idea first that hopefully you, or someone else, could answer (or if everyone's willing to wait a while I can find out myself, but my backlog is very long).

1. When you say:

Even if you don't merge this to master, making it a branch of github.com/mediocregopher/radix would allow people to reference that branch for at least some version of this repository.

How would that work? If someone is using an older version of go that doesn't support modules, how would that go get know to look in that branch?

2. If someone is currently using the import path github.com/mediocregopher/radix, without using modules, wouldn't this change break their builds? I've already broken people's import paths once for modules, I'm not super keen on doing it again.

3. Is there some fundamental reason why these other package managers can't properly support repos which have been designed to work with modules? Modules are the official "way forward", it seems like properly supporting them would be a top-priority issue for a package manager. [WIP] minimal module awareness golang/dep#1963 has been open for nearly a year now, that blows my mind. I know this isn't your fault, but it just seems like having individual projects bend over backwards to support various dependency managers, all because they refuse, or are too lazy, to support the "correct" way, is a crazy way to do things.

4. You made this statement in the PR:

it seems like the right approach while the Go community is in this transition period between dep and modules.

I would argue that the go community is transitioning from go get to modules. I'm pretty sure dep was always labeled as an experiment, so I'd be very surprised if people using dep outnumber people using plain old go get, but I'd love to see some actual numbers one way or the other. That said, if go get is the majority I would be against breaking their builds in order to fix dep, especially, as I mentioned before, when I've already broken their builds once before.

Sorry to be so nitpicky about this, and I do appreciate the effort you put in to put this plan together.

[cep21]

I would argue that the go community is transitioning from go get to modules.

I don't know any company or major go project that was using go get to manage third party dependencies. This was due to wanting to lock down specific versions of a library and ensure their binaries always built. This usually required a lock file or vendor folder, and some software (dep, glide, etc) was used for this. I do agree it would be bad to do any change that breaks go get and this should not.

That said, if go get is the majority I would be against breaking their builds

Your master branch is broken for old versions of go get, because old versions of go don't understand modules. This change would increase compatibility with go get, rather than decrease it. Versions of go updated to understand modules will continue to work with a /v3/ directory.

How would that work?

Ideally this change would be part of master, however in the branch situation their Gopkg.toml file would look like

[[constraint]]
  name = "github.com/mediocregopher/radix"
  branch = "working_with_dep"

Most other dependency tools would have similar support for a branch statement, the main constraint with most of them is that it has to live at the repository location.

If someone is using an older version of go that doesn't support modules, how would that go get know to look in that branch?

It would not. Just like your current master branch, they would continue to be out of luck.

Is there some fundamental reason why these other package managers can't properly support repos which have been designed to work with modules?

There are a few of them and I haven't looked at them all, but I would guess that modules are a strong departure from major version management (putting the /v3/ in the import path vs the branch name) and this breaks a bunch of assumptions their code has which would have to be unraveled.

If someone is currently using the import path github.com/mediocregopher/radix, without using modules, wouldn't this change break their builds?

Your master branch currently does not work for versions of Go that do not support modules. If this was merged, your master branch should continue to support versions of Go that do understand modules, as well as versions of Go that do not. I 100% understand not wanting to break people's builds.

A path to test this would be to push my PR as git tag v3.3.1 without pushing to master. You could then try switching between v3.3.1 and v3.3.0 and back to v3.3.1 to see if it works without an issue.

[mediocregopher]

I don't know any company or major go project that was using go get to manage third party dependencies.

I've worked for one company which did so. Since most go projects are pretty serious about backwards compatibility and not breaking builds, we never found it super necessary to do otherwise.

Your master branch is broken for old versions of go get, because old versions of go don't understand modules. This change would increase compatibility with go get, rather than decrease it.

radix supports the most recent two go versions. In fact the last 4 will work on the master branch, using the non-v3 import paths. So I don't see how you can say the master branch is broken. My issue is not that I'm trying to support old go version, I'm trying to keep from breaking people's builds. My first question regarding how older versions of go would work with the branch idea was unnecessary, so I apologize for that.

So right now the only two options I see are:

1. Push this PR to master, and break builds. I'm not going to do that.

2. Support a secondary branch.

I'm not opposed to the second option in theory, but in practice I don't want to spend the time supporting it. If you'd like, you could direct your PR against a branch called third-party-dep-mgrs, and as I release changes in master in the future I'll accept more PRs against third-party-dep-mgrs to keep them in sync. I don't think tagging will work in that situation, but hopefully most managers can pin to a git commit.

It's not a perfect solution, but would that be acceptable?

[cep21]

Thank you for the clarifications.

Why do you believe merging this PR will break people's existing builds?

[mediocregopher]

If someone was using go with modules turned off, just go get, and they were importing the package as import "github.com/mediocregopher/radix", moving all files into a v3 subdirectory would break that import, since the import path would have to now be github.com/mediocregopher/radix/v3.

In the README I explicitly instruct people who are using "legacy GOPATH mode" (aka go get) to use that import path. Using the .../v3 import path doesn't work if someone is using GOPATH mode as well.

So that PR would definitely break builds.

[cep21]

I see. If github.com/mediocregopher/radix ever needs to release a v4, how will you handle the go get case? Will you fork the repository into a new github repository?

[mediocregopher]

I believe I can make a v4 branch, right? Or maybe the v4 directory would work then. I'll cross that bridge if/when I get to it, at any rate.

[cep21]

I think I understand now. We should be able to support that use case with go aliases. I've pushed an example https://github.com/mediocregopher/radix/pull/128/files#diff-a944a5a7ca6a5df16d2a22a7117689a7 depending upon how it looks, I could also push aliases for resp/trace/etc directories.

[mediocregopher]

Now that's an interesting idea, I don't hate it. Is there a tool you used to generate that file? Or was it manual? If it was a tool it'd be nice to add it as a go generate line in that file. If it's not a tool... I might want to make a tool lol, that'll be easy to mess up otherwise.

Aliases for resp and resp/resp2 will be necessary, I don't think trace is necessary since that package is marked as unstable in its godoc anyway.

[mediocregopher]

Or we could do it the other way around, with the aliases living in the /v3 directory. If that works it might be better since we wouldn't need to have resp aliases (I don't think, haven't tested it, maybe go will get confused because of the v3 directory now being there).

EDIT: Or does dep/glide need resp to be in v3? I wouldn't think so since it's still using the go tool under the hood right?

[cep21]

The tool is https://go-review.googlesource.com/c/tools/+/137076/9 and I updated the PR to use the tool.

goforward github.com/mediocregopher/radix/v3 github.com/mediocregopher/radix
https://github.com/mediocregopher/radix/pull/128/files#diff-a685864ec85a24f304040785f3221330R1

Or we could do it the other way around, with the aliases living in the /v3 directory.
Both will satisfy backwards compatibility, but like you said you would have to keep updating the aliases.

It's possible you want to encourage legacy GOPATH users to eventually switch to github.com/mediocregopher/radix/v3 so that they are (1) better prepared for go modules when they do adopt them (2) can take your v4 changes when you want to release them.

If the /v3 code lived in github.com/mediocregopher/radix/v3 and you put aliases in github.com/mediocregopher/radix then existing legacy users that import with github.com/mediocregopher/radix and use go get for dependency management would continue to work and would continue to get bug fixes and improvements, but would need to add /v3 to import paths to use new v3 types or functions. Go modules and Dep/glide/etc users would be able to use radix as well, getting new features and fixes as they happen.

[mediocregopher]

I guess for now we can do it this way, if I'm feeling spry I can try and reverse it later on.

I'm having trouble getting that tool to work. I made a new repo to play around with: https://github.com/mediocregopher/radixxx

And when running it from the root, with the repo in my GOPATH, I get this:

 ~/src/go/src/github.com/mediocregopher/radixxx :: master :: echo $GOPATH
/home/mediocregopher/src/go
 ~/src/go/src/github.com/mediocregopher/radixxx :: master :: ./goforward github.com/mediocregopher/radixxx/v3 github.com/mediocregopher/radixxx
goforward: main.go:127: package not found: github.com/mediocregopher/radixxx/v3

So I'm not totally sure what's going on there. How did you get it to work?

[cep21]

Hmmm here is what I ran (on my branch checked out to your repo's location locally)

16:48:30 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jlindamo/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jlindamo/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hz/fdkmg98n3wx1xth0ypjq1s4c3xsp_j/T/go-build080332156=/tmp/go-build -gno-record-gcc-switches -fno-common"
16:48:32 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< go version
go version go1.12 darwin/amd64
16:48:41 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< which goforward
/Users/jlindamo/go/bin/goforward
16:48:44 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< rm forward.go 
16:48:50 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< goforward github.com/mediocregopher/radix/v3 github.com/mediocregopher/radix
16:48:52 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
< wc -l forward.go 
     605 forward.go
16:53:23 (master) [email protected]:~/go/src/github.com/mediocregopher/radix
> echo $GO111MODULE

[mediocregopher]

It seems that dep has been deprecated, and v4 will be coming out soon (before or just after the new year, hopefully), so I'm going to close this issue. Sorry I could never get goforward working properly.