Use `git update-ref --stdin -z` to delete refs36807-gc-unwanted-refs-after-import

5 files changed, 53 insertions, 5 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 5758aacd57f..d29d2a83708 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1246,6 +1246,6 @@ class Repository
 
     yield commit(sha)
   ensure
-    rugged.references.delete(tmp_ref) if tmp_ref
+    delete_refs(tmp_ref) if tmp_ref
   end
 end
diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb
index 107856885f3..e6a68d983ef 100644
--- a/app/services/projects/after_import_service.rb
+++ b/app/services/projects/after_import_service.rb
@@ -9,7 +9,7 @@ module Projects
 
     def execute
       Projects::HousekeepingService.new(@project).execute do
-        repository.delete_refs(garbage_refs)
+        repository.delete_refs(*garbage_refs)
       end
     rescue Projects::HousekeepingService::LeaseTaken => e
       Rails.logger.info(
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 8b39e92cc65..fb6504bdea0 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -17,6 +17,7 @@ module Gitlab
       NoRepository = Class.new(StandardError)
       InvalidBlobName = Class.new(StandardError)
       InvalidRef = Class.new(StandardError)
+      GitError = Class.new(StandardError)
 
       class << self
         # Unlike `new`, `create` takes the storage path, not the storage name
@@ -598,8 +599,21 @@ module Gitlab
         rugged.branches.delete(branch_name)
       end
 
-      def delete_refs(ref_names)
-        ref_names.each { |ref| rugged.references.delete(ref) }
+      def delete_refs(*ref_names)
+        instructions = ref_names.map do |ref|
+          "delete #{ref}\x00\x00"
+        end
+
+        command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
+        message, status = Gitlab::Popen.popen(
+          command,
+          path) do |stdin|
+          stdin.write(instructions.join)
+        end
+
+        unless status.zero?
+          raise GitError.new("Could not delete refs #{ref_names}: #{message}")
+        end
       end
 
       # Create a new branch named **ref+ based on **stat_point+, HEAD by default
diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake
index f76bef5f4bf..8ae1b6a626a 100644
--- a/lib/tasks/gitlab/cleanup.rake
+++ b/lib/tasks/gitlab/cleanup.rake
@@ -111,7 +111,7 @@ namespace :gitlab do
           next unless id > max_iid
 
           project.deployments.find(id).create_ref
-          rugged.references.delete(ref)
+          project.repository.delete_refs(ref)
         end
       end
     end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 6b9773c9b63..4cfb4b7d357 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -433,6 +433,40 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#delete_refs' do
+    before(:all) do
+      @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
+    end
+
+    it 'deletes the ref' do
+      @repo.delete_refs('refs/heads/feature')
+
+      expect(@repo.rugged.references['refs/heads/feature']).to be_nil
+    end
+
+    it 'deletes all refs' do
+      refs = %w[refs/heads/wip refs/tags/v1.1.0]
+      @repo.delete_refs(*refs)
+
+      refs.each do |ref|
+        expect(@repo.rugged.references[ref]).to be_nil
+      end
+    end
+
+    it 'raises an error if it failed' do
+      expect(Gitlab::Popen).to receive(:popen).and_return(['Error', 1])
+
+      expect do
+        @repo.delete_refs('refs/heads/fix')
+      end.to raise_error(Gitlab::Git::Repository::GitError)
+    end
+
+    after(:all) do
+      FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
+      ensure_seeds
+    end
+  end
+
   describe "#refs_hash" do
     let(:refs) { repository.refs_hash }