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/read_test.go b/modfile/read_test.go index 82c778d..efc75e1 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,28 +431,37 @@ 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}, - {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}, + {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 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) { 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 6bcde8f..b4dd799 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`, +// 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. // We ignore all unknown directives as well as main-module-only @@ -364,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") @@ -612,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) @@ -926,7 +968,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 +986,44 @@ 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) + } + 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.Toolchain.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. @@ -1387,13 +1467,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 +1584,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..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", @@ -1224,6 +1355,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 { @@ -1455,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/testdata/goline.golden b/modfile/testdata/goline.golden new file mode 100644 index 0000000..20b9f9c --- /dev/null +++ b/modfile/testdata/goline.golden @@ -0,0 +1,3 @@ +go 1.2.3 + +toolchain default diff --git a/modfile/testdata/goline.in b/modfile/testdata/goline.in new file mode 100644 index 0000000..fdc2c73 --- /dev/null +++ b/modfile/testdata/goline.in @@ -0,0 +1,2 @@ +go 1.2.3 +toolchain default diff --git a/modfile/testdata/work/goline.golden b/modfile/testdata/work/goline.golden new file mode 100644 index 0000000..20b9f9c --- /dev/null +++ b/modfile/testdata/work/goline.golden @@ -0,0 +1,3 @@ +go 1.2.3 + +toolchain default diff --git a/modfile/testdata/work/goline.in b/modfile/testdata/work/goline.in new file mode 100644 index 0000000..fdc2c73 --- /dev/null +++ b/modfile/testdata/work/goline.in @@ -0,0 +1,2 @@ +go 1.2.3 +toolchain default diff --git a/modfile/work.go b/modfile/work.go index 0c0e521..75dc1c5 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,56 @@ 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 +} + +// 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 46115a5..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) { @@ -332,10 +389,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) diff --git a/sumdb/client.go b/sumdb/client.go index 70dd56f..1c1b029 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) @@ -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) } 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/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...) } 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