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

cmd/go: comment indicating module is an indirect dependency is not removed when the comment has no whitespace #45932

Closed
komuw opened this issue May 3, 2021 · 5 comments

Comments

@komuw
Copy link
Contributor

@komuw komuw commented May 3, 2021

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

go version go1.16 darwin/amd64

Does this issue reproduce with the latest release?

yes

gotip version
go version devel go1.17-9f347035 Mon May 3 19:14:16 2021 +0000 darwin/amd64

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/me/Library/Caches/go-build"
GOENV="/Users/me/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/me/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/me/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/me/mystuff/ote/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gc/sfs6hvtd1r392kn91n3jp17m0000gn/T/go-build210366927=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

rm -rf /tmp/example; mkdir -p /tmp/example; cd /tmp/example
tee -a go.mod <<EOF
module example
go 1.16
require github.com/pkg/errors v0.9.1 //indirect
EOF

tee -a main.go <<EOF
package main
import "github.com/pkg/errors"
func main(){errors.New("hi")}
EOF
go mod tidy

What did you expect to see?

  • The //indirect comment should be removed from the require stanza

What did you see instead?

  • The //indirect comment is replaced with a //ct comment
@gopherbot gopherbot added this to the Unreleased milestone May 3, 2021
@komuw
Copy link
Contributor Author

@komuw komuw commented May 3, 2021

My guess is that this is caused by the call to len() here; https://github.com/golang/mod/blob/858fdbee9c245c8109c359106e89c6b8d321f19c/modfile/rule.go#L509-L514 which expects the //indirect comment to always have a space between // and indirect

@gopherbot
Copy link

@gopherbot gopherbot commented May 4, 2021

Change https://golang.org/cl/316569 mentions this issue: modfile: take into account that // indirect comments may not be well formatted

@bcmills
Copy link
Member

@bcmills bcmills commented May 4, 2021

CL 316569 fixes this in the x/mod repo (thanks, @komuw!). I'll vendor it into the cmd module to fix the bug in go mod tidy and add a regression test at that point.

gopherbot pushed a commit to golang/mod that referenced this issue May 4, 2021
…formatted

When there is an // indirect comment next to a dependency that is not actually indirect;
go mod tidy should remove it.
This was not the case when the //indirect comment was badly formatted.

We now check whether such a comment exists irrespective of the formatting.

Updates golang/go#45932

Change-Id: I6a7dca23059a0aca6f8f940da975a0d79f701571
GitHub-Last-Rev: b884ee1
GitHub-Pull-Request: #3
Reviewed-on: https://go-review.googlesource.com/c/mod/+/316569
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented May 4, 2021

Change https://golang.org/cl/316751 mentions this issue: cmd/go: update x/mod to fix "//indirect" comment editing

@gopherbot gopherbot closed this in 10a082a May 4, 2021
@bcmills bcmills changed the title x/mod: comment indicating module is an indirect dependency is not removed when the comment has no whitespace cmd/go: comment indicating module is an indirect dependency is not removed when the comment has no whitespace May 4, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented May 5, 2021

Change https://golang.org/cl/317129 mentions this issue: cmd/go.sum: remove untidy checksums

gopherbot pushed a commit that referenced this issue May 5, 2021
I missed the 'go mod tidy' step in CL 316751 because I forgot to run
the cmd/internal/moddeps test in long mode. 😞

Updates #45932

Change-Id: Ic3f9b303ad5798ecd8cb044d4b8c766aa820bf69
Reviewed-on: https://go-review.googlesource.com/c/go/+/317129
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants