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

Unstable Import Paths #61

Closed
markbates opened this issue Nov 19, 2018 · 62 comments · Fixed by #67
Closed

Unstable Import Paths #61

markbates opened this issue Nov 19, 2018 · 62 comments · Fixed by #67
Assignees

Comments

@markbates
Copy link

With the introduction of the go.mod file and declaration of github.com/gofrs/uuid/v3 there is no longer a single import path that works correctly for both modules and non-modules users.

This has become an issue with gobuffalo/pop and other Buffalo projects.

Below you find a selection of go get statements that represent the different import paths and what happens when trying to use them in different environments.

Go >1.9.7 (or GO111MODULE=off)

$ go get github.com/gofrs/uuid/v3

package github.com/gofrs/uuid/v3: cannot find package "github.com/gofrs/uuid/v3" in any of:
    /usr/local/go/src/github.com/gofrs/uuid/v3 (from $GOROOT)
    /go/src/github.com/gofrs/uuid/v3 (from $GOPATH)

Go 1.11 (GO111MODULE=on)

$ go get github.com/gofrs/uuid
module github.com/gobuffalo/uuidmods

require github.com/gofrs/uuid v3.1.0+incompatible // indirect
$ go get github.com/gofrs/uuid/v3
module github.com/gobuffalo/uuidmods

require (
	github.com/gofrs/uuid/v3 v3.1.2 // indirect
)
@adamdecaf
Copy link
Member

Isn't github.com/gofrs/uuid/v3 v3.1.2 // indirect the desired outcome? I'm not aware of any cross Go version support for Go module import paths.

@markbates
Copy link
Author

markbates commented Nov 19, 2018 via email

@adamdecaf
Copy link
Member

So, yes, when using modules and /v3 everything works correctly. But if you use that, then it breaks those not use modules.

Yea and I'm not sure of support for compiling Go <1.11 with module import paths. This sounds like a problem all Go projects that adopt modules and cross compile have.

@markbates
Copy link
Author

There are two ways you can fix it.

https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher

@itsjamie
Copy link
Contributor

If a branch is created and called v3, if that is set as the default branch does that make it so users of go get will fetch v3 as the root?

@markbates
Copy link
Author

I believe the branch approach only works for >1.9.7, but don't quote me on that.

@bcmills
Copy link

bcmills commented Nov 19, 2018

See golang/go#27430 and golang/go#25967 (comment).

For maximum compatibility, leave the original v3 package at its existing import path (with the +incompatible version suffix and no go.mod file), and start a new major version (/v4) in a subdirectory. That will be unambiguous for all importers, including those on pre-1.9.7 toolchains.

@bcmills
Copy link

bcmills commented Nov 19, 2018

@bcmills
Copy link

bcmills commented Nov 19, 2018

