x/build/cmd/coordinator: add control over build mode in subrepos with directory granularity #34190
Labels
Builders
x/build issues (builders, bots, dashboards)
FeatureRequest
FrozenDueToAge
NeedsFix
The path to resolution is known, but the work has not been done.
Testing
An issue that has been verified to require only test changes, not just a test failure.
Milestone
Comments
dmitshur
added
Testing
dmitshur
changed the title
|
Change https://golang.org/cl/194277 mentions this issue: |
|
Change https://golang.org/cl/194397 mentions this issue: |
|
I've tested this change on CL 191018 by restarting trybots just now, and watched the build logs very closely. So far, everything is looking good and working as intended. It correctly determined the build mode as GOPATH mode for the "linux-amd64 (Go 1.12.x)" builder, and as module mode for all other builders (since they're running Go 1.13 and newer, and x/tools repo has a go.mod file at root). Here are build logs for various builds of the successful trybot run: linux-386 (Go tip) - tests ran in module modelinux-amd64 (Go 1.13.x) - tests ran in module modelinux-amd64 (Go 1.12.x) - tests ran in GOPATH mode with x/tools/gopls directory skippedlinux-amd64-race (Go tip) - ran in module modewindows-386-2008 (Go tip) - ran in module modeandroid-amd64-emu (Go tip) - ran in module modeNotably, in the "linux-amd64 (Go 1.12.x)" build where tests ran in GOPATH mode, the I'll also watch the post-submit builders to make sure nothing unexpected happens there. |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Sep 11, 2019
It will be necessary to know whether the current environment and configuration has resulted in module mode or GOPATH mode being selected. Do this dynamically by invoking 'go env GOMOD', since the outcome depends on the version of Go and its default GO111MODULE behavior, as well as whether a go.mod file is added to the subrepo, and what environment overrides were applied by coordinator via the BuildConfig.ModulesEnv method. This requires the repository being tested to be available on disk, so move the step of fetching it to happen earlier. For now, this change only detects and logs the build mode. Future changes will use this information further. Updates golang/go#34190 Updates golang/go#32528 Updates golang/go#30233 Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3 Reviewed-on: https://go-review.googlesource.com/c/build/+/194277 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
|
There was a minor issue affecting the "build", "oauth2", "website", and "perf" repos being misdetected as being tested in GOPATH mode, while in reality they're always tested in module mode. It was fixed in Patch Set 8 of CL 194277. I've verified that the fix is working as expected. |
codebien
pushed a commit
to codebien/build
that referenced
this issue
Nov 13, 2019
It will be necessary to know whether the current environment and configuration has resulted in module mode or GOPATH mode being selected. Do this dynamically by invoking 'go env GOMOD', since the outcome depends on the version of Go and its default GO111MODULE behavior, as well as whether a go.mod file is added to the subrepo, and what environment overrides were applied by coordinator via the BuildConfig.ModulesEnv method. This requires the repository being tested to be available on disk, so move the step of fetching it to happen earlier. For now, this change only detects and logs the build mode. Future changes will use this information further. Updates golang/go#34190 Updates golang/go#32528 Updates golang/go#30233 Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3 Reviewed-on: https://go-review.googlesource.com/c/build/+/194277 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
codebien
pushed a commit
to codebien/build
that referenced
this issue
Nov 13, 2019
…TH mode This change adds controls to the dashboard package that makes it possible to skip certain directories in subrepos from being tested in GOPATH mode. It uses this control mechanism to skip the gopls directory in x/tools from being tested in GOPATH mode. That directory and everything inside is developed with support for module mode only. All other packages in x/tools continue to be supported both in module and GOPATH modes for the current and previous releases, as per release policy¹. ¹ https://golang.org/doc/devel/release.html#policy Fixes golang/go#34190 Change-Id: Icdb4d0e1419f69c84c9a3f9a33727a09a35599f4 Reviewed-on: https://go-review.googlesource.com/c/build/+/194397 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
cmd/coordinatoralready has support for running tests in module mode for some subrepos. For example, it's currently used foroauth2,build,perf, andwebsitesubrepos:https://github.com/golang/build/blob/a2cf19446e2ca6ac8153b261ab49f9be9394b5f4/dashboard/builders.go#L733-L734
Some subrepos, such as
x/tools, want to be able to mark parts of the subrepo as module-only, while still maintaining both GOPATH and module mode coverage for the rest of the subrepo. This will allow said subrepos to continue to benefit from test coverage by trybots and builders, without forcing them to support GOPATH mode in internal areas where it isn't worthwhile (e.g., the nested gopls module).We should add support for this to the coordinator. I've started the work.
/cc @toothrot @andybons @bradfitz @ianthehat
Plan
I plan to make it something we configure in code in
x/build/dashboard, since it's not expected to change very often. It will be a change to the algorithm inbuildStatus.runSubrepoTeststhat will use the data from the dashboard.This is related to issues #32528 and #32528 (work on this one will make those easier to resolve too).
The text was updated successfully, but these errors were encountered: