From cda258832f3e51a1b5ab4e270de99f59631cfb27 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 13:22:52 +0200 Subject: [PATCH 1/9] add git_shallow_clone? method to LocalRepository module --- lib/datadog/ci/git/local_repository.rb | 7 +++++++ sig/datadog/ci/git/local_repository.rbs | 2 ++ spec/datadog/ci/git/local_repository_spec.rb | 6 ++++++ 3 files changed, 15 insertions(+) diff --git a/lib/datadog/ci/git/local_repository.rb b/lib/datadog/ci/git/local_repository.rb index f002c7d5..ba301ee2 100644 --- a/lib/datadog/ci/git/local_repository.rb +++ b/lib/datadog/ci/git/local_repository.rb @@ -159,6 +159,13 @@ def self.git_generate_packfiles(included_commits:, excluded_commits:, path:) nil end + def self.git_shallow_clone? + exec_git_command("git rev-parse --is-shallow-repository") == "true" + rescue => e + log_failure(e, "git shallow clone") + false + end + # makes .exec_git_command private to make sure that this method # is not called from outside of this module with insecure parameters class << self diff --git a/sig/datadog/ci/git/local_repository.rbs b/sig/datadog/ci/git/local_repository.rbs index add6cb97..f3c969d9 100644 --- a/sig/datadog/ci/git/local_repository.rbs +++ b/sig/datadog/ci/git/local_repository.rbs @@ -33,6 +33,8 @@ module Datadog def self.git_generate_packfiles: (included_commits: Enumerable[String], excluded_commits: Enumerable[String], path: String) -> String? + def self.git_shallow_clone?: () -> bool + private def self.filter_invalid_commits: (Enumerable[String] commits) -> Array[String] diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index cdb8a166..8bfe69f5 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -182,6 +182,12 @@ def with_custom_git_environment end end + describe ".git_shallow_clone?" do + subject { described_class.git_shallow_clone? } + + it { is_expected.to be_falsey } + end + context "with git folder" do include_context "with git fixture", "gitdir_with_commit" From 5b2bd566b1e0ef34b3493176d49bc44890013b44 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 13:38:36 +0200 Subject: [PATCH 2/9] test .git_shallow_clone? method with a reals shallow clone --- spec/datadog/ci/git/local_repository_spec.rb | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index 8bfe69f5..01e93f56 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -299,4 +299,28 @@ def with_custom_git_environment it { is_expected.to eq("first-tag") } end end + + context "with shallow clone" do + let(:tmpdir) { Dir.mktmpdir } + after { FileUtils.remove_entry(tmpdir) } + + before do + # shallow clone datadog-ci-rb repository + `cd #{tmpdir} && git clone --depth 1 https://github.com/DataDog/datadog-ci-rb` + end + + def with_shallow_clone_git_dir + ClimateControl.modify("GIT_DIR" => File.join(tmpdir, "datadog-ci-rb/.git")) do + yield + end + end + + describe ".git_shallow_clone?" do + subject do + with_shallow_clone_git_dir { described_class.git_shallow_clone? } + end + + it { is_expected.to be_truthy } + end + end end From 6688f65661c4b4693598341ad349eab716a22515 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 13:41:06 +0200 Subject: [PATCH 3/9] test .git_commits with shallow clone --- spec/datadog/ci/git/local_repository_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index 01e93f56..80de249e 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -322,5 +322,18 @@ def with_shallow_clone_git_dir it { is_expected.to be_truthy } end + + describe ".git_commits" do + subject do + with_shallow_clone_git_dir { described_class.git_commits } + end + + it "returns a list of single git commit sha" do + expect(subject).to be_kind_of(Array) + expect(subject).not_to be_empty + expect(subject).to have(1).item + expect(subject.first).to match(/^\h{40}$/) + end + end end end From 1270d8f080cbcf1e189278c85654053ad6647ac4 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 13:47:52 +0200 Subject: [PATCH 4/9] add .git_unshallow method --- lib/datadog/ci/git/local_repository.rb | 14 ++++++++++++++ spec/datadog/ci/git/local_repository_spec.rb | 17 +++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lib/datadog/ci/git/local_repository.rb b/lib/datadog/ci/git/local_repository.rb index ba301ee2..cf19ef88 100644 --- a/lib/datadog/ci/git/local_repository.rb +++ b/lib/datadog/ci/git/local_repository.rb @@ -166,6 +166,20 @@ def self.git_shallow_clone? false end + def self.git_unshallow + exec_git_command( + "git fetch " \ + "--shallow-since=\"1 month ago\" " \ + "--update-shallow " \ + "--filter=\"blob:none\" " \ + "--recurse-submodules=no " \ + "$(git config --default origin --get clone.defaultRemoteName) $(git rev-parse HEAD)" + ) + rescue => e + log_failure(e, "git unshallow") + nil + end + # makes .exec_git_command private to make sure that this method # is not called from outside of this module with insecure parameters class << self diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index 80de249e..fd1c34f2 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -335,5 +335,22 @@ def with_shallow_clone_git_dir expect(subject.first).to match(/^\h{40}$/) end end + + describe ".git_unshallow" do + subject do + with_shallow_clone_git_dir { described_class.git_unshallow } + end + let(:commits0) do + with_shallow_clone_git_dir { described_class.git_commits } + end + let(:commits) do + with_shallow_clone_git_dir { described_class.git_commits } + end + + it "unshallows the repository" do + expect(subject).to be_truthy + expect(commits.size).to be > 1 + end + end end end From 1b6458cb8e74fa122378cc230d99a3a82e67a6d3 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 13:57:09 +0200 Subject: [PATCH 5/9] fix jruby specs --- sig/datadog/ci/git/local_repository.rbs | 2 ++ spec/datadog/ci/git/local_repository_spec.rb | 3 +++ 2 files changed, 5 insertions(+) diff --git a/sig/datadog/ci/git/local_repository.rbs b/sig/datadog/ci/git/local_repository.rbs index f3c969d9..6354161c 100644 --- a/sig/datadog/ci/git/local_repository.rbs +++ b/sig/datadog/ci/git/local_repository.rbs @@ -35,6 +35,8 @@ module Datadog def self.git_shallow_clone?: () -> bool + def self.git_unshallow: () -> String? + private def self.filter_invalid_commits: (Enumerable[String] commits) -> Array[String] diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index fd1c34f2..d7287adc 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -337,6 +337,9 @@ def with_shallow_clone_git_dir end describe ".git_unshallow" do + # skip for jruby for now - old git version DD docker image + before { skip if PlatformHelpers.jruby? } + subject do with_shallow_clone_git_dir { described_class.git_unshallow } end From 055edcbff9bb3ffe33b6aef6e1154e8ca88fa29e Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 14:39:58 +0200 Subject: [PATCH 6/9] add repository unshallowing logic to Git::TreeUploader --- lib/datadog/ci/git/tree_uploader.rb | 29 +++- spec/datadog/ci/git/tree_uploader_spec.rb | 156 +++++++++++++++------- 2 files changed, 135 insertions(+), 50 deletions(-) diff --git a/lib/datadog/ci/git/tree_uploader.rb b/lib/datadog/ci/git/tree_uploader.rb index cb2a117d..3bd9130b 100644 --- a/lib/datadog/ci/git/tree_uploader.rb +++ b/lib/datadog/ci/git/tree_uploader.rb @@ -26,8 +26,6 @@ def call(repository_url) Datadog.logger.debug { "Uploading git tree for repository #{repository_url}" } - # 2. Check if the repository clone is shallow and unshallow if appropriate - # TO BE ADDED IN CIVIS-2863 latest_commits = LocalRepository.git_commits head_commit = latest_commits&.first if head_commit.nil? @@ -36,10 +34,33 @@ def call(repository_url) end begin + # This "far from perfect" code stays here as is and not split up multiple methods and classes because + # the unshallowing logic with optimizations is inherently complex and I don't hide this + # complexity on purpose. excluded_commits, included_commits = split_known_commits(repository_url, latest_commits) if included_commits.empty? - Datadog.logger.debug("No new commits to upload") - return + if LocalRepository.git_shallow_clone? + Datadog.logger.debug("Detected shallow clone, unshallowing the git repository") + + unshallow_result = LocalRepository.git_unshallow + if unshallow_result.nil? + Datadog.logger.debug("Failed to unshallow the git repository, aborting git upload") + return + end + + excluded_commits, included_commits = split_known_commits( + repository_url, + LocalRepository.git_commits + ) + + if included_commits.empty? + Datadog.logger.debug("No new commits to upload after unshallowing") + return + end + else + Datadog.logger.debug("No new commits to upload") + return + end end rescue SearchCommits::ApiError => e Datadog.logger.debug("SearchCommits failed with #{e}, aborting git upload") diff --git a/spec/datadog/ci/git/tree_uploader_spec.rb b/spec/datadog/ci/git/tree_uploader_spec.rb index 7af2d61a..a3172ca3 100644 --- a/spec/datadog/ci/git/tree_uploader_spec.rb +++ b/spec/datadog/ci/git/tree_uploader_spec.rb @@ -19,7 +19,6 @@ let(:search_commits) { double("search_commits", call: backend_commits) } before do - allow(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return(latest_commits) allow(Datadog::CI::Git::SearchCommits).to receive(:new).with(api: api).and_return(search_commits) end @@ -33,71 +32,136 @@ end end - context "when the latest commits list is empty" do - let(:latest_commits) { [] } - - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("Got empty latest commits list, aborting git upload") - - tree_uploader.call(repository_url) - end - end - - context "when the backend commits search fails" do + context "when API is configured" do before do - expect(search_commits).to receive(:call).and_raise(Datadog::CI::Git::SearchCommits::ApiError, "test error") + expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return(latest_commits) end - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("SearchCommits failed with test error, aborting git upload") + context "when the latest commits list is empty" do + let(:latest_commits) { [] } - tree_uploader.call(repository_url) + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("Got empty latest commits list, aborting git upload") + + tree_uploader.call(repository_url) + end end - end - context "when all commits are known to the backend" do - let(:backend_commits) { latest_commits } + context "when the backend commits search fails" do + before do + expect(search_commits).to receive(:call).and_raise(Datadog::CI::Git::SearchCommits::ApiError, "test error") + end - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("No new commits to upload") + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("SearchCommits failed with test error, aborting git upload") - tree_uploader.call(repository_url) + tree_uploader.call(repository_url) + end end - end - context "when some commits are new" do - let(:upload_packfile) { double("upload_packfile", call: nil) } + context "when all commits are known to the backend" do + let(:backend_commits) { latest_commits } - before do - expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( - included_commits: latest_commits - backend_commits.to_a, - excluded_commits: backend_commits - ).and_yield("packfile_path") - - expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( - api: api, - head_commit_sha: head_commit, - repository_url: repository_url - ).and_return(upload_packfile) + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("No new commits to upload") + + tree_uploader.call(repository_url) + end + + context "when the repository is shallow cloned" do + before do + expect(Datadog::CI::Git::LocalRepository).to receive(:git_shallow_clone?).and_return(true) + end + + context "when the unshallowing fails" do + before do + expect(Datadog::CI::Git::LocalRepository).to receive(:git_unshallow).and_return(nil) + end + + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("Failed to unshallow the git repository, aborting git upload") + + tree_uploader.call(repository_url) + end + end + + context "when the unshallowing succeeds" do + before do + expect(Datadog::CI::Git::LocalRepository).to receive(:git_unshallow).and_return("unshallow_result") + end + + context "when there are new commits after unshallowing" do + before do + expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return( + latest_commits + %w[782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5] + ) + end + + it "uploads the new commits" do + expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( + included_commits: %w[782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5], + excluded_commits: backend_commits + ).and_yield("packfile_path") + + expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( + api: api, + head_commit_sha: head_commit, + repository_url: repository_url + ).and_return(double("upload_packfile", call: nil)) + + tree_uploader.call(repository_url) + end + end + + context "when there are no new commits" do + before do + expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return(latest_commits) + end + + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("No new commits to upload after unshallowing") + + tree_uploader.call(repository_url) + end + end + end + end end - context "when the packfile upload fails" do + context "when some commits are new" do + let(:upload_packfile) { double("upload_packfile", call: nil) } + before do - expect(upload_packfile).to receive(:call).and_raise(Datadog::CI::Git::UploadPackfile::ApiError, "test error") + expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( + included_commits: latest_commits - backend_commits.to_a, + excluded_commits: backend_commits + ).and_yield("packfile_path") + + expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( + api: api, + head_commit_sha: head_commit, + repository_url: repository_url + ).and_return(upload_packfile) end - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("Packfile upload failed with test error") + context "when the packfile upload fails" do + before do + expect(upload_packfile).to receive(:call).and_raise(Datadog::CI::Git::UploadPackfile::ApiError, "test error") + end - tree_uploader.call(repository_url) + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("Packfile upload failed with test error") + + tree_uploader.call(repository_url) + end end - end - context "when the packfile upload succeeds" do - it "uploads the new commits" do - expect(upload_packfile).to receive(:call).with(filepath: "packfile_path").and_return(nil) + context "when the packfile upload succeeds" do + it "uploads the new commits" do + expect(upload_packfile).to receive(:call).with(filepath: "packfile_path").and_return(nil) - tree_uploader.call(repository_url) + tree_uploader.call(repository_url) + end end end end From b4f78cc9ac99ef71e7d71d59c5037933d1e04c75 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 15:05:44 +0200 Subject: [PATCH 7/9] actually the unshallowing logic is the other way around: unshallow when there are new commits --- lib/datadog/ci/git/tree_uploader.rb | 36 +++----- spec/datadog/ci/git/tree_uploader_spec.rb | 106 +++++++++------------- 2 files changed, 58 insertions(+), 84 deletions(-) diff --git a/lib/datadog/ci/git/tree_uploader.rb b/lib/datadog/ci/git/tree_uploader.rb index 3bd9130b..6e57a176 100644 --- a/lib/datadog/ci/git/tree_uploader.rb +++ b/lib/datadog/ci/git/tree_uploader.rb @@ -34,33 +34,23 @@ def call(repository_url) end begin - # This "far from perfect" code stays here as is and not split up multiple methods and classes because - # the unshallowing logic with optimizations is inherently complex and I don't hide this - # complexity on purpose. + # ask the backend for the list of commits it already has excluded_commits, included_commits = split_known_commits(repository_url, latest_commits) + # if all commits are present in the backend, we don't need to upload anything if included_commits.empty? - if LocalRepository.git_shallow_clone? - Datadog.logger.debug("Detected shallow clone, unshallowing the git repository") - - unshallow_result = LocalRepository.git_unshallow - if unshallow_result.nil? - Datadog.logger.debug("Failed to unshallow the git repository, aborting git upload") - return - end + Datadog.logger.debug("No new commits to upload") + return + end - excluded_commits, included_commits = split_known_commits( - repository_url, - LocalRepository.git_commits - ) + # quite often we deal with shallow clones in CI environment + if LocalRepository.git_shallow_clone? && LocalRepository.git_unshallow + Datadog.logger.debug("Detected shallow clone and unshallowed the repository, repeating commits search") - if included_commits.empty? - Datadog.logger.debug("No new commits to upload after unshallowing") - return - end - else - Datadog.logger.debug("No new commits to upload") - return - end + # re-run the search with the updated commit list after unshallowing + excluded_commits, included_commits = split_known_commits( + repository_url, + LocalRepository.git_commits + ) end rescue SearchCommits::ApiError => e Datadog.logger.debug("SearchCommits failed with #{e}, aborting git upload") diff --git a/spec/datadog/ci/git/tree_uploader_spec.rb b/spec/datadog/ci/git/tree_uploader_spec.rb index a3172ca3..19e9c55a 100644 --- a/spec/datadog/ci/git/tree_uploader_spec.rb +++ b/spec/datadog/ci/git/tree_uploader_spec.rb @@ -67,6 +67,10 @@ tree_uploader.call(repository_url) end + end + + context "when some commits are new" do + let(:upload_packfile) { double("upload_packfile", call: nil) } context "when the repository is shallow cloned" do before do @@ -78,8 +82,11 @@ expect(Datadog::CI::Git::LocalRepository).to receive(:git_unshallow).and_return(nil) end - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("Failed to unshallow the git repository, aborting git upload") + it "uploads what we can upload" do + expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( + included_commits: %w[13c988d4f15e06bcdd0b0af290086a3079cdadb0], + excluded_commits: backend_commits + ).and_yield("packfile_path") tree_uploader.call(repository_url) end @@ -88,79 +95,56 @@ context "when the unshallowing succeeds" do before do expect(Datadog::CI::Git::LocalRepository).to receive(:git_unshallow).and_return("unshallow_result") + expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return( + latest_commits + %w[782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5] + ) end - context "when there are new commits after unshallowing" do - before do - expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return( - latest_commits + %w[782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5] - ) - end - - it "uploads the new commits" do - expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( - included_commits: %w[782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5], - excluded_commits: backend_commits - ).and_yield("packfile_path") - - expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( - api: api, - head_commit_sha: head_commit, - repository_url: repository_url - ).and_return(double("upload_packfile", call: nil)) - - tree_uploader.call(repository_url) - end - end - - context "when there are no new commits" do - before do - expect(Datadog::CI::Git::LocalRepository).to receive(:git_commits).and_return(latest_commits) - end + it "uploads the new commits" do + expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( + included_commits: %w[13c988d4f15e06bcdd0b0af290086a3079cdadb0 782d09e3fbfd8cf1b5c13f3eb9621362f9089ed5], + excluded_commits: backend_commits + ).and_yield("packfile_path") - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("No new commits to upload after unshallowing") - - tree_uploader.call(repository_url) - end + tree_uploader.call(repository_url) end end end - end - - context "when some commits are new" do - let(:upload_packfile) { double("upload_packfile", call: nil) } - before do - expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( - included_commits: latest_commits - backend_commits.to_a, - excluded_commits: backend_commits - ).and_yield("packfile_path") - - expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( - api: api, - head_commit_sha: head_commit, - repository_url: repository_url - ).and_return(upload_packfile) - end - - context "when the packfile upload fails" do + context "when the repository is not shallow cloned" do before do - expect(upload_packfile).to receive(:call).and_raise(Datadog::CI::Git::UploadPackfile::ApiError, "test error") + expect(Datadog::CI::Git::LocalRepository).to receive(:git_shallow_clone?).and_return(false) + + expect(Datadog::CI::Git::Packfiles).to receive(:generate).with( + included_commits: latest_commits - backend_commits.to_a, + excluded_commits: backend_commits + ).and_yield("packfile_path") + + expect(Datadog::CI::Git::UploadPackfile).to receive(:new).with( + api: api, + head_commit_sha: head_commit, + repository_url: repository_url + ).and_return(upload_packfile) end - it "logs a debug message and aborts the git upload" do - expect(Datadog.logger).to receive(:debug).with("Packfile upload failed with test error") + context "when the packfile upload fails" do + before do + expect(upload_packfile).to receive(:call).and_raise(Datadog::CI::Git::UploadPackfile::ApiError, "test error") + end + + it "logs a debug message and aborts the git upload" do + expect(Datadog.logger).to receive(:debug).with("Packfile upload failed with test error") - tree_uploader.call(repository_url) + tree_uploader.call(repository_url) + end end - end - context "when the packfile upload succeeds" do - it "uploads the new commits" do - expect(upload_packfile).to receive(:call).with(filepath: "packfile_path").and_return(nil) + context "when the packfile upload succeeds" do + it "uploads the new commits" do + expect(upload_packfile).to receive(:call).with(filepath: "packfile_path").and_return(nil) - tree_uploader.call(repository_url) + tree_uploader.call(repository_url) + end end end end From 97bcfac57d94871e47ab8b2e1629f72bc4c0381c Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 9 Apr 2024 15:27:00 +0200 Subject: [PATCH 8/9] remove unneeded code --- spec/datadog/ci/git/local_repository_spec.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index d7287adc..477b1c95 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -343,9 +343,6 @@ def with_shallow_clone_git_dir subject do with_shallow_clone_git_dir { described_class.git_unshallow } end - let(:commits0) do - with_shallow_clone_git_dir { described_class.git_commits } - end let(:commits) do with_shallow_clone_git_dir { described_class.git_commits } end From 2bcb5f05e9559fc138de8eac54858c8b2e54e184 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 10 Apr 2024 10:13:53 +0200 Subject: [PATCH 9/9] better naming, replace filter_map with filter, better tests for .is_shallow_clone? --- lib/datadog/ci/git/local_repository.rb | 6 +--- lib/datadog/ci/git/tree_uploader.rb | 14 +++++---- sig/datadog/ci/git/tree_uploader.rbs | 2 +- spec/datadog/ci/git/local_repository_spec.rb | 30 ++++++++++++++++---- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/lib/datadog/ci/git/local_repository.rb b/lib/datadog/ci/git/local_repository.rb index cf19ef88..02dca051 100644 --- a/lib/datadog/ci/git/local_repository.rb +++ b/lib/datadog/ci/git/local_repository.rb @@ -186,11 +186,7 @@ class << self private def filter_invalid_commits(commits) - commits.filter_map do |commit| - next unless Utils::Git.valid_commit_sha?(commit) - - commit - end + commits.filter { |commit| Utils::Git.valid_commit_sha?(commit) } end def exec_git_command(cmd, stdin: nil) diff --git a/lib/datadog/ci/git/tree_uploader.rb b/lib/datadog/ci/git/tree_uploader.rb index 6e57a176..36ea0142 100644 --- a/lib/datadog/ci/git/tree_uploader.rb +++ b/lib/datadog/ci/git/tree_uploader.rb @@ -35,9 +35,9 @@ def call(repository_url) begin # ask the backend for the list of commits it already has - excluded_commits, included_commits = split_known_commits(repository_url, latest_commits) + known_commits, new_commits = fetch_known_commits_and_split(repository_url, latest_commits) # if all commits are present in the backend, we don't need to upload anything - if included_commits.empty? + if new_commits.empty? Datadog.logger.debug("No new commits to upload") return end @@ -47,7 +47,7 @@ def call(repository_url) Datadog.logger.debug("Detected shallow clone and unshallowed the repository, repeating commits search") # re-run the search with the updated commit list after unshallowing - excluded_commits, included_commits = split_known_commits( + known_commits, new_commits = fetch_known_commits_and_split( repository_url, LocalRepository.git_commits ) @@ -57,13 +57,13 @@ def call(repository_url) return end - Datadog.logger.debug { "Uploading packfiles for commits: #{included_commits}" } + Datadog.logger.debug { "Uploading packfiles for commits: #{new_commits}" } uploader = UploadPackfile.new( api: api, head_commit_sha: head_commit, repository_url: repository_url ) - Packfiles.generate(included_commits: included_commits, excluded_commits: excluded_commits) do |filepath| + Packfiles.generate(included_commits: new_commits, excluded_commits: known_commits) do |filepath| uploader.call(filepath: filepath) rescue UploadPackfile::ApiError => e Datadog.logger.debug("Packfile upload failed with #{e}") @@ -73,7 +73,9 @@ def call(repository_url) private - def split_known_commits(repository_url, latest_commits) + # Split the latest commits list into known and new commits + # based on the backend response provided by /search_commits endpoint + def fetch_known_commits_and_split(repository_url, latest_commits) Datadog.logger.debug { "Checking the latest commits list with backend: #{latest_commits}" } backend_commits = SearchCommits.new(api: api).call(repository_url, latest_commits) latest_commits.partition do |commit| diff --git a/sig/datadog/ci/git/tree_uploader.rbs b/sig/datadog/ci/git/tree_uploader.rbs index 5ef2c7dd..43eeb649 100644 --- a/sig/datadog/ci/git/tree_uploader.rbs +++ b/sig/datadog/ci/git/tree_uploader.rbs @@ -11,7 +11,7 @@ module Datadog private - def split_known_commits: (String repository_url, Array[String] latest_commits) -> [Array[String], Array[String]] + def fetch_known_commits_and_split: (String repository_url, Array[String] latest_commits) -> [Array[String], Array[String]] end end end diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index 477b1c95..9ad07089 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -182,12 +182,6 @@ def with_custom_git_environment end end - describe ".git_shallow_clone?" do - subject { described_class.git_shallow_clone? } - - it { is_expected.to be_falsey } - end - context "with git folder" do include_context "with git fixture", "gitdir_with_commit" @@ -353,4 +347,28 @@ def with_shallow_clone_git_dir end end end + + context "with full clone" do + let(:tmpdir) { Dir.mktmpdir } + after { FileUtils.remove_entry(tmpdir) } + + before do + # shallow clone datadog-ci-rb repository + `cd #{tmpdir} && git clone https://github.com/DataDog/datadog-ci-rb` + end + + def with_full_clone_git_dir + ClimateControl.modify("GIT_DIR" => File.join(tmpdir, "datadog-ci-rb/.git")) do + yield + end + end + + describe ".git_shallow_clone?" do + subject do + with_full_clone_git_dir { described_class.git_shallow_clone? } + end + + it { is_expected.to be_falsey } + end + end end