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

x/vgo: add ability to trim go.mod #24101

Closed
rogpeppe opened this issue Feb 24, 2018 · 10 comments
Closed

x/vgo: add ability to trim go.mod #24101

rogpeppe opened this issue Feb 24, 2018 · 10 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@rogpeppe
Copy link
Contributor

go version devel +104445e Wed Feb 7 19:22:09 2018 +0000 linux/amd64 vgo:2018-02-20.1

There seems to be no straightforward way of pruning unused dependencies from a go.mod file.
Perhaps a variant of vgo clean might work for this.

@gopherbot gopherbot added this to the vgo milestone Feb 24, 2018
@kardianos
Copy link
Contributor

kardianos commented Feb 28, 2018

Related to #24150 I'll paste what I wrote there:

The hard thing about unused dependencies is they are build tag dependant. While there is a maximum dependency graph for all tags, it is reasonably common to not care about a subset of them.

In other words, there are three definitions of unused:

  1. GOOS GOARCH current unused.
  2. Some module specific defined list of tags or list of ignore tags unused.
  3. Max graph unused.

It would be nice if go list could take a tag Boolean expression and report the use status of the module. We could then use that primative to compute and of the thee views above. Then to prune something like:

go list -unused -tag "!appengine AND !solaris" | go get -remove

@rsc
Copy link
Contributor

rsc commented Mar 27, 2018

After the script in #24150 I don't see much in go.mod:

$ cat go.mod
module "github.com/test/testmod"

require (
	"gopkg.in/macaroon-bakery.v2" v0.0.0-20180209090814-22c04a94d902
)
$ 

What is there to prune?

@rsc
Copy link
Contributor

rsc commented Mar 27, 2018

Never mind, I see this was filed before #24150 (despite the link confusing me) and is about the general pruning topic (which is difficult).

@akavel
Copy link
Contributor

akavel commented Mar 28, 2018

FWIW, to add to the discussion, I'd like to quickly show one of the possible solutions, which worked perfectly well for us. In our in-house vendoring tool, we went with the semantics of pruning = "burning the house and rebuilding it from scratch, in additive way" (see zpas-lab/vendo/use-cases.md, search for "vendo-recreate"). I see it as having various advantages over pruning = "selectively deleting", such as good reproducibility, simpler operation, and keeping only one code path to the same result (no hysteresis), also meaning only one code path ever excercised (no different bugs on different paths, which again could lead to possible hysteresis).

I'll try to explain the approach here, as tersely as I can. I purposefully don't try to stick too strictly to go.mod concepts, to have more freedom of expression; I assume fine-tuning this, exploring further and fitting to vgo concepts can be done later if needed.

In our tool (named vendo), whenever we need to add or remove some package(s) from _vendor/ directory, we run: vendo recreate -platforms=linux_amd64,windows_amd64. This can actually be split conceptually to two/three operations; those don't really exist as such in vendo, but they could; recreate is just a merged amalgm of them. I'll try to rephrase them as operations in some abstract vgox tool, an "experimental fork of vgo":

$ vgox burn
$ GOOS=linux GOARCH=amd64 vgox build
$ GOOS=windows GOARCH=amd64 vgox build

In a simplest and crudest implementation, vgo burn could potentially be just rm go.mod, thus becoming a simple:

$ rm go.mod
$ GOOS=linux GOARCH=amd64 vgox build
$ GOOS=windows GOARCH=amd64 vgox build

However, the most glaring problem for us in this approach was that it would be losing all "explicit overrides" information included in the original "go.mod". So, in our tool, the recreate command was more equivalent to something like:

$ mv go.mod go.mod.old
$ GOOS=linux GOARCH=amd64 vgox build -lookup go.mod.old
$ GOOS=windows GOARCH=amd64 vgox build -lookup go.mod.old

The -lookup go.mod.old (-consult? -override?...) option would have such an effect, that whenever vgox would want to add a require "foo/bar" ... entry to go.mod, it would first look into go.mod.old, and if an entry for "foo/bar" existed there, it would be prioritized (reused/copied) over what vgox would deduce otherwise.

