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

Jump to bottom

std,cmd: add a unit test to ensure that src/go.mod and src/cmd/go.mod are in sync #36907

Closed
bcmills opened this issue Jan 30, 2020 · 9 comments
Closed

std,cmd: add a unit test to ensure that src/go.mod and src/cmd/go.mod are in sync #36907

bcmills opened this issue Jan 30, 2020 · 9 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.15

Comments

@bcmills
Copy link
Contributor

bcmills commented Jan 30, 2020

In #36851 (comment), @FiloSottile noticed that the std module and the cmd module currently depend on different commits of the golang.org/x/sys module.

We should prevent such skew in the future. It should be straightforward to write a test that uses go list (or a similar approach) to check that the versions required by std and cmd agree for the modules in the intersection of their dependencies.
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. modules labels Jan 30, 2020
@bcmills bcmills added this to the Go1.15 milestone Jan 30, 2020
@bcmills bcmills self-assigned this Jan 30, 2020
@ianlancetaylor
Copy link
Contributor

It's not wholly clear to me why we need to vendor golang.org/x/sys in two different places.

@FiloSottile
Copy link
Contributor

See also #36852.

@bcmills
Copy link
Contributor Author

bcmills commented Jan 30, 2020

@ianlancetaylor: std needs golang.org/x/sys/cpu, whereas cmd needs golang.org/x/sys/unix.

We don't put std and cmd in the same module because we want to make it easy to see that std does not depend on anything unexpected, especially outside of x/. In contrast, cmd depends on a few modules external to the project (github.com/ianlancetaylor/demangle and github.com/google/pprof).

@bcmills
Copy link
Contributor Author

bcmills commented Jan 30, 2020

@FiloSottile, note that the test described in #36852 requires network access (in order to download the pristine dependencies for comparison), while the test described here can (at least partially) be computed offline from the contents of src/vendor/modules.txt and src/cmd/vendor/modules.txt.

@gopherbot
Copy link

Change https://golang.org/cl/217218 mentions this issue: cmd/internal/moddeps: check for consistent versioning among all modules in GOROOT

@bcmills bcmills added early-in-cycle A change that should be done early in the 3 month dev cycle. release-blocker labels Jan 31, 2020
@gopherbot
Copy link

Change https://golang.org/cl/217517 mentions this issue: all: update module dependencies

@dmitshur
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.15. That time is now, so friendly ping. If it no longer needs to be done early in cycle, that label can be removed.

gopherbot pushed a commit that referenced this issue Feb 19, 2020
all: update module dependencies
7299308 
Updates #36905
Updates #36907

Change-Id: I293dcef67800d5c81ff3a254bbd49309c5880710
Reviewed-on: https://go-review.googlesource.com/c/go/+/217517
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur mentioned this issue Feb 21, 2020
Closed
@dmitshur
Copy link
Contributor

dmitshur commented May 5, 2020

I'm very glad this test exists and found it to work well while developing CL 231657. Thank you @bcmills and @FiloSottile!


Here's an excerpt from a moment when the x/sys versions diverged between the two modules; the test reported it correctly.

--- FAIL: TestDependencyVersionsConsistent (0.00s)
    moddeps_test.go:217: Modules within GOROOT require different versions of golang.org/x/sys.
    moddeps_test.go:229: cmd	requires v0.0.0-20191204072324-ce4227a45e2e
    moddeps_test.go:229: std	requires v0.0.0-20200323222414-85ca7c5b95cd
FAIL
FAIL	cmd/internal/moddeps	10.795s

@gopherbot
Copy link

Change https://golang.org/cl/236600 mentions this issue: cmd/internal/moddeps: don't skip directories if there are unusual files

gopherbot pushed a commit that referenced this issue Jun 8, 2020
@dmitshur
cmd/internal/moddeps: don't skip directories if there are unusual files
bb89014 
Previously, if there was a non-directory file with the name vendor or
testdata in the Go source tree, it was possible for some directories
to be skipped by filepath.Walk performed in findGorootModules.

As unusual and unlikely as such non-directory files are, it's better
to ensure all directories are visited, and all modules in the GOROOT
source tree are found.

This increases confidence that tests relying on findGorootModule
will not have unexpected false negatives.

For #36851.
For #36907.

Change-Id: I468e80d8f57119e2c72d546b3fd1e23c31fd6e6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/236600
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@dmitshur dmitshur mentioned this issue Sep 24, 2020
Closed
@golang golang locked and limited conversation to collaborators Jun 4, 2021
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
5 participants
@FiloSottile @dmitshur @ianlancetaylor @bcmills @gopherbot