From 8460ce50621a0962fca8d703f50c2bcbaa8b3836 Mon Sep 17 00:00:00 2001 From: Fatih Arslan Date: Tue, 8 Dec 2020 12:05:22 -0800 Subject: [PATCH 1/6] go_modules: don't raise error for go mod tidy `go mod tidy` makes sure that go.mod matches the source code in a Go module. This also means that `dependabot` is doing a quassi linting on the code, which prevents updating versions in a go.mod if the source code doesn't got build. One particular use case is when a project is missing generated files. Sometimes people don't submit and commit the generated files of a code generator (i.e: `protobuf`) and do it in pre-script in the CI. Because `dependabot` is not aware of this pre-script, it'll run `go mod tidy` and fail for the user. We should silently continue if `go mod tidy` fails and not raise an error. This will allow dependabot still to run `go mod tidy`, but will not block users for these kind of edge cases. --- .../go_modules/file_updater/go_mod_updater.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb index 94a872beee0..4466b9bd103 100644 --- a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb +++ b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb @@ -118,9 +118,12 @@ def update_files # rubocop:disable Metrics/AbcSize def run_go_mod_tidy return unless tidy? + # NOTE(arslan): use `go mod tidy -e` once Go 1.16 is out: + # https://github.com/golang/go/commit/3aa09489ab3aa13a3ac78b1ff012b148ffffe367 command = "go mod tidy" + _, stderr, status = Open3.capture3(ENVIRONMENT, command) - handle_subprocess_error(stderr) unless status.success? + handle_subprocess_error(stderr, true) unless status.success? end def run_go_vendor @@ -243,7 +246,7 @@ def substitute_all(substitutions) write_go_mod(body) end - def handle_subprocess_error(stderr) + def handle_subprocess_error(stderr, istidy=false) stderr = stderr.gsub(Dir.getwd, "") error_regex = RESOLVABILITY_ERROR_REGEXES.find { |r| stderr =~ r } @@ -259,6 +262,12 @@ def handle_subprocess_error(stderr) new(go_mod_path, match[1], match[2]) end + # we explicitly don't raise an error for 'go mod tidy' and silently + # continue here. `go mod tidy` shouldn't block updating versions + # because there are some edge cases where it's ok to fail (such as + # generated files not available yet to us). + return if istidy + # We don't know what happened so we raise a generic error msg = stderr.lines.last(10).join.strip raise Dependabot::DependabotError, msg From e839c12f90d20fbfa2266708cb8db7bb418feafe Mon Sep 17 00:00:00 2001 From: Fatih Arslan Date: Tue, 8 Dec 2020 16:29:34 -0800 Subject: [PATCH 2/6] go_modules: fail silently for all cases --- .../go_modules/file_updater/go_mod_updater.rb | 15 ++++++--------- .../file_updater/go_mod_updater_spec.rb | 4 ++-- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb index 4466b9bd103..4e33eb8469b 100644 --- a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb +++ b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb @@ -122,8 +122,11 @@ def run_go_mod_tidy # https://github.com/golang/go/commit/3aa09489ab3aa13a3ac78b1ff012b148ffffe367 command = "go mod tidy" - _, stderr, status = Open3.capture3(ENVIRONMENT, command) - handle_subprocess_error(stderr, true) unless status.success? + # we explicitly don't raise an error for 'go mod tidy' and silently + # continue here. `go mod tidy` shouldn't block updating versions + # because there are some edge cases where it's OK to fail (such as + # generated files not available yet to us). + Open3.capture3(ENVIRONMENT, command) end def run_go_vendor @@ -246,7 +249,7 @@ def substitute_all(substitutions) write_go_mod(body) end - def handle_subprocess_error(stderr, istidy=false) + def handle_subprocess_error(stderr) stderr = stderr.gsub(Dir.getwd, "") error_regex = RESOLVABILITY_ERROR_REGEXES.find { |r| stderr =~ r } @@ -262,12 +265,6 @@ def handle_subprocess_error(stderr, istidy=false) new(go_mod_path, match[1], match[2]) end - # we explicitly don't raise an error for 'go mod tidy' and silently - # continue here. `go mod tidy` shouldn't block updating versions - # because there are some edge cases where it's ok to fail (such as - # generated files not available yet to us). - return if istidy - # We don't know what happened so we raise a generic error msg = stderr.lines.last(10).join.strip raise Dependabot::DependabotError, msg diff --git a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb index e7a887b9f56..7ee88ee7db8 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb @@ -225,10 +225,10 @@ # OpenAPIV2 has been renamed to openapiv2 in this version let(:dependency_version) { "v0.5.1" } - it "raises a DependencyFileNotResolvable error" do + it "does not raises a DependencyFileNotResolvable error" do error_class = Dependabot::DependencyFileNotResolvable expect { updater.updated_go_sum_content }. - to raise_error(error_class) do |error| + to_not raise_error(error_class) do |error| expect(error.message).to include("googleapis/gnostic/OpenAPIv2") end end From a4963733a7904b16e9418d75ff6e505b7931f955 Mon Sep 17 00:00:00 2001 From: Fatih Arslan Date: Tue, 8 Dec 2020 16:51:29 -0800 Subject: [PATCH 3/6] go_modules: fix linter warning --- .../lib/dependabot/go_modules/file_updater/go_mod_updater.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb index 4e33eb8469b..42abe59b6c5 100644 --- a/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb +++ b/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb @@ -125,7 +125,7 @@ def run_go_mod_tidy # we explicitly don't raise an error for 'go mod tidy' and silently # continue here. `go mod tidy` shouldn't block updating versions # because there are some edge cases where it's OK to fail (such as - # generated files not available yet to us). + # generated files not available yet to us). Open3.capture3(ENVIRONMENT, command) end From c3995ee65af50ea2e46da443e64511ab7b2ad084 Mon Sep 17 00:00:00 2001 From: Fatih Arslan Date: Wed, 9 Dec 2020 10:11:29 -0800 Subject: [PATCH 4/6] Update go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb Co-authored-by: Philip Harrison --- .../dependabot/go_modules/file_updater/go_mod_updater_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb index 7ee88ee7db8..62b529f3b64 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb @@ -230,6 +230,10 @@ expect { updater.updated_go_sum_content }. to_not raise_error(error_class) do |error| expect(error.message).to include("googleapis/gnostic/OpenAPIv2") + it "updates the go.mod" do + expect(updater.updated_go_mod_content).to include( + %(github.com/googleapis/gnostic v0.5.1\n) + ) end end end From 61b4ec507257117bc79d1eb5618723d4a82558a7 Mon Sep 17 00:00:00 2001 From: Fatih Arslan Date: Wed, 9 Dec 2020 11:03:16 -0800 Subject: [PATCH 5/6] go_modules: fix the added new test --- .../go_modules/file_updater/go_mod_updater_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb index 62b529f3b64..3f84937041d 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb @@ -230,12 +230,14 @@ expect { updater.updated_go_sum_content }. to_not raise_error(error_class) do |error| expect(error.message).to include("googleapis/gnostic/OpenAPIv2") - it "updates the go.mod" do - expect(updater.updated_go_mod_content).to include( - %(github.com/googleapis/gnostic v0.5.1\n) - ) end end + + it "updates the go.mod" do + expect(updater.updated_go_mod_content).to include( + %(github.com/googleapis/gnostic v0.5.1 // indirect\n) + ) + end end end From aa67f7d3eb9b74ccf9e309cf6b5e742fd68571fe Mon Sep 17 00:00:00 2001 From: Philip Harrison Date: Thu, 10 Dec 2020 09:52:05 +0000 Subject: [PATCH 6/6] Remove expectation of error message and add a comment --- .../go_modules/file_updater/go_mod_updater_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb index 3f84937041d..ccfcd88f5e0 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb @@ -225,12 +225,10 @@ # OpenAPIV2 has been renamed to openapiv2 in this version let(:dependency_version) { "v0.5.1" } + # NOTE: We explitly don't want to raise a resolvability error from go mod tidy it "does not raises a DependencyFileNotResolvable error" do - error_class = Dependabot::DependencyFileNotResolvable expect { updater.updated_go_sum_content }. - to_not raise_error(error_class) do |error| - expect(error.message).to include("googleapis/gnostic/OpenAPIv2") - end + to_not raise_error(Dependabot::DependencyFileNotResolvable) end it "updates the go.mod" do