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: mod -sync should trim go.sum #26381

Closed
ainar-g opened this issue Jul 14, 2018 · 7 comments

Comments

Projects
None yet
8 participants
@ainar-g
Copy link
Contributor

commented Jul 14, 2018

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

Does this issue reproduce with the latest release?

go version devel +6f96cf2 Fri Jul 13 22:29:48 2018 +0000 linux/amd64

What did you do?

$ mkdir /tmp/foo && cd /tmp/foo
$ echo 'package foo // import "foo"' > main.go
$ go mod -init
$ go get github.com/lib/pq
$ cat go.sum
github.com/lib/pq v0.0.0-20180523175426-90697d60dd84/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
$ go mod -sync
$ cat go.sum

What did you expect to see?

Nothing.

What did you see instead?

github.com/lib/pq v0.0.0-20180523175426-90697d60dd84/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=

@oiooj oiooj added the modules label Jul 14, 2018

@oiooj oiooj added this to the Go1.11 milestone Jul 14, 2018

@kardianos

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2018

Files in go.sum are never intended to be removed. It keeps a list of all modules seen.

@ainar-g

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2018

That approach (if indeed the defined behaviour) is (1) badly documented, (2) probably wouldn't scale very well, (3) raises questions.

Regarding (1) here is what go help modules had to say about go.sum:

The go command maintains, in the main module's root directory alongside
go.mod, a file named go.sum containing the expected cryptographic checksums
of the content of specific module versions. Each time a dependency is
used, its checksum is added to go.sum if missing or else required to match
the existing entry in go.sum.

I added the emphasis. My assumptions from reading this section are:

  • "Maintaining" means adding what is used and removing what is unused
  • A dependency is "used" when one of packages in the module imports it.

By (2) I mean that when we consider a project like Hugo, with hundreds of dependencies being added, removed, and updated daily in several branches. go.sum would accumulate trash very quickly. (Ideally, go mod -sync would remove any line that is not a hash of a used package, so that resolving git conflicts is as easy as fixing go.mod and running go mod -sync.)

By (3) I simply mean that the "Go 1.11 Modules Experiment" comes with a number of tools to support modules in a project. So why not include a tool to maintain go.sum? Don't get me wrong, I can write an awk script to do that, but why, if almost any project will need something like that?

@myitcv

This comment has been minimized.

Copy link
Member

commented Jul 14, 2018

/cc @rsc @bcmills for confirmation of the intended semantics here.

@bcmills

This comment has been minimized.

Copy link
Member

commented Jul 16, 2018

I'd like to get @FiloSottile's input on what we should do here.

On the one hand, the more we leave in the go.sum file the more likely we are to catch actual verification failures, particularly if the dependencies in the go.sum are likely to be added again in the future (for example, if they are more recent versions of modules that we do still require). On the other hand, it does seem a bit strange to prune modules out of go.mod without also pruning them out of go.sum.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Jul 16, 2018

I appreciate the fact that go.sum is append only. It secures the case of reintroducing a dependency and improves future cross-pollination experiments where hashes are learned from other repositories.

One way to look at this is that go.sum is just a way (currently, the only one) to check package integrity, not really bound to go.mod. In the future we might have system-wide go.sum, or remote auditable services instead.

For projects so big it becomes a problem, there can be an external tool to trim go.sum.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2018

The current "add only" behavior (the file is sorted so it's not exactly "append-only") was intended. I think that was more defensible when it was opt-in. Now that it's a required part of development (at @FiloSottile's urging), I do think we owe it to ourselves to make it a bit nicer, and that probably does include throwing away things that are no longer needed.

Right now there's too much path dependence: if you go get something, decide not to use it, and drop it, ideally you would be back where you started and instead there's scar tissue left in go.mod go.sum. In the worst case you're leaking names of things you might have tried that you don't even want people to know exist. And since the file is not exactly human-friendly it would be easy for unfortunate diffs to be missed in review.

I think you're right that go mod -sync is a good time to clean it up. It's not completely trivial but we should do it.

@rsc rsc changed the title cmd/go: go mod -sync removes a module from go.mod but not go.sum cmd/go: mod -sync should trim go.sum Jul 17, 2018

@rsc rsc added NeedsFix and removed NeedsDecision labels Jul 17, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Jul 18, 2018

Change https://golang.org/cl/124713 mentions this issue: cmd/go: scrub go.sum during go mod -sync

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.