Now that I think of it, in case the vgox tool was built as described above, the go.mod.old file could actually be called e.g. go.mod.overrides, and just explicitly kept in the user's repo, besides regular go.mod. The vgox tool could the just check if such a file exists, and if yes, automatically assume that an equivalent to -lookup go.mod.overrides option was added to its commandline. The operation could then become:

$ vim go.mod.overrides
$ GOOS=linux GOARCH=amd64 vgox build     # detects go.mod.overrides and consults its contents
$ GOOS=windows GOARCH=amd64 vgox build   # again, consults go.mod.overrides

Some extra note regarding our real tool (vendo):

  • in our case, we used a vendor.json file; we kept the value of the -platform=... flag in an extra field there, so that a user wouldn't have to remember and re-type it each time.

@rsc
Copy link
Contributor

rsc commented Mar 30, 2018

There is already code (e.g. vgo vendor) that attempts to understand the full set of imports required for building a particular module's packages and its dependency packages (not dependency modules). It could easily help trim go.mod as well, and I intend to do that, maybe in go release. I also intend to have a comment // keep that means don't remove this even if it looks unused. People who want to list versions of associated commands used during development could use that comment, and then "go install" of those commands would use the version specified by go.mod.

@rsc rsc changed the title x/vgo: provide a way to prune unused dependencies from go.mod x/vgo: add ability to trim go.mod Mar 30, 2018
@rsc
Copy link
Contributor

rsc commented Mar 30, 2018

We still need to decide what command should do this. Actually doing it is mostly straightforward.

@rsc rsc added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 30, 2018
@rogpeppe
Copy link
Contributor Author

rogpeppe commented Apr 3, 2018

I like the idea of a // keep comment.

We still need to decide what command should do this.

My current opinion is that unused entries in go.mod should be aggressively pruned. AIUI any vgo command can add dependencies to go.mod - I think that probably any command should remove unused dependencies too (modulo the // keep comment).

Unused dependencies in a go.mod file can be thought of a little like unused dependencies in a Go file. Similar to those, I wonder if a dependency in a go.mod file that isn't used at all by a module or any of its dependencies should be considered a build error when it does have a // keep comment (arbitrarily removing a dependency with a // keep command seems like it would be against the intent of the comment).

@rogpeppe
Copy link
Contributor Author

rogpeppe commented Apr 3, 2018

Follow-on thought: I wonder if any dependency that is explicitly vgo get with a specific should have a // keep comment added automatically, as it implies that the user has explicitly thought about the version number, so we perhaps want to avoid losing that information. I'm quite unclear as to whether that's actually a good idea or not though.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2018

AIUI any vgo command can add dependencies to go.mod - I think that probably any command should remove unused dependencies too (modulo the // keep comment).

You can decide you are missing a dependency with purely local information: here is one package, it has an import, and nothing provides that import. In contrast, deciding that a dependency is unused requires global information: for all packages in the module, for all possible build tag combinations that might be used to build them, no source file needs that import. Most vgo commands do not look at "all packages". I don't believe they should start. At the least that would be confusing (reporting errors about code not being built) and inefficient (reading files that are unnecessary to the task at hand).

I wonder if any dependency that is explicitly vgo get with a specific should have a // keep comment added automatically, as it implies that the user has explicitly thought about the version number, so we perhaps want to avoid losing that information.

Note that // keep is supposed to be about "keep this module listed in the file" not pin this exact version. It's never possible to pin an exact version: some other part of the build can always force a newer one. So any implied intent about versions, even if true, doesn't matter. That leaves the question of whether there's an implied intent, when you say go get x@v, to keep x in the mod file. I don't think that's necessarily true. I expect the point of that command will at least some of the time (perhaps most of the time) be saying "well if you must use x, then at least use v or later". If the build stopped needing x, it would be consistent with that intent to drop it entirely.

In any event, if pruning is manually triggered (as it probably must be) then it will probably be OK for // keep to be manually triggered as well. Our experience with a similar directive in BUILD files at Google is that they are by far the exception rather than the rule, and I expect a similar outcome here. (If not we should absolutely try to understand why and whether that implies a change in design.)

@gopherbot
Copy link

Change https://golang.org/cl/118877 mentions this issue: cmd/go/internal/modcmd: remove unused modules during -sync

@golang golang locked and limited conversation to collaborators Jun 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants