From e7bea8f1d64fb6752bf603c5c6f39f12f99fc3a3 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Sat, 6 May 2023 15:59:30 -0400 Subject: [PATCH 1/8] modfile: use semantic sort for exclude blocks For golang/go#60028. Change-Id: I4c7a726a900fc7c4b34816eba5cfd0361c45315f Reviewed-on: https://go-review.googlesource.com/c/mod/+/492990 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gopher Robot Auto-Submit: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov Reviewed-by: Bryan Mills --- modfile/rule.go | 26 ++++++++++++++++++++++++- modfile/rule_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/modfile/rule.go b/modfile/rule.go index 6bcde8f..c20aef1 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -1387,13 +1387,21 @@ func (f *File) DropRetract(vi VersionInterval) error { func (f *File) SortBlocks() { f.removeDups() // otherwise sorting is unsafe + // semanticSortForExcludeVersionV is the Go version (plus leading "v") at which + // lines in exclude blocks start to use semantic sort instead of lexicographic sort. + // See go.dev/issue/60028. + const semanticSortForExcludeVersionV = "v1.21" + useSemanticSortForExclude := f.Go != nil && semver.Compare("v"+f.Go.Version, semanticSortForExcludeVersionV) >= 0 + for _, stmt := range f.Syntax.Stmt { block, ok := stmt.(*LineBlock) if !ok { continue } less := lineLess - if block.Token[0] == "retract" { + if block.Token[0] == "exclude" && useSemanticSortForExclude { + less = lineExcludeLess + } else if block.Token[0] == "retract" { less = lineRetractLess } sort.SliceStable(block.Line, func(i, j int) bool { @@ -1496,6 +1504,22 @@ func lineLess(li, lj *Line) bool { return len(li.Token) < len(lj.Token) } +// lineExcludeLess reports whether li should be sorted before lj for lines in +// an "exclude" block. +func lineExcludeLess(li, lj *Line) bool { + if len(li.Token) != 2 || len(lj.Token) != 2 { + // Not a known exclude specification. + // Fall back to sorting lexicographically. + return lineLess(li, lj) + } + // An exclude specification has two tokens: ModulePath and Version. + // Compare module path by string order and version by semver rules. + if pi, pj := li.Token[0], lj.Token[0]; pi != pj { + return pi < pj + } + return semver.Compare(li.Token[1], lj.Token[1]) < 0 +} + // lineRetractLess returns whether li should be sorted before lj for lines in // a "retract" block. It treats each line as a version interval. Single versions // are compared as if they were intervals with the same low and high version. diff --git a/modfile/rule_test.go b/modfile/rule_test.go index 26879fb..f8dd174 100644 --- a/modfile/rule_test.go +++ b/modfile/rule_test.go @@ -1224,6 +1224,52 @@ var sortBlocksTests = []struct { `, false, }, + // Exclude blocks are sorted using semver in ascending order + // in go.mod files that opt in to Go version 1.21 or newer. + { + `sort_exclude_go121_semver`, + `module m + go 1.21 + exclude ( + b.example/m v0.9.0 + a.example/m v1.0.0 + b.example/m v0.10.0 + c.example/m v1.1.0 + b.example/m v0.11.0 + )`, + `module m + go 1.21 + exclude ( + a.example/m v1.0.0 + b.example/m v0.9.0 + b.example/m v0.10.0 + b.example/m v0.11.0 + c.example/m v1.1.0 + ) + `, + true, + }, + { + `sort_exclude_!go121_lexicographically`, // Maintain the previous (less featureful) behavior to avoid unnecessary churn. + `module m + exclude ( + b.example/m v0.9.0 + a.example/m v1.0.0 + b.example/m v0.10.0 + c.example/m v1.1.0 + b.example/m v0.11.0 + )`, + `module m + exclude ( + a.example/m v1.0.0 + b.example/m v0.10.0 + b.example/m v0.11.0 + b.example/m v0.9.0 + c.example/m v1.1.0 + ) + `, + true, + }, } var addRetractValidateVersionTests = []struct { From a73672d4be603f5c3bb0120c69ac4cdde265d008 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 23 May 2023 13:09:04 -0400 Subject: [PATCH 2/8] modfile: add support for go and toolchain lines As part of the forward compatibility work, a new toolchain line is being added, and go lines are allowed to specify toolchain versions like "1.21.0" or "1.21rc1" now. (The lax RE has allowed this for quite some time; what's new here is allowing it in the main module.) For golang/go#57001. Change-Id: I1dc01289381fe080644a7a391b97a65158938f39 Reviewed-on: https://go-review.googlesource.com/c/mod/+/497397 TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Run-TryBot: Russ Cox --- modfile/read_test.go | 10 ++++----- modfile/rule.go | 49 ++++++++++++++++++++++++++++++++++++-------- modfile/work.go | 45 +++++++++++++++++++++++++++++++++++----- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/modfile/read_test.go b/modfile/read_test.go index 82c778d..df4f117 100644 --- a/modfile/read_test.go +++ b/modfile/read_test.go @@ -434,13 +434,13 @@ func TestGoVersion(t *testing.T) { {desc: "empty", input: "module m\ngo \n", ok: false}, {desc: "one", input: "module m\ngo 1\n", ok: false}, {desc: "two", input: "module m\ngo 1.22\n", ok: true}, - {desc: "three", input: "module m\ngo 1.22.333", ok: false}, + {desc: "three", input: "module m\ngo 1.22.333", ok: true}, {desc: "before", input: "module m\ngo v1.2\n", ok: false}, - {desc: "after", input: "module m\ngo 1.2rc1\n", ok: false}, + {desc: "after", input: "module m\ngo 1.2rc1\n", ok: true}, {desc: "space", input: "module m\ngo 1.2 3.4\n", ok: false}, - {desc: "alt1", input: "module m\ngo 1.2.3\n", ok: false, laxOK: true}, - {desc: "alt2", input: "module m\ngo 1.2rc1\n", ok: false, laxOK: true}, - {desc: "alt3", input: "module m\ngo 1.2beta1\n", ok: false, laxOK: true}, + {desc: "alt1", input: "module m\ngo 1.2.3\n", ok: true, laxOK: true}, + {desc: "alt2", input: "module m\ngo 1.2rc1\n", ok: true, laxOK: true}, + {desc: "alt3", input: "module m\ngo 1.2beta1\n", ok: true, laxOK: true}, {desc: "alt4", input: "module m\ngo 1.2.beta1\n", ok: false, laxOK: true}, {desc: "alt1", input: "module m\ngo v1.2.3\n", ok: false, laxOK: true}, {desc: "alt2", input: "module m\ngo v1.2rc1\n", ok: false, laxOK: true}, diff --git a/modfile/rule.go b/modfile/rule.go index c20aef1..3306f6f 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -35,12 +35,13 @@ import ( // A File is the parsed, interpreted form of a go.mod file. type File struct { - Module *Module - Go *Go - Require []*Require - Exclude []*Exclude - Replace []*Replace - Retract []*Retract + Module *Module + Go *Go + Toolchain *Toolchain + Require []*Require + Exclude []*Exclude + Replace []*Replace + Retract []*Retract Syntax *FileSyntax } @@ -58,6 +59,12 @@ type Go struct { Syntax *Line } +// A Toolchain is the toolchain statement. +type Toolchain struct { + Name string // "go1.21rc1" + Syntax *Line +} + // An Exclude is a single exclude statement. type Exclude struct { Mod module.Version @@ -296,9 +303,13 @@ func parseToFile(file string, data []byte, fix VersionFixer, strict bool) (parse return f, nil } -var GoVersionRE = lazyregexp.New(`^([1-9][0-9]*)\.(0|[1-9][0-9]*)$`) +var GoVersionRE = lazyregexp.New(`^([1-9][0-9]*)\.(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))?([a-z]+[0-9]+)?$`) var laxGoVersionRE = lazyregexp.New(`^v?(([1-9][0-9]*)\.(0|[1-9][0-9]*))([^0-9].*)$`) +// Toolchains must be named beginning with `go1` or containing `-go1` as a substring, +// like "go1.20.3" or "gccgo-go1.20.3". As a special case, "local" is also permitted. +var ToolchainRE = lazyregexp.New(`^local$|(^|-)go1`) + func (f *File) add(errs *ErrorList, block *LineBlock, line *Line, verb string, args []string, fix VersionFixer, strict bool) { // If strict is false, this module is a dependency. // We ignore all unknown directives as well as main-module-only @@ -926,7 +937,7 @@ func (f *File) Cleanup() { func (f *File) AddGoStmt(version string) error { if !GoVersionRE.MatchString(version) { - return fmt.Errorf("invalid language version string %q", version) + return fmt.Errorf("invalid language version %q", version) } if f.Go == nil { var hint Expr @@ -944,6 +955,28 @@ func (f *File) AddGoStmt(version string) error { return nil } +func (f *File) AddToolchainStmt(name string) error { + if !ToolchainRE.MatchString(name) { + return fmt.Errorf("invalid toolchain name %q", name) + } + if f.Toolchain == nil { + var hint Expr + if f.Go != nil && f.Go.Syntax != nil { + hint = f.Go.Syntax + } else if f.Module != nil && f.Module.Syntax != nil { + hint = f.Module.Syntax + } + f.Toolchain = &Toolchain{ + Name: name, + Syntax: f.Syntax.addLine(hint, "toolchain", name), + } + } else { + f.Toolchain.Name = name + f.Syntax.updateLine(f.Go.Syntax, "toolchain", name) + } + return nil +} + // AddRequire sets the first require line for path to version vers, // preserving any existing comments for that line and removing all // other lines for path. diff --git a/modfile/work.go b/modfile/work.go index 0c0e521..827a01d 100644 --- a/modfile/work.go +++ b/modfile/work.go @@ -12,9 +12,10 @@ import ( // A WorkFile is the parsed, interpreted form of a go.work file. type WorkFile struct { - Go *Go - Use []*Use - Replace []*Replace + Go *Go + Toolchain *Toolchain + Use []*Use + Replace []*Replace Syntax *FileSyntax } @@ -109,7 +110,7 @@ func (f *WorkFile) Cleanup() { func (f *WorkFile) AddGoStmt(version string) error { if !GoVersionRE.MatchString(version) { - return fmt.Errorf("invalid language version string %q", version) + return fmt.Errorf("invalid language version %q", version) } if f.Go == nil { stmt := &Line{Token: []string{"go", version}} @@ -117,7 +118,7 @@ func (f *WorkFile) AddGoStmt(version string) error { Version: version, Syntax: stmt, } - // Find the first non-comment-only block that's and add + // Find the first non-comment-only block and add // the go statement before it. That will keep file comments at the top. i := 0 for i = 0; i < len(f.Syntax.Stmt); i++ { @@ -133,6 +134,40 @@ func (f *WorkFile) AddGoStmt(version string) error { return nil } +func (f *WorkFile) AddToolchainStmt(name string) error { + if !ToolchainRE.MatchString(name) { + return fmt.Errorf("invalid toolchain name %q", name) + } + if f.Toolchain == nil { + stmt := &Line{Token: []string{"toolchain", name}} + f.Toolchain = &Toolchain{ + Name: name, + Syntax: stmt, + } + // Find the go line and add the toolchain line after it. + // Or else find the first non-comment-only block and add + // the toolchain line before it. That will keep file comments at the top. + i := 0 + for i = 0; i < len(f.Syntax.Stmt); i++ { + if line, ok := f.Syntax.Stmt[i].(*Line); ok && len(line.Token) > 0 && line.Token[0] == "go" { + i++ + goto Found + } + } + for i = 0; i < len(f.Syntax.Stmt); i++ { + if _, ok := f.Syntax.Stmt[i].(*CommentBlock); !ok { + break + } + } + Found: + f.Syntax.Stmt = append(append(f.Syntax.Stmt[:i:i], stmt), f.Syntax.Stmt[i:]...) + } else { + f.Toolchain.Name = name + f.Syntax.updateLine(f.Toolchain.Syntax, "toolchain", name) + } + return nil +} + func (f *WorkFile) AddUse(diskPath, modulePath string) error { need := true for _, d := range f.Use { From 1846133a84c76607dd1d28fd099344fe91e79b56 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 23 May 2023 14:05:04 -0400 Subject: [PATCH 3/8] modfile: add parsing support for toolchain Add new toolchain directive to go.mod and go.work parser. Also fix error checking in parsing tests. For golang/go#57001. Change-Id: Ib7603f82cbd667f2152ed6b0c5989f08c28ceb1c Reviewed-on: https://go-review.googlesource.com/c/mod/+/497399 Run-TryBot: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills --- modfile/read_test.go | 27 +++++++++++++++++++------ modfile/rule.go | 31 +++++++++++++++++++++++++++++ modfile/testdata/goline.golden | 3 +++ modfile/testdata/goline.in | 2 ++ modfile/testdata/work/goline.golden | 3 +++ modfile/testdata/work/goline.in | 2 ++ modfile/work_test.go | 5 +---- 7 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 modfile/testdata/goline.golden create mode 100644 modfile/testdata/goline.in create mode 100644 modfile/testdata/work/goline.golden create mode 100644 modfile/testdata/work/goline.in diff --git a/modfile/read_test.go b/modfile/read_test.go index df4f117..7017ee1 100644 --- a/modfile/read_test.go +++ b/modfile/read_test.go @@ -181,7 +181,13 @@ func TestPrintParse(t *testing.T) { pf1, err := Parse(base, data, nil) if err != nil { switch base { - case "testdata/replace2.in", "testdata/gopkg.in.golden": + case "testdata/block.golden", + "testdata/block.in", + "testdata/comment.golden", + "testdata/comment.in", + "testdata/rule1.golden": + // ignore + default: t.Errorf("should parse %v: %v", base, err) } } @@ -425,12 +431,13 @@ func TestModulePath(t *testing.T) { } } -func TestGoVersion(t *testing.T) { +func TestParseVersions(t *testing.T) { tests := []struct { desc, input string ok bool laxOK bool // ok=true implies laxOK=true; only set if ok=false }{ + // go lines {desc: "empty", input: "module m\ngo \n", ok: false}, {desc: "one", input: "module m\ngo 1\n", ok: false}, {desc: "two", input: "module m\ngo 1.22\n", ok: true}, @@ -438,15 +445,23 @@ func TestGoVersion(t *testing.T) { {desc: "before", input: "module m\ngo v1.2\n", ok: false}, {desc: "after", input: "module m\ngo 1.2rc1\n", ok: true}, {desc: "space", input: "module m\ngo 1.2 3.4\n", ok: false}, - {desc: "alt1", input: "module m\ngo 1.2.3\n", ok: true, laxOK: true}, - {desc: "alt2", input: "module m\ngo 1.2rc1\n", ok: true, laxOK: true}, - {desc: "alt3", input: "module m\ngo 1.2beta1\n", ok: true, laxOK: true}, + {desc: "alt1", input: "module m\ngo 1.2.3\n", ok: true}, + {desc: "alt2", input: "module m\ngo 1.2rc1\n", ok: true}, + {desc: "alt3", input: "module m\ngo 1.2beta1\n", ok: true}, {desc: "alt4", input: "module m\ngo 1.2.beta1\n", ok: false, laxOK: true}, {desc: "alt1", input: "module m\ngo v1.2.3\n", ok: false, laxOK: true}, {desc: "alt2", input: "module m\ngo v1.2rc1\n", ok: false, laxOK: true}, {desc: "alt3", input: "module m\ngo v1.2beta1\n", ok: false, laxOK: true}, {desc: "alt4", input: "module m\ngo v1.2.beta1\n", ok: false, laxOK: true}, {desc: "alt1", input: "module m\ngo v1.2\n", ok: false, laxOK: true}, + + // toolchain lines + {desc: "tool", input: "module m\ntoolchain go1.2\n", ok: true}, + {desc: "tool1", input: "module m\ntoolchain go1.2.3\n", ok: true}, + {desc: "tool2", input: "module m\ntoolchain go1.2rc1\n", ok: true}, + {desc: "tool3", input: "module m\ntoolchain gccgo-go1.2rc1\n", ok: true}, + {desc: "tool4", input: "module m\ntoolchain local\n", ok: true}, + {desc: "tool5", input: "module m\ntoolchain inconceivable!\n", ok: false, laxOK: true}, } t.Run("Strict", func(t *testing.T) { for _, test := range tests { @@ -462,7 +477,7 @@ func TestGoVersion(t *testing.T) { t.Run("Lax", func(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - if _, err := Parse("go.mod", []byte(test.input), nil); err == nil && !(test.ok || test.laxOK) { + if _, err := ParseLax("go.mod", []byte(test.input), nil); err == nil && !(test.ok || test.laxOK) { t.Error("unexpected success") } else if err != nil && test.ok { t.Errorf("unexpected error: %v", err) diff --git a/modfile/rule.go b/modfile/rule.go index 3306f6f..45e7b6a 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -375,6 +375,21 @@ func (f *File) add(errs *ErrorList, block *LineBlock, line *Line, verb string, a f.Go = &Go{Syntax: line} f.Go.Version = args[0] + case "toolchain": + if f.Toolchain != nil { + errorf("repeated toolchain statement") + return + } + if len(args) != 1 { + errorf("toolchain directive expects exactly one argument") + return + } else if strict && !ToolchainRE.MatchString(args[0]) { + errorf("invalid toolchain version '%s': must match format go1.23 or local", args[0]) + return + } + f.Toolchain = &Toolchain{Syntax: line} + f.Toolchain.Name = args[0] + case "module": if f.Module != nil { errorf("repeated module statement") @@ -623,6 +638,22 @@ func (f *WorkFile) add(errs *ErrorList, line *Line, verb string, args []string, f.Go = &Go{Syntax: line} f.Go.Version = args[0] + case "toolchain": + if f.Toolchain != nil { + errorf("repeated toolchain statement") + return + } + if len(args) != 1 { + errorf("toolchain directive expects exactly one argument") + return + } else if !ToolchainRE.MatchString(args[0]) { + errorf("invalid toolchain version '%s': must match format go1.23 or local", args[0]) + return + } + + f.Toolchain = &Toolchain{Syntax: line} + f.Toolchain.Name = args[0] + case "use": if len(args) != 1 { errorf("usage: %s local/dir", verb) diff --git a/modfile/testdata/goline.golden b/modfile/testdata/goline.golden new file mode 100644 index 0000000..1f07989 --- /dev/null +++ b/modfile/testdata/goline.golden @@ -0,0 +1,3 @@ +go 1.2.3 + +toolchain local diff --git a/modfile/testdata/goline.in b/modfile/testdata/goline.in new file mode 100644 index 0000000..498c1b8 --- /dev/null +++ b/modfile/testdata/goline.in @@ -0,0 +1,2 @@ +go 1.2.3 +toolchain local diff --git a/modfile/testdata/work/goline.golden b/modfile/testdata/work/goline.golden new file mode 100644 index 0000000..1f07989 --- /dev/null +++ b/modfile/testdata/work/goline.golden @@ -0,0 +1,3 @@ +go 1.2.3 + +toolchain local diff --git a/modfile/testdata/work/goline.in b/modfile/testdata/work/goline.in new file mode 100644 index 0000000..498c1b8 --- /dev/null +++ b/modfile/testdata/work/goline.in @@ -0,0 +1,2 @@ +go 1.2.3 +toolchain local diff --git a/modfile/work_test.go b/modfile/work_test.go index 46115a5..096ed5c 100644 --- a/modfile/work_test.go +++ b/modfile/work_test.go @@ -332,10 +332,7 @@ func TestWorkPrintParse(t *testing.T) { pf1, err := ParseWork(base, data, nil) if err != nil { - switch base { - case "testdata/replace2.in", "testdata/gopkg.in.golden": - t.Errorf("should parse %v: %v", base, err) - } + t.Errorf("should parse %v: %v", base, err) } if err == nil { pf2, err := ParseWork(base, ndata, nil) From fc83a8faf9931f09514a196d1b7ff229c8efedc6 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 23 May 2023 14:59:55 -0400 Subject: [PATCH 4/8] modfile: add support for dropping go and toolchain stmts Also add tests of previous CLs. For golang/go#57001. Change-Id: I755429dd07c0e84910108ce9807d607115329b79 Reviewed-on: https://go-review.googlesource.com/c/mod/+/497400 Run-TryBot: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills --- modfile/print.go | 14 +++- modfile/rule.go | 18 ++++- modfile/rule_test.go | 169 ++++++++++++++++++++++++++++++++++++++++++- modfile/work.go | 16 ++++ modfile/work_test.go | 57 +++++++++++++++ 5 files changed, 268 insertions(+), 6 deletions(-) diff --git a/modfile/print.go b/modfile/print.go index 524f930..2a0123d 100644 --- a/modfile/print.go +++ b/modfile/print.go @@ -16,7 +16,13 @@ import ( func Format(f *FileSyntax) []byte { pr := &printer{} pr.file(f) - return pr.Bytes() + + // remove trailing blank lines + b := pr.Bytes() + for len(b) > 0 && b[len(b)-1] == '\n' && (len(b) == 1 || b[len(b)-2] == '\n') { + b = b[:len(b)-1] + } + return b } // A printer collects the state during printing of a file or expression. @@ -59,7 +65,11 @@ func (p *printer) newline() { } p.trim() - p.printf("\n") + if b := p.Bytes(); len(b) == 0 || (len(b) >= 2 && b[len(b)-1] == '\n' && b[len(b)-2] == '\n') { + // skip the blank line at top of file or after a blank line + } else { + p.printf("\n") + } for i := 0; i < p.margin; i++ { p.printf("\t") } diff --git a/modfile/rule.go b/modfile/rule.go index 45e7b6a..39f03f2 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -986,6 +986,22 @@ func (f *File) AddGoStmt(version string) error { return nil } +// DropGoStmt deletes the go statement from the file. +func (f *File) DropGoStmt() { + if f.Go != nil { + f.Go.Syntax.markRemoved() + f.Go = nil + } +} + +// DropToolchainStmt deletes the toolchain statement from the file. +func (f *File) DropToolchainStmt() { + if f.Toolchain != nil { + f.Toolchain.Syntax.markRemoved() + f.Toolchain = nil + } +} + func (f *File) AddToolchainStmt(name string) error { if !ToolchainRE.MatchString(name) { return fmt.Errorf("invalid toolchain name %q", name) @@ -1003,7 +1019,7 @@ func (f *File) AddToolchainStmt(name string) error { } } else { f.Toolchain.Name = name - f.Syntax.updateLine(f.Go.Syntax, "toolchain", name) + f.Syntax.updateLine(f.Toolchain.Syntax, "toolchain", name) } return nil } diff --git a/modfile/rule_test.go b/modfile/rule_test.go index f8dd174..57c8be6 100644 --- a/modfile/rule_test.go +++ b/modfile/rule_test.go @@ -696,6 +696,137 @@ var addGoTests = []struct { }, } +var dropGoTests = []struct { + desc string + in string + out string +}{ + { + `module_only`, + `module m + go 1.14 + `, + `module m + `, + }, + { + `module_before_require`, + `module m + go 1.14 + require x.y/a v1.2.3 + `, + `module m + require x.y/a v1.2.3 + `, + }, + { + `require_before_module`, + `require x.y/a v1.2.3 + module example.com/inverted + go 1.14 + `, + `require x.y/a v1.2.3 + module example.com/inverted + `, + }, + { + `require_only`, + `require x.y/a v1.2.3 + go 1.14 + `, + `require x.y/a v1.2.3 + `, + }, +} + +var addToolchainTests = []struct { + desc string + in string + version string + out string +}{ + { + `empty`, + ``, + `go1.17`, + `toolchain go1.17 + `, + }, + { + `aftergo`, + `// this is a comment + require x v1.0.0 + + go 1.17 + + require y v1.0.0 + `, + `go1.17`, + `// this is a comment + require x v1.0.0 + + go 1.17 + + toolchain go1.17 + + require y v1.0.0 + `, + }, + { + `already_have_toolchain`, + `go 1.17 + + toolchain go1.18 + `, + `go1.19`, + `go 1.17 + + toolchain go1.19 + `, + }, +} + +var dropToolchainTests = []struct { + desc string + in string + out string +}{ + { + `empty`, + `toolchain go1.17 + `, + ``, + }, + { + `aftergo`, + `// this is a comment + require x v1.0.0 + + go 1.17 + + toolchain go1.17 + + require y v1.0.0 + `, + `// this is a comment + require x v1.0.0 + + go 1.17 + + require y v1.0.0 + `, + }, + { + `already_have_toolchain`, + `go 1.17 + + toolchain go1.18 + `, + `go 1.17 + `, + }, +} + var addExcludeTests = []struct { desc string in string @@ -948,7 +1079,7 @@ var retractRationaleTests = []struct { `prefix_one`, `module m // prefix - retract v1.0.0 + retract v1.0.0 `, `prefix`, }, @@ -959,7 +1090,7 @@ var retractRationaleTests = []struct { // // two // - // three + // three retract v1.0.0`, `one @@ -1083,7 +1214,7 @@ var moduleDeprecatedTests = []struct { { `deprecated_paragraph_space`, `// Deprecated: the next line has a space - // + // // c module m`, "the next line has a space", @@ -1501,6 +1632,38 @@ func TestAddGo(t *testing.T) { } } +func TestDropGo(t *testing.T) { + for _, tt := range dropGoTests { + t.Run(tt.desc, func(t *testing.T) { + testEdit(t, tt.in, tt.out, true, func(f *File) error { + f.DropGoStmt() + return nil + }) + }) + } +} + +func TestAddToolchain(t *testing.T) { + for _, tt := range addToolchainTests { + t.Run(tt.desc, func(t *testing.T) { + testEdit(t, tt.in, tt.out, true, func(f *File) error { + return f.AddToolchainStmt(tt.version) + }) + }) + } +} + +func TestDropToolchain(t *testing.T) { + for _, tt := range dropToolchainTests { + t.Run(tt.desc, func(t *testing.T) { + testEdit(t, tt.in, tt.out, true, func(f *File) error { + f.DropToolchainStmt() + return nil + }) + }) + } +} + func TestAddExclude(t *testing.T) { for _, tt := range addExcludeTests { t.Run(tt.desc, func(t *testing.T) { diff --git a/modfile/work.go b/modfile/work.go index 827a01d..75dc1c5 100644 --- a/modfile/work.go +++ b/modfile/work.go @@ -168,6 +168,22 @@ func (f *WorkFile) AddToolchainStmt(name string) error { return nil } +// DropGoStmt deletes the go statement from the file. +func (f *WorkFile) DropGoStmt() { + if f.Go != nil { + f.Go.Syntax.markRemoved() + f.Go = nil + } +} + +// DropToolchainStmt deletes the toolchain statement from the file. +func (f *WorkFile) DropToolchainStmt() { + if f.Toolchain != nil { + f.Toolchain.Syntax.markRemoved() + f.Toolchain = nil + } +} + func (f *WorkFile) AddUse(diskPath, modulePath string) error { need := true for _, d := range f.Use { diff --git a/modfile/work_test.go b/modfile/work_test.go index 096ed5c..dcc0810 100644 --- a/modfile/work_test.go +++ b/modfile/work_test.go @@ -212,6 +212,53 @@ var workAddGoTests = []struct { }, } +var workAddToolchainTests = []struct { + desc string + in string + version string + out string +}{ + { + `empty`, + ``, + `go1.17`, + `toolchain go1.17 + `, + }, + { + `aftergo`, + `// this is a comment + use foo + + go 1.17 + + use bar + `, + `go1.17`, + `// this is a comment + use foo + + go 1.17 + + toolchain go1.17 + + use bar + `, + }, + { + `already_have_toolchain`, + `go 1.17 + + toolchain go1.18 + `, + `go1.19`, + `go 1.17 + + toolchain go1.19 + `, + }, +} + var workSortBlocksTests = []struct { desc, in, out string }{ @@ -284,6 +331,16 @@ func TestWorkAddGo(t *testing.T) { } } +func TestWorkAddToolchain(t *testing.T) { + for _, tt := range workAddToolchainTests { + t.Run(tt.desc, func(t *testing.T) { + testWorkEdit(t, tt.in, tt.out, func(f *WorkFile) error { + return f.AddToolchainStmt(tt.version) + }) + }) + } +} + func TestWorkSortBlocks(t *testing.T) { for _, tt := range workSortBlocksTests { t.Run(tt.desc, func(t *testing.T) { From e343115d42363d625a8424d854065a547fe86f5c Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Fri, 2 Jun 2023 21:06:51 +0300 Subject: [PATCH 5/8] sumdb: add missing return after http.Error Change-Id: Ic0a9149713f7981e44629b2d96cf2a6ec52d6976 Reviewed-on: https://go-review.googlesource.com/c/mod/+/500455 Reviewed-by: Bryan Mills Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Reviewed-by: Michael Knyszek --- sumdb/server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sumdb/server.go b/sumdb/server.go index 2e523a5..899bd46 100644 --- a/sumdb/server.go +++ b/sumdb/server.go @@ -148,6 +148,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { msg, err := tlog.FormatRecord(start+int64(i), text) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) + return } data = append(data, msg...) } From e81ad1007af3dfbbc0761df9e1a7ace49b7fd7d2 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Fri, 2 Jun 2023 21:12:43 +0300 Subject: [PATCH 6/8] sumdb: correct spelling mistakes Change-Id: Idb3276b201dbce094201fc5824a1e151f4f71ce8 Reviewed-on: https://go-review.googlesource.com/c/mod/+/500456 Reviewed-by: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot Auto-Submit: Dmitri Shuralyov Run-TryBot: Dmitri Shuralyov Reviewed-by: Bryan Mills --- sumdb/client.go | 2 +- sumdb/note/note.go | 2 +- sumdb/storage/storage.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sumdb/client.go b/sumdb/client.go index 70dd56f..965a7e2 100644 --- a/sumdb/client.go +++ b/sumdb/client.go @@ -109,7 +109,7 @@ func NewClient(ops ClientOps) *Client { } } -// init initiailzes the client (if not already initialized) +// init initializes the client (if not already initialized) // and returns any initialization error. func (c *Client) init() error { c.initOnce.Do(c.initWork) diff --git a/sumdb/note/note.go b/sumdb/note/note.go index 140b937..8c22b19 100644 --- a/sumdb/note/note.go +++ b/sumdb/note/note.go @@ -124,7 +124,7 @@ // not contain spaces or newlines). // // If Open is given access to a Verifiers including the -// Verifier for this key, then it will succeed at verifiying +// Verifier for this key, then it will succeed at verifying // the encoded message and returning the parsed Note: // // vkey := "PeterNeumann+c74f20a3+ARpc2QcUPDhMQegwxbzhKqiBfsVkmqq/LDE4izWy10TW" diff --git a/sumdb/storage/storage.go b/sumdb/storage/storage.go index 3c80ac2..1806380 100644 --- a/sumdb/storage/storage.go +++ b/sumdb/storage/storage.go @@ -18,7 +18,7 @@ type Storage interface { // ReadWrite runs f in a read-write transaction. // If f returns an error, the transaction aborts and returns that error. - // If f returns nil, the transaction attempts to commit and then then return nil. + // If f returns nil, the transaction attempts to commit and then return nil. // Otherwise it tries again. Note that f may be called multiple times and that // the result only describes the effect of the final call to f. // The caller must take care not to use any state computed during From 2a1c06a60797e734ab13afef967c60867445e1e5 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 4 Jun 2023 22:14:51 -0400 Subject: [PATCH 7/8] modfile: update toolchain syntax Final revisions for Go 1.21: lock down the toolchain syntax to use 'toolchain default' instead of 'toolchain local', to avoid confusion with 'GOTOOLCHAIN=local'; 'toolchain local' does not always mean the same thing. Also remove the prefix-go1.2.3 toolchain name form, leaving only go1.2.3-suffix. There is no need to have two different forms, and it's confusing. For golang/go#57001. Change-Id: I16623c796c620e98178deed899e28a00e85fcc21 Reviewed-on: https://go-review.googlesource.com/c/mod/+/500755 TryBot-Result: Gopher Robot Run-TryBot: Russ Cox Reviewed-by: Michael Matloob --- modfile/read_test.go | 4 ++-- modfile/rule.go | 6 +++--- modfile/testdata/goline.golden | 2 +- modfile/testdata/goline.in | 2 +- modfile/testdata/work/goline.golden | 2 +- modfile/testdata/work/goline.in | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/modfile/read_test.go b/modfile/read_test.go index 7017ee1..efc75e1 100644 --- a/modfile/read_test.go +++ b/modfile/read_test.go @@ -459,8 +459,8 @@ func TestParseVersions(t *testing.T) { {desc: "tool", input: "module m\ntoolchain go1.2\n", ok: true}, {desc: "tool1", input: "module m\ntoolchain go1.2.3\n", ok: true}, {desc: "tool2", input: "module m\ntoolchain go1.2rc1\n", ok: true}, - {desc: "tool3", input: "module m\ntoolchain gccgo-go1.2rc1\n", ok: true}, - {desc: "tool4", input: "module m\ntoolchain local\n", ok: true}, + {desc: "tool3", input: "module m\ntoolchain go1.2rc1-gccgo\n", ok: true}, + {desc: "tool4", input: "module m\ntoolchain default\n", ok: true}, {desc: "tool5", input: "module m\ntoolchain inconceivable!\n", ok: false, laxOK: true}, } t.Run("Strict", func(t *testing.T) { diff --git a/modfile/rule.go b/modfile/rule.go index 39f03f2..b4dd799 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -306,9 +306,9 @@ func parseToFile(file string, data []byte, fix VersionFixer, strict bool) (parse var GoVersionRE = lazyregexp.New(`^([1-9][0-9]*)\.(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))?([a-z]+[0-9]+)?$`) var laxGoVersionRE = lazyregexp.New(`^v?(([1-9][0-9]*)\.(0|[1-9][0-9]*))([^0-9].*)$`) -// Toolchains must be named beginning with `go1` or containing `-go1` as a substring, -// like "go1.20.3" or "gccgo-go1.20.3". As a special case, "local" is also permitted. -var ToolchainRE = lazyregexp.New(`^local$|(^|-)go1`) +// Toolchains must be named beginning with `go1`, +// like "go1.20.3" or "go1.20.3-gccgo". As a special case, "default" is also permitted. +var ToolchainRE = lazyregexp.New(`^default$|^go1($|\.)`) func (f *File) add(errs *ErrorList, block *LineBlock, line *Line, verb string, args []string, fix VersionFixer, strict bool) { // If strict is false, this module is a dependency. diff --git a/modfile/testdata/goline.golden b/modfile/testdata/goline.golden index 1f07989..20b9f9c 100644 --- a/modfile/testdata/goline.golden +++ b/modfile/testdata/goline.golden @@ -1,3 +1,3 @@ go 1.2.3 -toolchain local +toolchain default diff --git a/modfile/testdata/goline.in b/modfile/testdata/goline.in index 498c1b8..fdc2c73 100644 --- a/modfile/testdata/goline.in +++ b/modfile/testdata/goline.in @@ -1,2 +1,2 @@ go 1.2.3 -toolchain local +toolchain default diff --git a/modfile/testdata/work/goline.golden b/modfile/testdata/work/goline.golden index 1f07989..20b9f9c 100644 --- a/modfile/testdata/work/goline.golden +++ b/modfile/testdata/work/goline.golden @@ -1,3 +1,3 @@ go 1.2.3 -toolchain local +toolchain default diff --git a/modfile/testdata/work/goline.in b/modfile/testdata/work/goline.in index 498c1b8..fdc2c73 100644 --- a/modfile/testdata/work/goline.in +++ b/modfile/testdata/work/goline.in @@ -1,2 +1,2 @@ go 1.2.3 -toolchain local +toolchain default From 62c7e578f1a7275d934c99dd48715525bd52b17e Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 5 Jun 2023 16:24:29 -0400 Subject: [PATCH 8/8] sumdb: handle panic from c.ReadRemote during ReadTiles The go command tests use a network stack that sometimes panics during read. This code was assuming a network stack that returns errors instead. If a tile-reading goroutine panicked, ReadTiles considered it done with data, err = nil, nil, and then there was a race between ReadTiles failing with an error about a suspiciously short tile and the panicking goroutine getting to run and bring down the entire program. Remove the race by converting the panic into an error during ReadTiles. For a test in golang/go#57001. Change-Id: I9b18a244238e67c27a15b93f8397bf3ab44b06e6 Reviewed-on: https://go-review.googlesource.com/c/mod/+/501035 Run-TryBot: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Michael Matloob --- sumdb/client.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sumdb/client.go b/sumdb/client.go index 965a7e2..1c1b029 100644 --- a/sumdb/client.go +++ b/sumdb/client.go @@ -553,6 +553,11 @@ func (r *tileReader) ReadTiles(tiles []tlog.Tile) ([][]byte, error) { wg.Add(1) go func(i int, tile tlog.Tile) { defer wg.Done() + defer func() { + if e := recover(); e != nil { + errs[i] = fmt.Errorf("panic: %v", e) + } + }() data[i], errs[i] = r.c.readTile(tile) }(i, tile) }