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

cmd/go: malformed go.sum error is ignored #62345

Closed
chmeliik opened this issue Aug 29, 2023 · 6 comments
Closed

cmd/go: malformed go.sum error is ignored #62345

chmeliik opened this issue Aug 29, 2023 · 6 comments
Labels
GoCommand cmd/go help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@chmeliik
Copy link
Contributor

chmeliik commented Aug 29, 2023 •
edited

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

$ go version
go version go1.19.12 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/acmiel/.cache/go-build"
GOENV="/home/acmiel/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/acmiel/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/acmiel/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/golang"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.12"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/test/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2703022439=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Intentionally break a go.sum file and observe go mod download behavior

create test project 
cat << EOF > main.go
package main

import (
	"fmt"

	"rsc.io/quote/v4"
)

func main() {
	fmt.Println(quote.Hello())
}
EOF

cat << EOF > go.mod
module acmiel.test/foo

go 1.19

require rsc.io/quote/v4 v4.0.1

require (
	golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c // indirect
	rsc.io/sampler v1.3.0 // indirect
)
EOF

cat << EOF > go.sum
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkL0i8=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
rsc.io/quote/v4 v4.0.1
rsc.io/quote/v4 v4.0.1/go.mod h1:w/DafQky66grMesu3uPhdDMS3knhBippwwemZtMOyCI=
rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
EOF

go.sum (note that line 3 is broken):

golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkL0i8=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
rsc.io/quote/v4 v4.0.1
rsc.io/quote/v4 v4.0.1/go.mod h1:w/DafQky66grMesu3uPhdDMS3knhBippwwemZtMOyCI=
rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=

go.sum after running go mod download:

golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkL0i8=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
rsc.io/quote/v4 v4.0.1/go.mod h1:w/DafQky66grMesu3uPhdDMS3knhBippwwemZtMOyCI=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=

The zip checksums in the broken part of go.sum were removed. I think the reason is that readGoSum returns an error but the callers of readGoSum (first, second) ignore that error. Before returning the error, readGoSum will already have updated the global goSum structure. Effectively, this means that commands which parse go.sum will parse only the non-broken part and ignore the rest.

What did you expect to see?

An error telling me that my go.sum is broken

What did you see instead?

No error, handling only the non-broken part of go.sum
@Jorropo
Copy link
Member

Jorropo commented Aug 29, 2023

I enjoy this behavior on go mod tidy, when I have merge conflicts with my go.mod and go.sum files I just merge two go.mod files and leave go.sum as-is, then go mod tidy fix all of it. Double check for good measure but it always gets it right AFAIT (picks the oldest versions of the two).

@chmeliik
Copy link
Contributor Author

Yup, go mod tidy does fix broken go.sum 👍

Definitely a low-priority bug, just reporting it for awareness

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Aug 29, 2023
@bcmills bcmills added this to the Backlog milestone Aug 29, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 29, 2023

I agree that go mod tidy should continue to fix a broken go.sum file, and I also agree that filling in those missing error checks seems like a good idea. Want to send a fix?

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. modules labels Aug 29, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 29, 2023
@chmeliik
Copy link
Contributor Author

@bcmills I'd be happy to!

chmeliik added a commit to chmeliik/go that referenced this issue Sep 11, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
80fc35b 
In 6232dad, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 12, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
82c84e6 
In 6232dad, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345

Signed-off-by: Adam Cmiel <acmiel1@gmail.com>
chmeliik added a commit to chmeliik/go that referenced this issue Sep 12, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
175dd76 
In 6232dad, many errors related to
go.sum were changed from base.Fatal to error returns. The malformed
go.sum error was lost in the process. Currently, when go encounters
a malformed go.sum file, go will read the well-formed part of the file
and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345

Signed-off-by: Adam Cmiel <acmiel1@gmail.com>
chmeliik added a commit to chmeliik/go that referenced this issue Sep 12, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
f487da2 
In 6232dad, many errors related to
go.sum were changed from base.Fatal to error returns. The malformed
go.sum error was lost in the process. Currently, when go encounters
a malformed go.sum file, go will read the well-formed part of the file
and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 12, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
8608968 
In 6232dad, many errors related to
go.sum were changed from base.Fatal to error returns. The malformed
go.sum error was lost in the process. Currently, when go encounters
a malformed go.sum file, go will read the well-formed part of the file
and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
@chmeliik chmeliik mentioned this issue Sep 12, 2023
Closed
@chmeliik
Copy link
Contributor Author

Ended up not being as simple as I expected (simply filling in the missing error checks did not achieve what I needed)

Nevertheless, I've sent a somewhat hacky fix 😅

@gopherbot
Copy link

Change https://go.dev/cl/527575 mentions this issue: cmd/go: make malformed go.sum a fatal error

chmeliik added a commit to chmeliik/go that referenced this issue Sep 14, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
b176d66 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 14, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
40569c4 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 18, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
b5d5478 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 18, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
ae4603c 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 18, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
219ee55 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
chmeliik added a commit to chmeliik/go that referenced this issue Sep 19, 2023
@chmeliik
cmd/go: make malformed go.sum a fatal error
feae769 
In CL 197062, many errors related to go.sum were changed from base.Fatal
to error returns. The malformed go.sum error was lost in the process.
Currently, when go encounters a malformed go.sum file, go will read the
well-formed part of the file and then silently ignore the rest.

The motivation behind moving away from base.Fatal was to make the errors
show up in -json output. Simply propagating the malformed go.sum error
would not achieve this:

- For an argument-less 'go mod download -json' with a go>=1.17 module,
  a malformed go.sum causes an error during LoadModGraph already, before
  go ever starts downloading modules and printing their JSON.
- In other cases, a malformed go.sum would be reported as Error for one
  of the modules (presumably the one which gets downloaded first) but
  none of the others.
- In either case, 'go mod download' manages to download enough data to
  succeed on a re-run, making the error look intermittent.

Switch the error back to a Fatal one, but give 'go mod tidy' an
exception to let it fix broken go.sum files.

Fixes golang#62345
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

cmd/go: make malformed go.sum a fatal error chmeliik/go
4 participants
@bcmills @gopherbot @Jorropo @chmeliik