Skip to content

Commit

Permalink
cmd/go: ignore unknown directives in dependency go.mod files
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
rsc committed Jul 28, 2018
1 parent 9cde804 commit 0090c13
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 10 deletions.
15 changes: 15 additions & 0 deletions src/cmd/go/internal/modfile/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 34 additions & 8 deletions src/cmd/go/internal/modfile/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
}
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/internal/modload/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/cmd/go/testdata/mod/rsc.io_badmod_v1.0.0.txt
Original file line number Diff line number Diff line change
@@ -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

26 changes: 26 additions & 0 deletions src/cmd/go/testdata/script/mod_load_badmod.txt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 0090c13

Please sign in to comment.