From 0090c13c91e8ccf4a91312e8237fd3ad2d00b729 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 24 Jul 2018 20:21:37 -0400 Subject: [PATCH] cmd/go: ignore unknown directives in dependency go.mod files This will help with forward compatibility when we add additional directives that only matter for the main module (or that can be safely ignored otherwise). Change-Id: Ida1e186fb2669b128aeb5a9a1187e2535b72b763 Reviewed-on: https://go-review.googlesource.com/125936 Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/modfile/read_test.go | 15 +++++++ src/cmd/go/internal/modfile/rule.go | 42 +++++++++++++++---- src/cmd/go/internal/modload/load.go | 4 +- .../go/testdata/mod/rsc.io_badmod_v1.0.0.txt | 11 +++++ .../go/testdata/script/mod_load_badmod.txt | 26 ++++++++++++ 5 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 src/cmd/go/testdata/mod/rsc.io_badmod_v1.0.0.txt create mode 100644 src/cmd/go/testdata/script/mod_load_badmod.txt diff --git a/src/cmd/go/internal/modfile/read_test.go b/src/cmd/go/internal/modfile/read_test.go index 254e549384..8cb1a3908c 100644 --- a/src/cmd/go/internal/modfile/read_test.go +++ b/src/cmd/go/internal/modfile/read_test.go @@ -66,6 +66,21 @@ func testPrint(t *testing.T, in, out string) { } } +func TestParseLax(t *testing.T) { + badFile := []byte(`module m + surprise attack + x y ( + z + ) + exclude v1.2.3 + replace <-!!! + `) + _, err := ParseLax("file", badFile, nil) + if err != nil { + t.Fatalf("ParseLax did not ignore irrelevant errors: %v", err) + } +} + // Test that when files in the testdata directory are parsed // and printed and parsed again, we get the same parse tree // both times. diff --git a/src/cmd/go/internal/modfile/rule.go b/src/cmd/go/internal/modfile/rule.go index bf6dd5aefc..21fce58331 100644 --- a/src/cmd/go/internal/modfile/rule.go +++ b/src/cmd/go/internal/modfile/rule.go @@ -87,7 +87,24 @@ func (f *File) AddComment(text string) { type VersionFixer func(path, version string) (string, error) +// Parse parses the data, reported in errors as being from file, +// into a File struct. It applies fix, if non-nil, to canonicalize all module versions found. func Parse(file string, data []byte, fix VersionFixer) (*File, error) { + return parseToFile(file, data, fix, true) +} + +// ParseLax is like Parse but ignores unknown statements. +// It is used when parsing go.mod files other than the main module, +// under the theory that most statement types we add in the future will +// only apply in the main module, like exclude and replace, +// and so we get better gradual deployments if old go commands +// simply ignore those statements when found in go.mod files +// in dependencies. +func ParseLax(file string, data []byte, fix VersionFixer) (*File, error) { + return parseToFile(file, data, fix, false) +} + +func parseToFile(file string, data []byte, fix VersionFixer, strict bool) (*File, error) { fs, err := parse(file, data) if err != nil { return nil, err @@ -100,20 +117,24 @@ func Parse(file string, data []byte, fix VersionFixer) (*File, error) { for _, x := range fs.Stmt { switch x := x.(type) { case *Line: - f.add(&errs, x, x.Token[0], x.Token[1:], fix) + f.add(&errs, x, x.Token[0], x.Token[1:], fix, strict) case *LineBlock: if len(x.Token) > 1 { - fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " ")) + if strict { + fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " ")) + } continue } switch x.Token[0] { default: - fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " ")) + if strict { + fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " ")) + } continue case "module", "require", "exclude", "replace": for _, l := range x.Line { - f.add(&errs, l, x.Token[0], l.Token, fix) + f.add(&errs, l, x.Token[0], l.Token, fix, strict) } } } @@ -125,15 +146,20 @@ func Parse(file string, data []byte, fix VersionFixer) (*File, error) { return f, nil } -func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, fix VersionFixer) { - // TODO: We should pass in a flag saying whether this module is a dependency. - // If so, we should ignore all unknown directives and not attempt to parse +func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, fix VersionFixer, strict bool) { + // If strict is false, this module is a dependency. + // We ignore all unknown directives and do not attempt to parse // replace and exclude either. They don't matter, and it will work better for - // forward compatibility if we can depend on modules that have local changes. + // forward compatibility if we can depend on modules that have unknown + // statements (presumed relevant only when acting as the main module). + if !strict && verb != "module" && verb != "require" { + return + } switch verb { default: fmt.Fprintf(errs, "%s:%d: unknown directive: %s\n", f.Syntax.Name, line.Start.Line, verb) + case "module": if f.Module != nil { fmt.Fprintf(errs, "%s:%d: repeated module statement\n", f.Syntax.Name, line.Start.Line) diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 5333c65d2c..dd8a60eb09 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -764,7 +764,7 @@ func (r *mvsReqs) required(mod module.Version) ([]module.Version, error) { base.Errorf("go: parsing %s: %v", base.ShortPath(gomod), err) return nil, ErrRequire } - f, err := modfile.Parse(gomod, data, nil) + f, err := modfile.ParseLax(gomod, data, nil) if err != nil { base.Errorf("go: parsing %s: %v", base.ShortPath(gomod), err) return nil, ErrRequire @@ -792,7 +792,7 @@ func (r *mvsReqs) required(mod module.Version) ([]module.Version, error) { base.Errorf("go: %s@%s: %v\n", mod.Path, mod.Version, err) return nil, ErrRequire } - f, err := modfile.Parse("go.mod", data, nil) + f, err := modfile.ParseLax("go.mod", data, nil) if err != nil { base.Errorf("go: %s@%s: parsing go.mod: %v", mod.Path, mod.Version, err) return nil, ErrRequire diff --git a/src/cmd/go/testdata/mod/rsc.io_badmod_v1.0.0.txt b/src/cmd/go/testdata/mod/rsc.io_badmod_v1.0.0.txt new file mode 100644 index 0000000000..993ceb7a0b --- /dev/null +++ b/src/cmd/go/testdata/mod/rsc.io_badmod_v1.0.0.txt @@ -0,0 +1,11 @@ +rsc.io/badmod v1.0.0 +written by hand + +-- .mod -- +module rsc.io/badmod +hello world +-- .info -- +{"Version":"v1.0.0"} +-- x.go -- +package x + diff --git a/src/cmd/go/testdata/script/mod_load_badmod.txt b/src/cmd/go/testdata/script/mod_load_badmod.txt new file mode 100644 index 0000000000..68c8b3792b --- /dev/null +++ b/src/cmd/go/testdata/script/mod_load_badmod.txt @@ -0,0 +1,26 @@ +# Unknown lines should be ignored in dependency go.mod files. +env GO111MODULE=on +go list -m all + +# ... and in replaced dependency go.mod files. +cp go.mod go.mod.usesub +go list -m all + +# ... but not in the main module. +cp go.mod.bad go.mod +! go list -m all +stderr 'unknown directive: hello' + +-- go.mod -- +module m +require rsc.io/badmod v1.0.0 +-- go.mod.bad -- +module m +hello world +-- go.mod.usesub -- +module m +require rsc.io/badmod v1.0.0 +replace rsc.io/badmod v1.0.0 => ./sub +-- sub/go.mod -- +module sub +hello world