From ae88a97cb46b420618fb1d363fbe3343fc4e4613 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Wed, 6 Nov 2024 03:20:22 +0800 Subject: [PATCH 1/2] all: change from sort functions to slices functions The sorting functions in the slices package are slightly faster as they don't use reflection internally. Change-Id: I3c0ab61336d44c1928ee2a9da12a00d914f5e637 Reviewed-on: https://go-review.googlesource.com/c/mod/+/625635 Reviewed-by: Dmitri Shuralyov Reviewed-by: Michael Matloob Reviewed-by: Michael Matloob LUCI-TryBot-Result: Go LUCI --- modfile/rule.go | 48 +++++++++++++++++++++---------------------- modfile/work.go | 6 ++---- module/module.go | 19 ++++++++--------- semver/semver.go | 30 ++++++++++++++++----------- semver/semver_test.go | 39 +++++++++++++++++++++++++++++++++++ sumdb/dirhash/hash.go | 6 +++--- 6 files changed, 94 insertions(+), 54 deletions(-) diff --git a/modfile/rule.go b/modfile/rule.go index 3e4a1d0..1a6f3ef 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -20,10 +20,11 @@ package modfile import ( + "cmp" "errors" "fmt" "path/filepath" - "sort" + "slices" "strconv" "strings" "unicode" @@ -1633,15 +1634,13 @@ func (f *File) SortBlocks() { if !ok { continue } - less := lineLess + less := compareLine if block.Token[0] == "exclude" && useSemanticSortForExclude { - less = lineExcludeLess + less = compareLineExclude } else if block.Token[0] == "retract" { - less = lineRetractLess + less = compareLineRetract } - sort.SliceStable(block.Line, func(i, j int) bool { - return less(block.Line[i], block.Line[j]) - }) + slices.SortStableFunc(block.Line, less) } } @@ -1746,39 +1745,38 @@ func removeDups(syntax *FileSyntax, exclude *[]*Exclude, replace *[]*Replace, to syntax.Stmt = stmts } -// lineLess returns whether li should be sorted before lj. It sorts -// lexicographically without assigning any special meaning to tokens. -func lineLess(li, lj *Line) bool { +// compareLine compares li and lj. It sorts lexicographically without assigning +// any special meaning to tokens. +func compareLine(li, lj *Line) int { for k := 0; k < len(li.Token) && k < len(lj.Token); k++ { if li.Token[k] != lj.Token[k] { - return li.Token[k] < lj.Token[k] + return cmp.Compare(li.Token[k], lj.Token[k]) } } - return len(li.Token) < len(lj.Token) + return cmp.Compare(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 { +// compareLineExclude compares li and lj for lines in an "exclude" block. +func compareLineExclude(li, lj *Line) int { if len(li.Token) != 2 || len(lj.Token) != 2 { // Not a known exclude specification. // Fall back to sorting lexicographically. - return lineLess(li, lj) + return compareLine(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 cmp.Compare(pi, pj) } - return semver.Compare(li.Token[1], lj.Token[1]) < 0 + return semver.Compare(li.Token[1], lj.Token[1]) } -// 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. +// compareLineRetract compares li and 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. // Intervals are sorted in descending order, first by low version, then by -// high version, using semver.Compare. -func lineRetractLess(li, lj *Line) bool { +// high version, using [semver.Compare]. +func compareLineRetract(li, lj *Line) int { interval := func(l *Line) VersionInterval { if len(l.Token) == 1 { return VersionInterval{Low: l.Token[0], High: l.Token[0]} @@ -1792,9 +1790,9 @@ func lineRetractLess(li, lj *Line) bool { vii := interval(li) vij := interval(lj) if cmp := semver.Compare(vii.Low, vij.Low); cmp != 0 { - return cmp > 0 + return -cmp } - return semver.Compare(vii.High, vij.High) > 0 + return -semver.Compare(vii.High, vij.High) } // checkCanonicalVersion returns a non-nil error if vers is not a canonical diff --git a/modfile/work.go b/modfile/work.go index 5387d0c..6f92644 100644 --- a/modfile/work.go +++ b/modfile/work.go @@ -6,7 +6,7 @@ package modfile import ( "fmt" - "sort" + "slices" "strings" ) @@ -315,9 +315,7 @@ func (f *WorkFile) SortBlocks() { if !ok { continue } - sort.SliceStable(block.Line, func(i, j int) bool { - return lineLess(block.Line[i], block.Line[j]) - }) + slices.SortStableFunc(block.Line, compareLine) } } diff --git a/module/module.go b/module/module.go index 2a364b2..16e1aa7 100644 --- a/module/module.go +++ b/module/module.go @@ -96,10 +96,11 @@ package module // Changes to the semantics in this file require approval from rsc. import ( + "cmp" "errors" "fmt" "path" - "sort" + "slices" "strings" "unicode" "unicode/utf8" @@ -657,17 +658,15 @@ func CanonicalVersion(v string) string { // optionally followed by a tie-breaking suffix introduced by a slash character, // like in "v0.0.1/go.mod". func Sort(list []Version) { - sort.Slice(list, func(i, j int) bool { - mi := list[i] - mj := list[j] - if mi.Path != mj.Path { - return mi.Path < mj.Path + slices.SortFunc(list, func(i, j Version) int { + if i.Path != j.Path { + return strings.Compare(i.Path, j.Path) } // To help go.sum formatting, allow version/file. // Compare semver prefix by semver rules, // file by string order. - vi := mi.Version - vj := mj.Version + vi := i.Version + vj := j.Version var fi, fj string if k := strings.Index(vi, "/"); k >= 0 { vi, fi = vi[:k], vi[k:] @@ -676,9 +675,9 @@ func Sort(list []Version) { vj, fj = vj[:k], vj[k:] } if vi != vj { - return semver.Compare(vi, vj) < 0 + return semver.Compare(vi, vj) } - return fi < fj + return cmp.Compare(fi, fj) }) } diff --git a/semver/semver.go b/semver/semver.go index 9a2dfd3..628f8fd 100644 --- a/semver/semver.go +++ b/semver/semver.go @@ -22,7 +22,10 @@ // as shorthands for vMAJOR.0.0 and vMAJOR.MINOR.0. package semver -import "sort" +import ( + "slices" + "strings" +) // parsed returns the parsed form of a semantic version string. type parsed struct { @@ -154,19 +157,22 @@ func Max(v, w string) string { // ByVersion implements [sort.Interface] for sorting semantic version strings. type ByVersion []string -func (vs ByVersion) Len() int { return len(vs) } -func (vs ByVersion) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] } -func (vs ByVersion) Less(i, j int) bool { - cmp := Compare(vs[i], vs[j]) - if cmp != 0 { - return cmp < 0 - } - return vs[i] < vs[j] -} +func (vs ByVersion) Len() int { return len(vs) } +func (vs ByVersion) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] } +func (vs ByVersion) Less(i, j int) bool { return compareVersion(vs[i], vs[j]) < 0 } -// Sort sorts a list of semantic version strings using [ByVersion]. +// Sort sorts a list of semantic version strings using [Compare] and falls back +// to use [strings.Compare] if both versions are considered equal. func Sort(list []string) { - sort.Sort(ByVersion(list)) + slices.SortFunc(list, compareVersion) +} + +func compareVersion(a, b string) int { + cmp := Compare(a, b) + if cmp != 0 { + return cmp + } + return strings.Compare(a, b) } func parse(v string) (p parsed, ok bool) { diff --git a/semver/semver_test.go b/semver/semver_test.go index 937e18e..56739ad 100644 --- a/semver/semver_test.go +++ b/semver/semver_test.go @@ -6,6 +6,7 @@ package semver import ( "math/rand" + "slices" "sort" "strings" "testing" @@ -166,6 +167,44 @@ func TestSort(t *testing.T) { if !sort.IsSorted(ByVersion(versions)) { t.Errorf("list is not sorted:\n%s", strings.Join(versions, "\n")) } + + golden := []string{ + "bad", + "v1+meta", + "v1-alpha.beta.gamma", + "v1-pre", + "v1-pre+meta", + "v1.2+meta", + "v1.2-pre", + "v1.2-pre+meta", + "v1.0.0-alpha", + "v1.0.0-alpha.1", + "v1.0.0-alpha.beta", + "v1.0.0-beta", + "v1.0.0-beta.2", + "v1.0.0-beta.11", + "v1.0.0-rc.1", + "v1", + "v1.0", + "v1.0.0", + "v1.2", + "v1.2.0", + "v1.2.3-456", + "v1.2.3-456.789", + "v1.2.3-456-789", + "v1.2.3-456a", + "v1.2.3-pre", + "v1.2.3-pre+meta", + "v1.2.3-pre.1", + "v1.2.3-zzz", + "v1.2.3", + "v1.2.3+meta", + "v1.2.3+meta-pre", + "v1.2.3+meta-pre.sha.256a", + } + if !slices.Equal(versions, golden) { + t.Errorf("list is not sorted correctly\ngot:\n%v\nwant:\n%v", versions, golden) + } } func TestMax(t *testing.T) { diff --git a/sumdb/dirhash/hash.go b/sumdb/dirhash/hash.go index 51ec4db..117985a 100644 --- a/sumdb/dirhash/hash.go +++ b/sumdb/dirhash/hash.go @@ -16,7 +16,7 @@ import ( "io" "os" "path/filepath" - "sort" + "slices" "strings" ) @@ -36,7 +36,7 @@ type Hash func(files []string, open func(string) (io.ReadCloser, error)) (string // sha256sum $(find . -type f | sort) | sha256sum // // More precisely, the hashed summary contains a single line for each file in the list, -// ordered by sort.Strings applied to the file names, where each line consists of +// ordered by [slices.Sort] applied to the file names, where each line consists of // the hexadecimal SHA-256 hash of the file content, // two spaces (U+0020), the file name, and a newline (U+000A). // @@ -44,7 +44,7 @@ type Hash func(files []string, open func(string) (io.ReadCloser, error)) (string func Hash1(files []string, open func(string) (io.ReadCloser, error)) (string, error) { h := sha256.New() files = append([]string(nil), files...) - sort.Strings(files) + slices.Sort(files) for _, file := range files { if strings.Contains(file, "\n") { return "", errors.New("dirhash: filenames with newlines are not supported") From 9d3333156f465c85f68264344b5c08fbcf5fcacb Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Wed, 7 May 2025 20:07:28 +0000 Subject: [PATCH 2/2] x/mod: add the ignore directive This CL adds the ignore directive that will be used to support the global ignore mechanism in http://go.dev/cl/643355 For golang/go#42965 Change-Id: I6d0b25de1b4d26185298f6e3aea5ba66651256cb Reviewed-on: https://go-review.googlesource.com/c/mod/+/670656 Reviewed-by: Michael Matloob LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Matloob --- modfile/rule.go | 78 ++++++++++++++++++++++++-- modfile/rule_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++ modfile/work.go | 2 +- 3 files changed, 203 insertions(+), 5 deletions(-) diff --git a/modfile/rule.go b/modfile/rule.go index 1a6f3ef..a86ee4f 100644 --- a/modfile/rule.go +++ b/modfile/rule.go @@ -45,6 +45,7 @@ type File struct { Replace []*Replace Retract []*Retract Tool []*Tool + Ignore []*Ignore Syntax *FileSyntax } @@ -101,6 +102,12 @@ type Tool struct { Syntax *Line } +// An Ignore is a single ignore statement. +type Ignore struct { + Path string + Syntax *Line +} + // A VersionInterval represents a range of versions with upper and lower bounds. // Intervals are closed: both bounds are included. When Low is equal to High, // the interval may refer to a single version ('v1.2.3') or an interval @@ -305,7 +312,7 @@ func parseToFile(file string, data []byte, fix VersionFixer, strict bool) (parse }) } continue - case "module", "godebug", "require", "exclude", "replace", "retract", "tool": + case "module", "godebug", "require", "exclude", "replace", "retract", "tool", "ignore": for _, l := range x.Line { f.add(&errs, x, l, x.Token[0], l.Token, fix, strict) } @@ -338,7 +345,7 @@ func (f *File) add(errs *ErrorList, block *LineBlock, line *Line, verb string, a // and simply ignore those statements. if !strict { switch verb { - case "go", "module", "retract", "require": + case "go", "module", "retract", "require", "ignore": // want these even for dependency go.mods default: return @@ -532,6 +539,21 @@ func (f *File) add(errs *ErrorList, block *LineBlock, line *Line, verb string, a Path: s, Syntax: line, }) + + case "ignore": + if len(args) != 1 { + errorf("ignore directive expects exactly one argument") + return + } + s, err := parseString(&args[0]) + if err != nil { + errorf("invalid quoted string: %v", err) + return + } + f.Ignore = append(f.Ignore, &Ignore{ + Path: s, + Syntax: line, + }) } } @@ -1620,6 +1642,36 @@ func (f *File) DropTool(path string) error { return nil } +// AddIgnore adds a new ignore directive with the given path. +// It does nothing if the ignore line already exists. +func (f *File) AddIgnore(path string) error { + for _, t := range f.Ignore { + if t.Path == path { + return nil + } + } + + f.Ignore = append(f.Ignore, &Ignore{ + Path: path, + Syntax: f.Syntax.addLine(nil, "ignore", path), + }) + + f.SortBlocks() + return nil +} + +// DropIgnore removes a ignore directive with the given path. +// It does nothing if no such ignore directive exists. +func (f *File) DropIgnore(path string) error { + for _, t := range f.Ignore { + if t.Path == path { + t.Syntax.markRemoved() + *t = Ignore{} + } + } + return nil +} + func (f *File) SortBlocks() { f.removeDups() // otherwise sorting is unsafe @@ -1656,10 +1708,10 @@ func (f *File) SortBlocks() { // retract directives are not de-duplicated since comments are // meaningful, and versions may be retracted multiple times. func (f *File) removeDups() { - removeDups(f.Syntax, &f.Exclude, &f.Replace, &f.Tool) + removeDups(f.Syntax, &f.Exclude, &f.Replace, &f.Tool, &f.Ignore) } -func removeDups(syntax *FileSyntax, exclude *[]*Exclude, replace *[]*Replace, tool *[]*Tool) { +func removeDups(syntax *FileSyntax, exclude *[]*Exclude, replace *[]*Replace, tool *[]*Tool, ignore *[]*Ignore) { kill := make(map[*Line]bool) // Remove duplicate excludes. @@ -1718,6 +1770,24 @@ func removeDups(syntax *FileSyntax, exclude *[]*Exclude, replace *[]*Replace, to *tool = newTool } + if ignore != nil { + haveIgnore := make(map[string]bool) + for _, i := range *ignore { + if haveIgnore[i.Path] { + kill[i.Syntax] = true + continue + } + haveIgnore[i.Path] = true + } + var newIgnore []*Ignore + for _, i := range *ignore { + if !kill[i.Syntax] { + newIgnore = append(newIgnore, i) + } + } + *ignore = newIgnore + } + // Duplicate require and retract directives are not removed. // Drop killed statements from the syntax tree. diff --git a/modfile/rule_test.go b/modfile/rule_test.go index c75a77a..5ee8e3e 100644 --- a/modfile/rule_test.go +++ b/modfile/rule_test.go @@ -1780,6 +1780,72 @@ var dropToolTests = []struct { }, } +var addIgnoreTests = []struct { + desc, in, path, want string +}{ + { + `add_first`, + `module example.com/m`, + `foo/bar`, + `module example.com/m + ignore foo/bar`, + }, + { + `sorted_correctly`, + `module example.com/m + ignore foo/bar + `, + `./foo/bar/path`, + `module example.com/m + ignore ( + ./foo/bar/path + foo/bar + )`, + }, + { + `duplicates_ignored`, + `module example.com/m + ignore foo/bar + `, + `foo/bar`, + `module example.com/m + ignore foo/bar`, + }, +} + +var dropIgnoreTests = []struct { + desc, in, path, want string +}{ + { + `only`, + `module example.com/m + ignore foo/bar`, + `foo/bar`, + `module example.com/m`, + }, + { + `parenthesized`, + `module example.com/m + ignore ( + ./foo/bar + foo/bar + )`, + `foo/bar`, + `module example.com/m + ignore ./foo/bar`, + }, + { + `missing`, + `module example.com/m + ignore ( + foo/bar + )`, + `./foo/bar`, + `module example.com/m + ignore foo/bar`, + }, +} + func fixV(path, version string) (string, error) { if path != "example.com/m" { return "", fmt.Errorf("module path must be example.com/m") @@ -2190,3 +2256,65 @@ func TestDropTool(t *testing.T) { }) } } + +func TestAddIgnore(t *testing.T) { + for _, tt := range addIgnoreTests { + t.Run(tt.desc, func(t *testing.T) { + inFile, err := Parse("in", []byte(tt.in), nil) + if err != nil { + t.Fatal(err) + } + if err := inFile.AddIgnore(tt.path); err != nil { + t.Fatal(err) + } + inFile.Cleanup() + got, err := inFile.Format() + if err != nil { + t.Fatal(err) + } + + outFile, err := Parse("out", []byte(tt.want), nil) + if err != nil { + t.Fatal(err) + } + want, err := outFile.Format() + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, want) { + t.Fatalf("got:\n%s\nwant:\n%s", got, want) + } + }) + } +} + +func TestDropIgnore(t *testing.T) { + for _, tt := range dropIgnoreTests { + t.Run(tt.desc, func(t *testing.T) { + inFile, err := Parse("in", []byte(tt.in), nil) + if err != nil { + t.Fatal(err) + } + if err := inFile.DropIgnore(tt.path); err != nil { + t.Fatal(err) + } + inFile.Cleanup() + got, err := inFile.Format() + if err != nil { + t.Fatal(err) + } + + outFile, err := Parse("out", []byte(tt.want), nil) + if err != nil { + t.Fatal(err) + } + want, err := outFile.Format() + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, want) { + t.Fatalf("got:\n%s\nwant:\n%s", got, want) + } + }) + } +} diff --git a/modfile/work.go b/modfile/work.go index 6f92644..09df5ea 100644 --- a/modfile/work.go +++ b/modfile/work.go @@ -329,5 +329,5 @@ func (f *WorkFile) SortBlocks() { // retract directives are not de-duplicated since comments are // meaningful, and versions may be retracted multiple times. func (f *WorkFile) removeDups() { - removeDups(f.Syntax, nil, &f.Replace, nil) + removeDups(f.Syntax, nil, &f.Replace, nil, nil) }