search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.33k stars 17.58k forks source link

cmd/go: creating v2+ modules has lower success rate than it could #31543

Open thepudds opened 5 years ago

thepudds commented 5 years ago

What version of Go are you using (go version)?

go 1.12.4

Does this issue reproduce with the latest release?

Yes, including tip.

What did you do?

Observed many problems in go.mod files for repositories with v2+ semver tags.

What did you expect to see?

A smaller rate of problems.

Some concrete issues

"go.mod files that I happen to examine" is not an unbiased random sample, but from what I have seen, the failure rate is not low when someone from the broader community opts in to modules for a pre-existing v2+ set of packages.

Issue 1

Of the various go.mod files I have looked at in repos with v2+ semver tags over the last several months, I estimate more than 50% of those go.mod files are incorrect due to missing the required /vN at the end of the module path.

Three samples I happened to see in the last few days:

All of those are missing the required /v2 in the module statement.

Issue 2

Even when someone does know to add a /vN to their module statement, they might not know to update their own import statements in their .go code within the module to add the /vN, or might miss some import statements. The end result in that case is a module accidentally depending on the v1 version of itself.

Issue 3

Many modules created from v2+ repositories are not following the best practice outlined in https://github.com/golang/go/issues/25967#issuecomment-422828770 of incrementing the major version when adopting modules for a pre-existing v2+ repository. There can be a fairly narrow set of cases where it is not desirable to do so (for example, if you are pursuing a more sophisticated forwarding or type aliasing solution). However, based on what I have observed, I would certainly guess in the large majority of cases I have seen it was not after considered deliberation by the module author of the pros and cons of doing so, but rather more often due to the module author not knowing it is considered desirable in most circumstances.

Some likely contributing factors

Factor 1

go mod init seems to complete successfully for a v2+ repository that has not yet opted in to modules, but produces the wrong result.

Sample:

GO111MODULE=off go get -d github.com/pierrec/lz4
cd $GOPATH/src/github.com/pierrec/lz4
git describe
# output: v2.1.2
rm go.mod
go mod init
head -1 go.mod
# output: module github.com/pierrec/lz4

At first glance, the go.mod file seems to have been created properly or at least there is no error or warning, but the result is missing the required /v2 in the module statement given that repository is tagged v2.1.2.

I would guess a healthy percentage of gophers might trust the resulting go.mod created by go mod init.

Factor 2

go get does not complain if you do go get foo without a /vN for repository foo that has v2+ semver tags but foo does not have /vN in the module statement in foo's go.mod.

If someone incorrectly creates a go.mod without a required /vN in the module statement (e.g., manually, or after hitting Factor 1 above) and then does one of several permutations of go get foo (again without the /vN), they do not get a warning or error, and can move forward thinking their v2+ module has been created successfully after doing basic tests.

Here is a sample. For this lz4 repo:

Sample results:

# allows "bad" version:
require github.com/pierrec/lz4 v2.1.1+incompatible
 result github.com/pierrec/lz4 v2.1.1+incompatible                

# allows "bad" version converted to pseudo-version:
require github.com/pierrec/lz4 v2.1.1
 result github.com/pierrec/lz4 v0.0.0-20190327172049-315a67e90e41 

# silently gives you version just before first "bad" version:
require github.com/pierrec/lz4 latest
 result github.com/pierrec/lz4 v2.0.5+incompatible

You get the same results if you specify require in a consumer go.mod, or if you do go get with the same version string.

On the other hand, if you use /v2 in the go get or require, it reports an error:

# error if asked for v2.1.1 using /v2
require github.com/pierrec/lz4/v2 v2.1.1
go: github.com/pierrec/lz4/v2@v2.1.1: 
 go.mod has non-.../v2 module path "github.com/pierrec/lz4" (and .../v2/go.mod does not exist) at revision v2.1.1
go: error loading module requirements

# error if asked for latest using /v2
require github.com/pierrec/lz4/v2 latest
go: github.com/pierrec/lz4/v2@v2.1.2: 
 go.mod has non-.../v2 module path "github.com/pierrec/lz4" (and .../v2/go.mod does not exist) at revision v2.1.2
go: error loading module requirements

However, accidentally doing go get foo for a v2+ module (and not knowing it should be go get foo/v2) is likely somewhat correlated with not knowing to create the go.mod with module foo/v2 in the first place.

Separately, that is not the easiest to understand error if you are not steeped in modules, but it is at least an error.

Sample test that generates results above:

cd $(mktemp -d)
export GOPATH=$(mktemp -d)
echo 'package "tempmod"' > tempmod.go

for ver in ' v2.1.1+incompatible' ' v2.1.1' ' latest' '/v2 v2.1.1' '/v2 latest'; do
  echo 'module tempmod' > go.mod
  echo "require github.com/pierrec/lz4$ver" | tee --append go.mod
  echo 'result:'
  go list -m all
  echo
done

Factor 3

