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.

Sign up for GitHub

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

Jump to bottom

Support go modules #39

Open
Scapal opened this issue Oct 24, 2018 · 22 comments
Open

Support go modules #39

Scapal opened this issue Oct 24, 2018 · 22 comments
Milestone
v3

Comments

@Scapal
Copy link

Scapal commented Oct 24, 2018

Support go module versioning conventions and when used don't redirect to gopkg.in/mail.v2

https://github.com/golang/go/wiki/Modules#semantic-import-versioning
@ivy
Copy link

ivy commented Nov 10, 2018

@Scapal Thanks for opening this issue. We had some discussion in #35 about this, but I think it's worth revisiting. I'll look into this more soon. 😄

ivy added a commit that referenced this issue Nov 10, 2018
@ivy
Add go.mod manifest
6698faf 
This file specifies the require path for go-mail and the v2 branch.

Fixes #39
@ivy ivy closed this as completed in fc6fc6b Nov 10, 2018
@ivy
Copy link

ivy commented Nov 10, 2018

@Scapal I just tagged v2.3.0 with support for Go modules. Let me know if this fixes your issue.

@markbates
Copy link

This has broken both modules and go get. :(

According to the go.mod if using modules you must import with github.com/go-mail/mail/v2 however, if you doing that it breaks go get as there is no path on GitHub to match that.

It's important that the module name also match the import path for those not currently using modules, which is a lot of people.

The suggested route to solve this problem is to create a /v2 folder in the repo, that contains the v2 module and code. This allows for people to migrate to /v2 at their leisure, while not breaking those trying to support both go modules and go get.

Please consider either rolling back this change or making the suggested corrections. Thanks.

This release has broken https://gobuffalo/buffalo.

markbates added a commit to gobuffalo/buffalo that referenced this issue Nov 12, 2018
@markbates
move gomail into interal because of go-mail/mail#39
95c2e1a
@ivy
Copy link

ivy commented Nov 12, 2018

@markbates Apologies for the trouble. What does your go version report? From what I read, specifying /v2 should work for branches too:

Go versions 1.9.7+, 1.10.3+, and 1.11 are able to properly consume and build a v2+ module created using this approach without requiring updates to consumer code that has not yet opted in to modules (as described in the the "Semantic Import Versioning" section above).

Also, from looking at your commit in gobuffalo/buffalo@95c2e1a, I notice you were using @alexcesaro's (upstream) import path, gopkg.in/gomail.v2, and not gopkg.in/mail.v2. Was that intentional?

To use this fork, you'll want to import either:

  • github.com/go-mail/mail - This should be the latest HEAD of the v2 branch.
  • github.com/go-mail/mail/v2 - If using Go modules, this should be the latest v2.*.* tag.
  • gopkg.in/mail.v2 - This should be the latest HEAD of the v2 branch.

I'll double check to be sure I'm not making an obvious mistake here. If anyone else can confirm this breakage, we can roll this back. 😅

@markbates
Copy link

markbates commented Nov 12, 2018 via email

@markbates
Copy link

markbates commented Nov 12, 2018 via email

@ivy
Copy link

ivy commented Nov 12, 2018 •
edited

@markbates I understand your frustration and I want to help. A number of users have expressed interest in utilizing Go modules and I'm only supporting it under the condition that it works for everyone. I want to find a solution that works for us all and in order to do that, I need a bit more detail…

  • What go version are you using?
  • What exact error messages are you getting?

I just tried importing go-mail with a pristine GOPATH without any trouble. One hiccup I noticed, however was that I needed my project to reside within GOPATH, otherwise I get:

go: finding gopkg.in/mail.v2 v2.3.0
go: downloading gopkg.in/mail.v2 v2.3.0
build mytest: cannot find module for path gopkg.in/mail.v2

After making sure my project was moved/symlinked into GOPATH, I had no trouble outside of a small warning:

go get: warning: modules disabled by GO111MODULE=auto in GOPATH/src;
	ignoring ../../../go.mod;
	see 'go help modules'
Fetching https://gopkg.in/mail.v2?go-get=1
Parsing meta tags from https://gopkg.in/mail.v2?go-get=1 (status code 200)
get "gopkg.in/mail.v2": found meta tag get.metaImport{Prefix:"gopkg.in/mail.v2", VCS:"git", RepoRoot:"https://gopkg.in/mail.v2"} at https://gopkg.in/mail.v2?go-get=1
gopkg.in/mail.v2 (download)
gopkg.in/mail.v2
test

Is this the same case for you too?

Edit: I pasted the wrong go get output.

@ivy
Copy link

ivy commented Nov 12, 2018

@markbates It might also help to see the output of pwd and go env.

@ivy ivy mentioned this issue Nov 12, 2018
Closed
@markbates
Copy link

With mods on (off works fine):

go get gopkg.in/mail.v2

go: gopkg.in/mail.v2@v2.3.0: go.mod has non-....v2 module path "github.com/go-mail/mail/v2" at revision v2.3.0
go: error loading module requirements

With mods off (mods are fine):

go get github.com/go-mail/mail/v2

package github.com/go-mail/mail/v2: cannot find package "github.com/go-mail/mail/v2" in any of:
	/usr/local/go/src/github.com/go-mail/mail/v2 (from $GOROOT)
	/Users/markbates/Dropbox/go/src/github.com/go-mail/mail/v2 (from $GOPATH)

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/markbates/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/markbates/Dropbox/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/markbates/Dropbox/go/src/github.com/gobuffalo/buffalo/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2 -w"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/zj/ktv0trrj4l79dfq0dkm1b6d40000gn/T/go-build141527679=/tmp/go-build -gno-record-gcc-switches -fno-common"

It's easy to reproduce. You just need to pick an import path, and support that one.

@markbates
Copy link

markbates commented Nov 12, 2018 via email

@markbates
Copy link

I’ve been working closely with members of the Go team on improving modules support, and I’ve brought this ticket to their attention as an experience report. Hopefully it’ll help them with docs and improvement s

@dhui
Copy link

dhui commented Nov 12, 2018

FYI, for a package maintainer, opting into modules for a non-v1 package is a breaking change.
e.g. the version of Go the downstream package(s) uses needs to support modules and import paths need to be updated since it's a non-v1 package.

You also can’t rever the change now either as go modules will pick up the last release with a go.mod file; so you can’t revert and need to fix the problem head on.

You actually can revert the change in v2 and support modules in v3. go get and any module aware dependency manager will do the right thing. The problem is that not all dependency managers are module aware. For golang-migrate, I had to back-out module support in v3 and support modules in v4.

@markbates I believe you need to use github.com/go-mail/mail/v2 imports with modules enabled. You can also look into using the replace directive as a big hammer, but that probably won't help with gobuffalo since it only works for the top-level package. e.g. consumers of gobuffalo would also need to use replace.

@Scapal
Copy link
Author

Scapal commented Nov 12, 2018

Using go modules, outside GOPATH, it works as expected.
imported as "github.com/go-mail/mail/v2"

It creates the following entry in my go.mod:

github.com/go-mail/mail/v2 v2.3.0

No error nor warning during testing or compilation.

@markbates
Copy link

markbates commented Nov 12, 2018 •
edited

again, this is broken if you want to use the same import path for both mods or non-mods.

We’ve internalized the package because this is broken. It’s easy to reproduce in the GOPATH.

Pick an import path and support just the one. It’s that simple.

I get the impression, from this ticket, that there is no interest in changing that. Again, not my project, so I can’t dictate.

I’m moving on from this issue; since we’ve fixed it by vendoring for now.

If this gets fixed, we’ll remove the vendor.

Please consider fixing this. Considering how easy it is to support both; the fact I’m getting such push back on this, makes me believe that this won’t happen. :(

@stanislas-m
Copy link

stanislas-m commented Nov 12, 2018 •
edited

I can confirm what @markbates said: buffalo, as a lib importing go-mail/mail, need to support both users using go modules and those who don't, that's why the current solution can't work.

@dreampuf
Copy link

We are affected by this problem. Please consider to fix it.

@ivy
Copy link

ivy commented Nov 12, 2018

Okay, the fix I'm considering is:

  1. I can remove the go.mod file and tag a new v2.3.1 release. This should fix the issue for almost everyone, correct?

  2. I can also delete the v2.3.0 tag. At least, that seems like the solution to @markbates's earlier response:

    You also can’t rever the change now either as go modules will pick up the last release with a go.mod file; so you can’t revert and need to fix the problem head on.

Can anyone sanity check this solution to be sure I don't inadvertantly make things worse?

@ivy
Copy link

ivy commented Nov 12, 2018

v2.3.1 is tagged and removes the go.mod manifest. I'm going to hold off on deleting the v2.3.0 tag until someone more knowledgable can chime in (I imagine a go mod cache purge will be needed). Sorry for the inconvenience!

@ivy ivy reopened this Nov 12, 2018
@ivy ivy added this to the v3 milestone Nov 12, 2018
@pedromorgan
Copy link

Snag is @ivy

  • That i lost trust with gopkg.in
  • then embraced this fork in same manner..
  • But its stunk me a few times (other project also) so i lost trust in gopkg.in for that reason

The new go.mod solves the problem...

@ivy
Copy link

ivy commented Nov 14, 2018 •
edited

I'm going to assume by the silence that there's no need to yank the previous tag.

@pedromorgan Agreed 100%, Gopkg.in is no longer necessary. It's really only around for the small minority of developers who might still rely on v1 and/or an older Go. I'm open to deprecating it.

Unfortunately, the go.mod addition created more problems than it solved…

@flibustenet
Copy link

I believe @dhui is right, it's safe to keep v2 as it is and switch to go modules in v3 that you could start as soon as you can, (even if it's just for that ?).
Go modules is more and more in use even buffallo use it and need Go 1.9.7+ , it will be a shame to don't jump in the wagon.

@bvisness
Copy link

bvisness commented Oct 9, 2019

It's been a while; let's revisit this.

The Go wiki has some guidelines on how to migrate a package to modules that is already on a version >= 2.0: https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher. The general advice they give is to bump up to a new major version (so dep, etc. can stick with their current version), and to either put the new major version on a separate branch or in a subdirectory. Given the stability of this library, I think the major branch approach would be fine, although it would probably prevent dep from importing v3 until golang/dep#1962 is fixed.

I can submit a quick PR that adds modules at v3, but the important parts of the process will be outside the scope of the PR:

  • (Optional) Create a v3 branch and make it the repo default. There are a couple reasons to do this. First, the dep issue I previously mentioned will prevent dep from importing v3, so creating a separate branch would allow important future updates to be easily backported to v2 for dep users. Second, this would be friendlier to legacy go get users who are just grabbing master.
  • Tag the new commit at v3.0.0.

If you want more info about this process, the wiki link above has links to more resources.

This was referenced Jan 6, 2021
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3
Development

No branches or pull requests
9 participants
@markbates @pedromorgan @dreampuf @dhui @stanislas-m @ivy @Scapal @bvisness @flibustenet