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/build: TryBots should check go.mod files are tidy #31640

Open
jayconrod opened this issue Apr 23, 2019 · 5 comments

Comments

@jayconrod
Copy link
Contributor

commented Apr 23, 2019

CL 172972 added a dependency on golang.org/x/sync in golang.org/x/tools. The go.mod and go.sum files were not updated.

There's nothing wrong with this dependency, but it should have been more noticeable during code review. If unexpected requirement were added to go.mod, human reviewers would see it.

TryBots should report an error if go.mod and go.sum change with go mod tidy. This will make sure dependency changes are explicit, and it will keep builds deterministic.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Is there a flag to go mod tidy that'll let it report whether things are tidy without doing any network requests?

Because we're increasingly blocking outbound network access on builders.

But we do allow GOPROXY access, which we set, so if it needs that, I suppose that's okay.

@jayconrod

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

There's not a simple way to do that, but it's a common feature request we should implement. I'm sure there is an issue filed somewhere.

go list -mod=readonly -test ./... would probably be good enough to check that we don't need to add new modules. Setting GOPROXY=off disables most requests, but the ?go-get=1 requests for resolving import paths don't go through the proxy, so those would still happen for unknown modules. Edit: resolving import paths actually does go through GOPROXY.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

There's not a simple way to do that, but it's a common feature request we should implement. I'm sure there is an issue filed somewhere.

If that's on the way, I'd rather wait for that.

Cross-reference the two bugs when you find it?

@jayconrod

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

#27005 would add a -check flag. go mod tidy would exit non-zero if changes are needed.

I don't think we'll have time to implement it before the freeze though.

@nmiyake

This comment has been minimized.

Copy link

commented May 23, 2019

Note that go list -mod=readonly -test ./... will only check that you don't need to add new modules given the build configuration of the environment that ran the command -- it will not flag any new modules that may be required when using different build tags (OS/arch/etc.). I believe that mod tidy -check (or an equivalent) is the only way to definitively guarantee that the go.mod and go.sum are in the proper state for all possible consumers.

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