Switch to letting go get mutate go.mod

go_modules/helpers/main.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/dependabot/dependabot-core/go_modules/helpers/importresolver"
 	"github.com/dependabot/dependabot-core/go_modules/helpers/updatechecker"
-	"github.com/dependabot/dependabot-core/go_modules/helpers/updater"
 )
 
 type HelperParams struct {
@@ -37,10 +36,6 @@ func main() {
 		var args updatechecker.Args
 		parseArgs(helperParams.Args, &args)
 		funcOut, funcErr = updatechecker.GetUpdatedVersion(&args)
-	case "updateDependencyFile":
-		var args updater.Args
-		parseArgs(helperParams.Args, &args)
-		funcOut, funcErr = updater.UpdateDependencyFile(&args)
 	case "getVcsRemoteForImport":
 		var args importresolver.Args
 		parseArgs(helperParams.Args, &args)

go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb

@@ -91,16 +91,17 @@ def update_files # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity
             # Replace full paths with path hashes in the go.mod
             substitute_all(substitutions)
 
-            # Set the stubbed replace directives
-            update_go_mod(dependencies)
-
-            # Then run `go get` to pick up other changes to the file caused by
-            # the upgrade
-            run_go_get
+            # TODO as far as I can tell, this array will only ever have one value, but we should assert that somehow
+            # and blow up if it's ever not true... I'm not a rubyist, so not sure how best to handle?
+            dep = dependencies.first
+            # `go get` will generate the required go.mod/go.sum updates for the new dep version
+            run_go_get(dep)
 
             # If we stubbed modules, don't run `go mod {tidy,vendor}` as
             # dependencies are incomplete
             if substitutions.empty?
+              # go mod tidy should run before go mod vendor to ensure any
+              # dependencies removed by go mod tidy are also removed from vendors.
               run_go_mod_tidy
               run_go_vendor
             else
@@ -151,26 +152,7 @@ def run_go_vendor
           handle_subprocess_error(stderr) unless status.success?
         end
 
-        def update_go_mod(dependencies)
-          deps = dependencies.map do |dep|
-            {
-              name: dep.name,
-              version: "v" + dep.version.sub(/^v/i, ""),
-              indirect: dep.requirements.empty?
-            }
-          end
-
-          body = SharedHelpers.run_helper_subprocess(
-            command: NativeHelpers.helper_path,
-            env: ENVIRONMENT,
-            function: "updateDependencyFile",
-            args: { dependencies: deps }
-          )
-
-          write_go_mod(body)
-        end
-
-        def run_go_get
+        def run_go_get(dep)
           tmp_go_file = "#{SecureRandom.hex}.go"
 
           package = Dir.glob("[^\._]*.go").any? do |path|
@@ -179,7 +161,12 @@ def run_go_get
 
           File.write(tmp_go_file, "package dummypkg\n") unless package
 
-          _, stderr, status = Open3.capture3(ENVIRONMENT, "go get -d")
+          # Use version pinning rather than `latest` just in case
+          # a new version gets released in the middle of our run.
+          version = "v" + dep.version.sub(/^v/i, "")
+          # TODO: go 1.18 will make `-d` the default behavior, so remove the flag then
+          command = "go get -d #{dep.name}@#{version}"
+          _, stderr, status = Open3.capture3(ENVIRONMENT, command)
           handle_subprocess_error(stderr) unless status.success?
         ensure
           File.delete(tmp_go_file) if File.exist?(tmp_go_file)

go_modules/lib/dependabot/go_modules/update_checker.rb

@@ -64,6 +64,18 @@ def find_latest_resolvable_version
             # private git dependencies
             env = { "GOPRIVATE" => "*" }
 
+            # Note: rather than leveraging our custom getUpdatedVersion
+            # and then passing that into `go get` within go_mod_updater
+            # we could invert this and run `go get dep@latest` and then
+            # inspect the go.mod/go.sum file to see what it was bumped
+            # to, and then flow from there. It would remove the need for
+            # https://github.com/dependabot/gomodules-extracted.
+            # But currently `go get` is a bit slower than our fast
+            # version checker. That may change in go 1.17 with
+            # lazy-loading: https://github.com/golang/go/issues/36460
+            # Also it's unclear if this would work with the pipeline
+            # approach of dependabot which naturally leads to splitting
+            # the finding of updates from actually updating to them.
             SharedHelpers.run_helper_subprocess(
               command: NativeHelpers.helper_path,
               env: env,