cmd/go/internal/modget: if building packages, only update go.mod if the build succeeds

Fixes #41315

Change-Id: I5b18a0c2d1d72ff556a882e862b95133deb3ef98
Reviewed-on: https://go-review.googlesource.com/c/go/+/255970
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index 7e573bacb786593cb2ffef9a2e2500a514b17bad..f1cf8b17a8a641e11f0381394774afef2709e1c4 100644 (file)
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -588,6 +588,20 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
                modload.LoadPackages(ctx, loadOpts, pkgPatterns...)
        }
 
+       // If -d was specified, we're done after the module work.
+       // We've already downloaded modules by loading packages above.
+       // Otherwise, we need to build and install the packages matched by
+       // command line arguments. This may be a different set of packages,
+       // since we only build packages for the target platform.
+       // Note that 'go get -u' without arguments is equivalent to
+       // 'go get -u .', so we'll typically build the package in the current
+       // directory.
+       if !*getD && len(pkgPatterns) > 0 {
+               work.BuildInit()
+               pkgs := load.PackagesForBuild(ctx, pkgPatterns)
+               work.InstallPackages(ctx, pkgPatterns, pkgs)
+       }
+
        // Everything succeeded. Update go.mod.
        modload.AllowWriteGoMod()
        modload.WriteGoMod()
@@ -600,21 +614,6 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
        // contains information about direct dependencies that WriteGoMod uses.
        // Refactor to avoid these kinds of global side effects.
        reportRetractions(ctx)
-
-       // If -d was specified, we're done after the module work.
-       // We've already downloaded modules by loading packages above.
-       // Otherwise, we need to build and install the packages matched by
-       // command line arguments. This may be a different set of packages,
-       // since we only build packages for the target platform.
-       // Note that 'go get -u' without arguments is equivalent to
-       // 'go get -u .', so we'll typically build the package in the current
-       // directory.
-       if *getD || len(pkgPatterns) == 0 {
-               return
-       }
-       work.BuildInit()
-       pkgs := load.PackagesForBuild(ctx, pkgPatterns)
-       work.InstallPackages(ctx, pkgPatterns, pkgs)
 }
 
 // parseArgs parses command-line arguments and reports errors.

diff --git a/src/cmd/go/testdata/script/mod_get_errors.txt b/src/cmd/go/testdata/script/mod_get_errors.txt
index 5b1b8367bb31f072ef39c4058dde54d10982f559..7ce045ff82e422fbf3ea9febf5a09c49ab31f375 100644 (file)
--- a/src/cmd/go/testdata/script/mod_get_errors.txt
+++ b/src/cmd/go/testdata/script/mod_get_errors.txt
@@ -1,5 +1,10 @@
 cp go.mod go.mod.orig
 
+
+# Both 'go get' and 'go get -d' should fail, without updating go.mod,
+# if the transitive dependencies of the requested package (by default,
+# the package in the current directory) cannot be resolved.
+
 ! go get
 stderr '^example.com/m imports\n\texample.com/badimport imports\n\texample.net/oops: import missing$'  # TODO: better error message
 cmp go.mod.orig go.mod
@@ -8,16 +13,48 @@ cmp go.mod.orig go.mod
 stderr '^example.com/m imports\n\texample.com/badimport imports\n\texample.net/oops: import missing$'  # TODO: better error message
 cmp go.mod.orig go.mod
 
+cd importsyntax
+
+
+# If 'go get' fails due to a compile error (such as a syntax error),
+# it should not update the go.mod file.
+
+! go get
+stderr '^..[/\\]badimport[/\\]syntaxerror[/\\]syntaxerror.go:1:1: expected ''package'', found pack$'  # TODO: An import stack would be nice.
+cmp ../go.mod.orig ../go.mod
+
+
+# A syntax error in a dependency prevents the compiler from needing that
+# dependency's imports, so 'go get -d' should not report an error when those
+# imports cannot be resolved: it has all of the dependencies that the compiler
+# needs, and the user did not request to run the compiler.
+
+go get -d
+cmp ../go.mod.syntax-d ../go.mod
+
+
 -- go.mod --
 module example.com/m
 
 go 1.16
 
 replace example.com/badimport v0.1.0 => ./badimport
+-- go.mod.syntax-d --
+module example.com/m
+
+go 1.16
+
+replace example.com/badimport v0.1.0 => ./badimport
+
+require example.com/badimport v0.1.0
 -- m.go --
 package m
 
 import _ "example.com/badimport"
+-- importsyntax/importsyntax.go --
+package importsyntax
+
+import _ "example.com/badimport/syntaxerror"
 -- badimport/go.mod --
 module example.com/badimport
 
@@ -26,3 +63,7 @@ go 1.16
 package badimport
 
 import "example.net/oops"
+-- badimport/syntaxerror/syntaxerror.go --
+pack-age syntaxerror // sic
+
+import "example.net/oops"