https://github.com/golang/go/issues/25967#issuecomment-422828770 is not widely known and is likely undercommunicated (e.g., as mentioned by Bryan in https://github.com/golang/go/issues/27009#issuecomment-440456278). (edit: sorry, corrected first link here).

Factor 4

More generally, how to properly create modules has likely been undercommunicated across the various channels available to the core Go team, including the golang.org blog, twitter, golang-nuts, etc.

Suggestions

Given time before 1.13 is short, I suggest aiming for some cheap but useful improvements.

Suggestion 1

It is probably not advisable to treat something like go get github.com/pierrec/lz4@v2.1.1 as an error even though v2.1.1 has an incorrect go.mod. It is debatable what 1.11 could or should have done in that scenario, but it is likely too late to treat as an error.

However, it seems that at least issuing a warning would be advisable. In particular, of the three variations of require or go get that are are silent in Factor 2 above, the most common is likely go get foo or go get foo@latest. It would very likely help nudge the community in the right direction to make go get foo or go get foo@latest issue a warning if the latest version of foo has v2+ semver tags but has a go.mod file that is missing a /vN. Presumably the cmd/go code is already reading and rejecting that latest go.mod, so perhaps this could be a small change. That would likely cause an author to notice they have a go.mod file that is bad in this way, or at least trigger a consumer filing an issue so that the author becomes aware after releasing a bad go.mod file.

Suggestion 2

Make the error from go mod init (without any arguments) much clearer. This would help in the case when someone has cloned their repo and is outside of their traditional GOPATH. It could say something along the lines of:

$ go mod init
go: cannot determine module path for source directory /tmp/foo (outside GOPATH, module path must be specified)
    example usage:
       'go mod init example.com/my/repo' to initialize a v0 or v1 module
       'go mod init example.com/my/repo/v2' to initialize a v2 module

    see https://golang.org/s/go-mod-init-faq for more details.

A side benefit is that message also helps people who don't know what to provide to go mod init (which is a healthy count of gophers), as well as help hint to people the most common module path is the repo root. (If someone wants to have a multi-module repo or place a go.mod outside of the repo root, that person already has a need to get deeper with modules than just reading an error message from go mod init, and the FAQ link could cover some of that either directly or through pointers).

A better solution would be to make go mod init aware of the git tags so that the proper /vN is applied by default in the module statement. However, I would guess there is no time for that for Go 1.13 (unless of course some more people could help polish modules in the small amount of time before freeze).

Suggestion 3

Re-open #27248 (including see comment https://github.com/golang/go/issues/27248#issuecomment-480319522). Aim for a golang.org/x utility that is usable by the community prior to Go 1.13 going GA that can properly add and adjust the /vN in go.mod files and .go code. A golang.org/x utility could be done after the freeze but before 1.13, as far as I understand?

This would help on multiple fronts outlined here, including helping people properly update their own go.mod, and cut down on incidents of a module accidentally depending on the v1 version of itself due to not updating import statements, as well as generally reduce the transitional pain of Semantic Import Versioning.

Suggestion 4

There are various suggestions in the go release issue that would help here, but it seems most or all of those won't be happening for 1.13, and also go release will not catch everything, including in the near-term while people might not be using go release yet).

One question would be if go release could start out as a golang.org/x utility in order to allow for iteration during the 1.13 freeze? (edit: added this as a separate suggestion in https://github.com/golang/go/issues/26420#issuecomment-484617703).

Suggestion 5

Increase energy on blogging, twitter PSAs or reminders, golang-nuts PSAs or reminders, etc., as well as ideally a increased push on smaller polish items in modules like better error messages.

gopherbot commented 5 years ago

Change https://golang.org/cl/173318 mentions this issue: cmd/go: expand cannot determine module path error

tbpg commented 5 years ago

Hi @thepudds. I sent https://golang.org/cl/173318 to update the go mod init error with your second suggestion. I left out checking the current repo tags/remotes to try to guess at the module path/version for the sake of time.

Also, @jayconrod pointed out we need to be careful with modules not at the repo root.

gopherbot commented 5 years ago

Change https://golang.org/cl/173721 mentions this issue: cmd/go: warn if falling back to a pseudo-version

gopherbot commented 5 years ago

Change https://golang.org/cl/174180 mentions this issue: cmd/go: error if mismatched major versions causes use of pseudo-version

RoarkeRandall commented 5 years ago

Just giving my experience. It wasn't immediately clear to me that semantic import version was required for v2 and above, and caused a bit of a headache to debug why I was getting psuedo versions in the go.mod file. I believe we fell into the Factor 2 section. Some more clear warnings or error logs would have been of great help.

Additionally, I think many teams will want to continue committing to master without using subdirectories. Currently that "option" is listed under the "Branching" option as a note, and implies that issues may arise from it. I can see this being a major hinderance to people adopting go modules.

silverwind commented 5 months ago

Imho it's time to relax the v2+ module path requirement and allow version to be defined purely in the git tags alone, e.g. what https://github.com/golang/go/issues/44550 proposed.

Many authors are struggling with the module path requirement, I've just seen it again at https://github.com/editorconfig-checker/editorconfig-checker.