search

ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.86k stars 295 forks source link

This package cannot be easily constrained with go mod #291

Closed dcarbone closed 5 years ago

dcarbone commented 5 years ago

Take this simple example code:

package main

import (
    "fmt"
    "github.com/gammazero/nexus/router"
)

func main() {
    r := router.RawSocketServer{}
    fmt.Printf("%v", r)
}

Here is the go.mod file produced:

module localhost/superscratch

go 1.12

require (
    github.com/gammazero/nexus v2.1.0+incompatible
    github.com/gorilla/websocket v1.4.0 // indirect
    github.com/ugorji/go v1.1.4 // indirect
    golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a // indirect
)

and the go.sum:

github.com/gammazero/nexus v2.1.0+incompatible h1:1faYOz5Gp0sBJpb5L5/PPJ5j2K0iuNxoMtoDfxf2PK4=
github.com/gammazero/nexus v2.1.0+incompatible/go.mod h1:0qqt3FvYuipZoxpd+k7ZrNagbFd9BYGwlOPRloSZMKw=
github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
github.com/ugorji/go v1.1.4 h1:j4s+tAvLfL3bZyefP2SEWmhBzmuIlH/eqNuPdFPgngw=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a h1:Igim7XhdOpBnWPuYJ70XcNpq8q3BCACtVgNfoJxOV7g=
golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE=
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

With modules, this fails to build due to the breaking API change done here.

The typical way to try to address this would be to do something like this:

go mod edit -replace github.com/ugorji/go@v1.1.4=github.com/ugorji/go@v1.1.2

This adds the following line to the go.mod file:

replace github.com/ugorji/go v1.1.4 => github.com/ugorji/go v1.1.2

and after runninggo vendor or go tidy, the sum file is updated:

github.com/gammazero/nexus v2.1.0+incompatible h1:1faYOz5Gp0sBJpb5L5/PPJ5j2K0iuNxoMtoDfxf2PK4=
github.com/gammazero/nexus v2.1.0+incompatible/go.mod h1:0qqt3FvYuipZoxpd+k7ZrNagbFd9BYGwlOPRloSZMKw=
github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
github.com/ugorji/go v1.1.2 h1:JON3E2/GPW2iDNGoSAusl1KDf5TRQ8k8q7Tp097pZGs=
github.com/ugorji/go v1.1.2/go.mod h1:hnLbHMwcvSihnDhEfx2/BzKp2xb0Y+ErdfYcrs9tkJQ=
github.com/ugorji/go v1.1.4 h1:j4s+tAvLfL3bZyefP2SEWmhBzmuIlH/eqNuPdFPgngw=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a h1:Igim7XhdOpBnWPuYJ70XcNpq8q3BCACtVgNfoJxOV7g=
golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE=
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

You'll notice, however, that v1.1.4 is still in there.

When you attempt to go build, you see this error:

MANOS:superscratch dcarbone$ go build
go: finding github.com/ugorji/go/codec latest
build localhost/superscratch: cannot load github.com/ugorji/go/codec: cannot find module providing package github.com/ugorji/go/codec

This can be "fixed" by manually adding the following replace statements (this list is incomplete, as you've committed more to master since I gave up this):

replace(
    github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43 => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
    github.com/ugorji/go/codec v0.0.0-20190309163734-c4a1c341dc93 => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
    github.com/ugorji/go/codec v0.0.0-20190315113641-a70535d8491c => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
    github.com/ugorji/go/codec v0.0.0-20190316192920-e2bddce071ad => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
)

and manually adding this to the require block:

require (
    github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8 // indirect
)

Once done, you can now build the code.

This is not a penultimate solution, however, because the next time you run go get for any other dep, the line added to the mod file is removed.

This would not be a big issue if it were not for the BC issue introduced by https://github.com/ugorji/go-codec/commit/a70535d8491cc93f3813e0946f4399501f3cb47f, and I believe this could be solved given the implementation of #43.

ugorji commented 5 years ago

Is it sufficient if I put a holder file in github.com/ugorji/go? e.g. github.com/ugorji/go/doc.go?

There are many reasons why I have hesitated on #43 while keeping it open. The reasons are listed there and extremely compelling. I would rather fix this issue that bring out a big hammer.

dcarbone commented 5 years ago

Very possibly, yes. I will not profess to be an expert on modules, but I do wonder if even adding a doc.go file to the root would address this issue.

ugorji commented 5 years ago

go modules have been a source of pain for me too. A big part of me is depending on folks who work with this a lot to help. I definitely appreciate your kicking the tires on it here.

Is it possible to try out this change 5e5ed366c1769eb1629103d254b716d4cedf17d5 somehow, and see if it helps?

Thanks.

ugorji commented 5 years ago

The doc.go wouldn't work.

go tool doesn't support package names which are reserved words.

Back to the drawing board. If you have any ideas, please share.

ugorji commented 5 years ago

I spent the last day reading about go modules, and how it works. I also read Russ' initial documents about vgo.

I have been trying to see if there is anything I can do to help this.

What seems clear to me, is that we are supporting go modules as designed. The go tool still has issues to resolve in its module support, but there isn't much we can do here to solve it.

  1. 43 is about changing the package path, which basically creates a new package. This doesn't help the package user, who might as easily just change a few lines in their code for MsgpackHandle.

  2. The go tool by design should support packages which do not live at the root of the repo. This is supported by the documentation below:

https://golang.org/cmd/go/#hdr-Defining_a_module

The directory containing the go.mod file is called the module root. Typically the module root will also correspond to a source code repository root (but in general it need not).

The "module path" is the import path prefix corresponding to the module root.

https://research.swtch.com/vgo-module

We want to allow multiple modules to be developed in a single source code repository but versioned independently. While most developers will likely keep working with one module per repo, larger projects might benefit from having multiple modules in a single repo. For example, we'd like to keep golang.org/x/text a single repository but be able to version experimental new packages separately from established packages.

You actually see this in https://github.com/gammazero/nexus , where there is no .go file in the repo root, but all packages are sub-directories in here. This is a pattern that has been common for go packages for a very long time - where you would see packages like github.com/user/go-pkg/pkg.

Because of these, there is nothing I can really do to alleviate this issue. The main issue is that we introduced a breaking change. But even #43 does not resolve that, as #43 basically is about changing the import/package path, which means that no current user knows about this "new" package. It effectively splinters the ecosystem, of which we already have 2311 packages depending on this (as seen in https://godoc.org/github.com/ugorji/go/codec?importers and at bottom of https://godoc.org/github.com/ugorji/go/codec ).

When I slowly parse through your messages above, what I can see is that you are dependent on a package which is broken by our change, and go module support doesn't allow you to constrain its dependency to a specific commit of go. This is a go tool issue. I really think you should open this issue with the go tool, and let them dig into this. You have a very clear reproducer and detailed bug report for them.

My own issue alone is defined by #290 (Breaking change). It's extremely unfortunate oversight and my bad for that one. We will leave that open to see who else is affected before making a change.

I will close this one now. Please let me know if you think different.

@dcarbone ^^