Skip to content

Commit d28f106

Browse files
dmitshurgopherbot
authored andcommitted
internal/task: read nested module's own go.mod files
As of the last change, it becomes easy to read the nested module's own go.mod file and determine whether it had a toolchain directive. Before, it was reusing the top-level go.mod file's decision for nested modules. I didn't realize earlier there is a way to use the lower-level 'go mod edit' command to drop a toolchain directive, but it turns out there is. Switch to it now - it's equivalent but fits slightly better in context. Fixes golang/go#68873. Change-Id: I1ea4bfd9e5deb4e72843887d7f8d68a4b2a67f3e Reviewed-on: https://go-review.googlesource.com/c/build/+/622396 Reviewed-by: Dmitri Shuralyov <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent fa508ab commit d28f106

File tree

2 files changed

+44
-17
lines changed

2 files changed

+44
-17
lines changed

internal/task/tagx.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ type TagRepo struct {
7171
Name string // Gerrit project name, e.g., "tools".
7272
ModPath string // Module path, e.g., "golang.org/x/tools".
7373
Deps []*TagDep // Dependency modules.
74-
HasToolchain bool // Whether the go.mod file has a toolchain directive when the workflow started.
7574
StartVersion string // The version of the module when the workflow started. Empty string means repo hasn't begun release version tagging yet.
7675
NewerVersion string // The version of the module that will be tagged, or the empty string when the repo is being updated only and not tagged.
7776
}
@@ -192,7 +191,6 @@ func (x *TagXReposTasks) readRepo(ctx *wf.TaskContext, project string) (*TagRepo
192191
result := &TagRepo{
193192
Name: project,
194193
ModPath: mf.Module.Mod.Path,
195-
HasToolchain: mf.Toolchain != nil,
196194
StartVersion: currentTag,
197195
}
198196

@@ -385,10 +383,6 @@ func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo, deps []T
385383
for _, dep := range deps {
386384
script.WriteString(" " + dep.ModPath + "@" + dep.NewerVersion)
387385
}
388-
if !repo.HasToolchain {
389-
// Don't introduce a toolchain directive if it wasn't already there.
390-
script.WriteString(" toolchain@none")
391-
}
392386
script.WriteString("\n")
393387

394388
// Tidy the root module and nested modules.
@@ -418,10 +412,11 @@ func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo, deps []T
418412
if d.Name() == "go.mod" && !d.IsDir() { // A go.mod file.
419413
dir := pathpkg.Dir(path)
420414
dropToolchain := ""
421-
if !repo.HasToolchain {
415+
if had, err := hasToolchain(rootFS, path); err != nil {
416+
return err
417+
} else if !had {
422418
// Don't introduce a toolchain directive if it wasn't already there.
423-
// TODO(go.dev/issue/68873): Read the nested module's go.mod. For now, re-use decision from the top-level module.
424-
dropToolchain = " && go get toolchain@none"
419+
dropToolchain = " && go mod edit -toolchain=none"
425420
}
426421
script.WriteString(fmt.Sprintf("(cd %v && touch go.sum && go mod tidy%s)\n", dir, dropToolchain))
427422
outputs = append(outputs, dir+"/go.mod", dir+"/go.sum")
@@ -439,6 +434,20 @@ func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo, deps []T
439434
return buildToOutputs(ctx, x.CloudBuild, build)
440435
}
441436

437+
// hasToolchain parses the specified go.mod file, and
438+
// reports whether it has a toolchain directive in it.
439+
func hasToolchain(fsys fs.FS, goModPath string) (has bool, _ error) {
440+
b, err := fs.ReadFile(fsys, goModPath)
441+
if err != nil {
442+
return false, err
443+
}
444+
f, err := modfile.Parse(goModPath, b, nil)
445+
if err != nil {
446+
return false, err
447+
}
448+
return f.Toolchain != nil, nil
449+
}
450+
442451
func buildToOutputs(ctx *wf.TaskContext, buildClient CloudBuildClient, build CloudBuild) (map[string]string, error) {
443452
if _, err := AwaitCondition(ctx, 10*time.Second, func() (string, bool, error) {
444453
return buildClient.Completed(ctx, build)

internal/task/tagx_test.go

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,15 @@ case "$1" in
255255
done
256256
;;
257257
"mod")
258-
ls go.mod go.sum >/dev/null
259-
echo "tidied! $*" >> go.mod
258+
case "$2" in
259+
"tidy")
260+
ls go.mod go.sum >/dev/null
261+
echo "tidied! $*" >> go.mod
262+
;;
263+
"edit")
264+
echo "edited! $*" >> go.mod
265+
;;
266+
esac
260267
;;
261268
*)
262269
echo unexpected command $@
@@ -307,10 +314,11 @@ func TestTagXRepos(t *testing.T) {
307314
mod.Tag("v1.0.0", mod1)
308315
tools := NewFakeRepo(t, "tools")
309316
tools1 := tools.Commit(map[string]string{
310-
"go.mod": "module golang.org/x/tools\nrequire golang.org/x/mod v1.0.0\ngo 1.18\nrequire golang.org/x/sys v0.1.0\nrequire golang.org/x/build v0.0.0\n",
311-
"go.sum": "\n",
312-
"gopls/go.mod": "module golang.org/x/tools/gopls\nrequire golang.org/x/mod v1.0.0\n",
313-
"gopls/go.sum": "\n",
317+
"go.mod": "module golang.org/x/tools\nrequire golang.org/x/mod v1.0.0\ngo 1.18\nrequire golang.org/x/sys v0.1.0\nrequire golang.org/x/build v0.0.0\n",
318+
"go.sum": "\n",
319+
"gopls/go.mod": "module golang.org/x/tools/gopls\nrequire golang.org/x/mod v1.0.0\n",
320+
"gopls/go.sum": "\n",
321+
"withtoolchain/go.mod": "module golang.org/x/tools/withtoolchain\ngo 1.23.1\ntoolchain go1.23.2\n",
314322
})
315323
tools.Tag("v1.1.5", tools1)
316324
build := NewFakeRepo(t, "build")
@@ -364,12 +372,22 @@ func TestTagXRepos(t *testing.T) {
364372
if !strings.Contains(string(goMod), "tidied!") {
365373
t.Error("tools go.mod should be tidied")
366374
}
375+
if !strings.Contains(string(goMod), "edited! mod edit -toolchain=none") {
376+
t.Error("tools go.mod should be edited with -toolchain=none")
377+
}
367378
goplsMod, err := deps.gerrit.ReadFile(ctx, "tools", tag.Revision, "gopls/go.mod")
368379
if err != nil {
369380
t.Fatal(err)
370381
}
371-
if !strings.Contains(string(goplsMod), "tidied!") || strings.Contains(string(goplsMod), "upgraded") {
372-
t.Errorf("gopls go.mod should be tidied and not upgraded:\n%s", goplsMod)
382+
if !strings.Contains(string(goplsMod), "tidied!") || !strings.Contains(string(goplsMod), "edited!") || strings.Contains(string(goplsMod), "upgraded") {
383+
t.Errorf("gopls go.mod should be tidied+edited and not upgraded:\n%s", goplsMod)
384+
}
385+
withtoolchainMod, err := deps.gerrit.ReadFile(ctx, "tools", tag.Revision, "withtoolchain/go.mod")
386+
if err != nil {
387+
t.Fatal(err)
388+
}
389+
if !strings.Contains(string(withtoolchainMod), "tidied!") || strings.Contains(string(withtoolchainMod), "edited!") {
390+
t.Errorf("withtoolchain go.mod should be tidied and not edited:\n%s", withtoolchainMod)
373391
}
374392

375393
tags, err = deps.gerrit.ListTags(ctx, "build")

0 commit comments

Comments
 (0)