From e6ae8ea2beec5b4e7fea981868619436fd968410 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Mon, 25 Oct 2021 15:21:55 -0400 Subject: [PATCH 1/2] Remove tmp dirs after update_files runs --- .../lib/dependabot/go_modules/file_updater/go_mod_updater.rb | 2 ++ 1 file changed, 2 insertions(+) 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 7ec2b47aa78..93c71503da5 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 @@ -133,6 +133,8 @@ def update_files # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity end { go_mod: updated_go_mod, go_sum: updated_go_sum } + ensure + FileUtils.remove_dir(repo_contents_path) if File.exists?(repo_contents_path) end end From d6fd63543049266611bda764ec504158d0da6746 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 26 Oct 2021 13:52:51 -0400 Subject: [PATCH 2/2] Ensure tmp dirs are removed after running updated_dependency_files; update tests Failed attempt to remove temporary directories with `force=true`. There may be an open handle on the file that's preventing the directory from being removed. The directories can't be removed any earlier than here since `vendor_updater` in `common` `cd`s into the directory and will throw errors if the dirs are removed. --- .../lib/dependabot/go_modules/file_updater.rb | 4 +++- .../go_modules/file_updater/go_mod_updater.rb | 2 -- .../dependabot/go_modules/file_updater_spec.rb | 14 ++++++++------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/go_modules/lib/dependabot/go_modules/file_updater.rb b/go_modules/lib/dependabot/go_modules/file_updater.rb index 551dc26a1b3..8c0cf5d7275 100644 --- a/go_modules/lib/dependabot/go_modules/file_updater.rb +++ b/go_modules/lib/dependabot/go_modules/file_updater.rb @@ -44,13 +44,15 @@ def updated_dependency_files vendor_updater.updated_vendor_cache_files(base_directory: directory). each do |file| - updated_files << file + updated_files << file end end raise "No files changed!" if updated_files.none? updated_files + ensure + FileUtils.remove_entry(@repo_contents_path, force=true) if File.exists?(@repo_contents_path) end private 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 93c71503da5..7ec2b47aa78 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 @@ -133,8 +133,6 @@ def update_files # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity end { go_mod: updated_go_mod, go_sum: updated_go_sum } - ensure - FileUtils.remove_dir(repo_contents_path) if File.exists?(repo_contents_path) end end diff --git a/go_modules/spec/dependabot/go_modules/file_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater_spec.rb index 6f034678ac8..c214ee97cb2 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater_spec.rb @@ -150,6 +150,8 @@ module declares its path as: go.etcd.io/bbolt ) end + subject(:updated_files) { updater.updated_dependency_files } + it "includes an updated go.mod" do expect(updated_files.find { |f| f.name == "go.mod" }).to_not be_nil end @@ -226,12 +228,12 @@ module declares its path as: go.etcd.io/bbolt }] end + subject(:updated_files) { updater.updated_dependency_files } + it "updates the go.mod" do expect(go_mod_body).to include("github.com/pkg/errors v0.8.0") - updater.updated_dependency_files - - go_mod_file = updater.updated_dependency_files.find do |file| + go_mod_file = updated_files.find do |file| file.name == "go.mod" end @@ -239,7 +241,7 @@ module declares its path as: go.etcd.io/bbolt end it "includes the vendored files" do - expect(updater.updated_dependency_files.map(&:name)).to match_array( + expect(updated_files.map(&:name)).to match_array( %w( go.mod go.sum @@ -316,7 +318,7 @@ module declares its path as: go.etcd.io/bbolt end it "vendors in the right directory" do - expect(updater.updated_dependency_files.map(&:name)).to match_array( + expect(updated_files.map(&:name)).to match_array( %w( go.mod go.sum @@ -330,7 +332,7 @@ module declares its path as: go.etcd.io/bbolt ) ) - updater.updated_dependency_files.map(&:directory).each do |dir| + updated_files.map(&:directory).each do |dir| expect(dir).to eq("/nested") end end