Finally, I have a tool in the works that may help for cases like this one: the goforward command (under review in https://golang.org/cl/137076) makes it much easier to implement v3 in terms of v4 or vice-versa.

@thepudds
Copy link

thepudds commented Nov 20, 2018

Hi all,

I wanted to circle back to the original problem report for this issue:

With the introduction of the go.mod file and declaration of github.com/gofrs/uuid/v3 there is no longer a single import path that works correctly for both modules and non-modules users.

...

Below you find a selection of go get statements that represent the different import paths and what happens when trying to use them in different environments.

Go >1.9.7 (or GO111MODULE=off)

$ go get github.com/gofrs/uuid/v3

package github.com/gofrs/uuid/v3: cannot find package "github.com/gofrs/uuid/v3" in any of:
/usr/local/go/src/github.com/gofrs/uuid/v3 (from $GOROOT)
/go/src/github.com/gofrs/uuid/v3 (from $GOPATH)

and:

when using modules and /v3 everything works correctly. But if you use that, then it breaks those not use modules.

The bit I wanted to try to clarify is that IF you are using Go 1.9.7 as a client of this uuid package, then things do work and you can avoid that error shown in the original problem report.

Part of the tricky bit is that if you are using Go 1.9.7, then I believe you should not include the /v3 in the 'go get' command. Using the /v3 in the 'go get' command is what I think is triggering the error in the original report.

It might be simplest to show by example. Here we'll download an older Go 1.9.7 toolset, then show getting, importing, and using github.com/gofrs/uuid in a trivial client:

# 1. Verify we do *not* have GO111MODULE set:

$ echo $GO111MODULE

# 2. Install Go 1.9.7 if we don't have it:

$ go get golang.org/dl/go1.9.7
$ go1.9.7 download

# 3. Using Go 1.9.7, 'go get' the package of interest.
#    KEY POINT: do not use '/v3' for 'go get' here:

$ go1.9.7 get github.com/gofrs/uuid

# 4. Still using Go 1.9.7, we create, build, and run a trivial client within GOPATH:

$ mkdir -p $GOPATH/src/example.com/sratchpad/uuidhello
$ cd $GOPATH/src/example.com/sratchpad/uuidhello
$ cat <<EOF > hello.go
package main

import (
	"fmt"
	"github.com/gofrs/uuid"
)

func main() { fmt.Println(uuid.Must(uuid.NewV4())) }

EOF

# 5. Build using Go 1.9.7, then run successfully:

$ go1.9.7 build .
$ ./uuidhello

e3ef4eea-6df5-420e-a6b7-39e3b2be0927

That behavior demonstrated there is part of the "minimal module awareness" that was added to Go versions 1.9.7+, 1.10.3+ and non-module-mode 1.11 so that code built with those releases can properly consume v2+ modules without requiring modification of pre-existing code. That behavior is covered in a little more detail here:

And as @markbates already demonstrated above in #61 (comment), code that has opted in to modules using Go 1.11 ends up working as well.

The net of all that is that IF you are willing to require clients to use versions Go 1.9.7+, 1.10.3+, and 1.11, then I think things work.

However, if you want to support versions older than Go 1.9.7, then the /v3 subdirectory approach mentioned by @markbates and @bcmills (#61 (comment)) is an avenue to support even older versions of Go.

All that said, I am not sure what is the best alternative to support the particular goals of this project. I am just a member of the community trying to share my personal understanding of how things currently work, but I am certainly happy to learn more, and apologies in advance if anything I've said here is off-base.

@acln0 acln0 mentioned this issue Nov 20, 2018
@markbates
Copy link
Author

@thepudds you're missing the point. there is NO SINGLE import path that works for EVERYONE.

If you use gofrs/uuid it work for fine for everyone NOT USUING MODULES.

If use gofrs/uuid and your are using modules you STUCK at 3.1.0! Do not pass GO, do not collect $200.

If use gofrs/uuid/v3 with modules, life is grand. If you aren't use modules that things go BOOM.

As someone out there writing code in the real world, I need to support everyone.

I feel as though I have clearly demonstrated this is broken on this ticket.

@markbates
Copy link
Author

I apologize if my tone is strong on that, but I continue to fight this same issue across many repos because it is not clearly understood and can easily be broken if not done correctly.

This is only a problem if you care about those either using mods/ or not using mods. If you don't care, then this isn't an issue.

@thepudds
Copy link

thepudds commented Nov 20, 2018

Hi @markbates no worries, we are all on a voyage of discovery here. ;-)

The main distinction I was trying to draw is that IF support is restricted to Go 1.9.7+, 1.10.3+ and 1.11, then I think "things work" (and without requiring use of a /v3 subdirectory).

Supporting older Go versions like Go 1.8 is a bit trickier, though.

@acln0
Copy link
Member

acln0 commented Nov 20, 2018

Thanks for the detailed report. I think we should do everything we can to support all users, and as many Go versions as we can. It is still not clear to me exactly what we should do, though.

@markbates says we should use a v3 subdirectory.

On the other hand, @bcmills says we should use a v4 subdirectory and leave the original v3 package at its current import path with no go.mod. But we do have a go.mod for v3, at v3.1.1. Is that one not good? Should we remove it?

In any case, it seems like moving one version of the package into a vX subdirectory is the way forward. In this case, where do we put the go.mod? I noticed in @markbates packr repository that both the root directory and the v2 subdirectory have their own go.mod.

@markbates
Copy link
Author

I'm with fine with either /v3 or /v4 sub directory. I would tend to agree with @bcmills as it's best to just put the whole thing in the past to move it forward.

Packr is 1.x in the root and 2.x in /v2. From what I believe, and the docs aren't clear on it, is whether you need one in each directory or not. I err'd on the side of "go modules do weird things right now and better safe than sorry".

@acln0
Copy link
Member

acln0 commented Nov 20, 2018

Hmm, looking at it more, it seems like we should have bumped major versions when adding modules support, but we didn't. Is that the cause of go get github.com/gofrs/uuid resolving to require github.com/gofrs/uuid v3.1.0+incompatible when using modules and not explicitly specifying /v3?

Given that we, unfortunately, didn't bump major versions, what is the best course of action now? Should we rectify the mistake by moving to v4?

@markbates
Copy link
Author

markbates commented Nov 20, 2018

Yeah, the "logic" (I mean modules logic) is to use the last version that didn't have a go.mod if you don't use /v3 but are using modules. That's the underlying issue with this this bug.

My vote, is /v4, and y'all should write a blog post to demonstrate this issue. :)

@acln0
Copy link
Member

acln0 commented Nov 20, 2018

Okay. Sorry if I'm being obtuse, but I want to make sure we get everything right, so I will recap. My understanding is that for maximum compatibility, we should:

  • leave the current contents of the root directory untouched
  • create a copy of the code in a /v4 subdirectory
  • create a go.mod with module github.com/gofrs/uuid/v4 in the /v4 subdirectory
  • update the documentation to recommend github.com/gofrs/uuid/v4 as the one true import path that works for everyone
  • push all of this up, then cut v4.0.0

Does that sound right?

@acln0 acln0 self-assigned this Nov 20, 2018
@markbates
Copy link
Author

markbates commented Nov 20, 2018 via email

acln0 added a commit that referenced this issue Nov 20, 2018
Create a new /v4 directory, containing a copy of the existing code,
allowing the package to move forward and be consumed by all users
under a unified import path: github.com/gofrs/uuid/v4.

Remove go.mod file from the top level directory and add it to the v4
directory. Update v3 to v4 in go.mod.

Update documentation to reflect the change: Recommend v4.0.0+ as the
go-to version for new users and existing modules users. Recommend
v2.1.0 under github.com/gofrs/uuid as the go-to version for users
looking for a replacement for satori/go.uuid at v1.2.0. Document the
two different import paths and their behavior.

Fixes #61.
@bcmills
Copy link

bcmills commented Nov 20, 2018

I agree with @markbates: if existing call sites are written against v3.x.x with the non-/v3 import path, to get them to build in module mode you'll want a v3.x.x+incompatible version, which implies removing the go.mod file and tagging the result (probably as v3.1.3).

(The presence of the go.mod file signals that you're using semantic import paths.)

@bcmills
Copy link

bcmills commented Nov 20, 2018

@acln0:

create a copy of the code in a /v4 subdirectory

Note that /v4 doesn't have to be a copy. It can forward functions and types back to the non-/v4 path, or you can move (not copy) the code to /v4 and rewrite the non-/v4 path to forward to /v4. Either would reduce the maintenance overhead, since bug-fixes in one path would automatically apply to the other as well. (That's what the goforward tool is for.)

@thepudds
Copy link

thepudds commented Nov 20, 2018

Two related but perhaps separable questions might be:

Q1. What does the gofrs organization view as the best way forward for the gofrs/uuid package regarding modules?

Q2. Is the gofrs/uuid repo broken as it stands today?

There is a discussion going on elsewhere within the gofrs team regarding Q1, I think.

One semi-related question: does anyone know the minimum version of Go required for gobuffalo/pop currently? The gobuffalo documentation seems to suggest Go 1.9.7+ is required:

https://gobuffalo.io/en/docs/installation#requirements

Before installing make sure you have the required dependencies installed:
...
Go version 1.9.7 or greater.

But I am not a gobuffalo user, so not sure if that is the right place to look.

@thepudds
Copy link

thepudds commented Nov 20, 2018

Regarding:

Q2. Is the gofrs/uuid repo broken as it stands today?

Having looked at it a bit more, as far as I can tell:

  • It seems the gofrs/uuid repo in effect currently requires Go 1.9.7+, 1.10.3+ or 1.11.
  • The gofrs/uuid repo in its current state seems to work with those versions of Go.
  • However, it might not have been intentional that gofrs/uuid in effect requires Go 1.9.7+, 1.10.3+ or 1.11.

Of course, happy to learn more here, and this is not based off of any type of exhausting testing or assessment.

@stanislas-m
Copy link

One semi-related question: does anyone know the minimum version of Go required for gobuffalo/pop currently?

Go 1.9.7+, the same requirements as gobuffalo/buffalo.

@markbates
Copy link
Author

markbates commented Nov 20, 2018 via email

@bcmills
Copy link

bcmills commented Nov 20, 2018

@markbates, we're all trying to figure out a useful path forward. Please be patient and charitable.

You say:

This has become an issue with gobuffalo/pop and other Buffalo projects.

But you haven't said what the concrete issue is. If gofrs/uuid already requires Go 1.9.7+, then all existing code that does not have a go.mod file can use the non-/v3 import path, and all existing code that does have a go.mod file can use the /v3 import path (in both module mode and GOPATH mode).

Is the problem:

  • that users of gobuffalo/pop are using Go versions that predate 1.9.7?
  • that users are confused as to how to enable the /v3 import path?
  • something else?

@acln0
Copy link
Member

acln0 commented Dec 3, 2018

What about the forwarding declarations? Did you catch that part of the proposed solution? Have you looked at c6f696b? Is this not acceptable either? Are we breaking current /v3 callers altogether?

@theckman
Copy link
Member

theckman commented Dec 3, 2018

@markbates Please don't apologize for being frustrated; it's on the Go authors for pushing modules on to us. I'm getting more and more frustrated trying to defend the choices to outsides / newcomers to Go, especially when I don't believe in it myself.

@theckman
Copy link
Member

theckman commented Dec 3, 2018

@acln0 I think the streams may have been crossed with my edit:

Edit: To be clear, I firmly believe in Rob Pike's "A little copying is better than a little dependency", but creating a separate directory for different code versions isn't acceptable to me.

@acln0
Copy link
Member

acln0 commented Dec 3, 2018

You are proposing to intentionally break the builds of callers who are using the /v3 import path. Under any other circumstances, I don't think you would have agreed to a change that breaks someone's build intentionally. But in this particular case, you are letting your personal opinions about modules guide your reasoning on this matter, and you are going in a direction that I do not want to be a part of.

We made a mistake introducing the go.mod file as we did, and now we need to own up to that mistake, not break people's builds because of our personal opinions.

@theckman
Copy link
Member

theckman commented Dec 3, 2018

@acln0 Everyone here has commit access and can pursue anything they want. I am not a decider, gate-keeper, or benevolent dictator. I am offering my opinion of what I don't wish to support or pursue, if others in the Gofrs wish to purse that I am not a blocking voice.

And to that point, since it seems my dislike of modules has become so contentious to the Gofrs organization and my intent when starting the group, I hereby relieve myself of technical involvement in deciding in this issue and others.

@theckman
Copy link
Member

theckman commented Dec 3, 2018

In other words, move forward how y'all feel appropriate on this issue. Let me know what I can do to support that decision.

@theckman
Copy link
Member

theckman commented Dec 3, 2018

It's worth doing some homework on this to understand what exactly we're going to break and how. We should test with multiple toolchain versions, dep, glide, and with modules support.

We're also going to provide a detailed experience report to the Go authors if any interesting findings come out of it.

@ghost
Copy link

ghost commented Jan 11, 2019

Hi @markbates,

After quite a bit of deliberation over the span of a month, we decided that the best thing to do to deal with this issue was to remove module support until it becomes more mature.

We realize that this is likely less than ideal for you. But this decision comes based on several factors:

  1. At this time, SIV does not support having one import that always resolves to latest for both module and non-module users without using type forwarding or subdirectories.
  2. Our view is the usage of version number subpackages and type forwarding is untenable for us, and we worry it will be a long term maintenance issue for the library while modules is in its experimental stage.
  3. Currently the behavior where the importer will inconsistently resolve to 3.1.0 or 3.1.2 depending on configuration must be addressed, and we need to return this library to a "stable" state as early as possible while impacting the least amount of people.

We feel that the best solution to address the above problems and provide a single canonical path that resolves to the latest version is to drop support for modules in the interim and release 3.2.0. This will allow all consumers on 3.2.0 to once again import without the /vN path and have version consistency. Module-based consumers will not be able to upgrade to 3.2.0 until they remove the /v3 from their gofrs import path, but this is the workflow that is most likely to signal to our user base of the decision to remove module support (for now).

The README update in #67 offers some additional context and what precisely will be conveyed to consumers of this dependency as they upgrade to v3.2.0 or beyond from the module supported versions.

Thanks for reporting this issue and your continued patience with us as we worked through this.

@theckman
Copy link
Member

theckman commented Jan 11, 2019

@DamareYoh Thank you for adding the summary here.

In the interest of full transparency because there's a lot of context in this issue: as modules becomes mature, and other things happen in the ecosystem to better support interoperability, we will adopt modules so that we are able to meet the needs of the community. As is probably clear, TBD on a lot of that.

@markbates
Copy link
Author

markbates commented Jan 11, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants