Commit b5b78c33 authored by Patrick Steinhardt's avatar Patrick Steinhardt

merge_request: Drop checks whether a squash is in progress

Originally, Gitaly has implemented squashes via a separate worktree of
the repository. This has changed though in v14.1, where Gitaly now uses
an in-memory implementation of squashes. The result of this is that it
is both faster (~4x) and less error-prone.

One side effect though is that it's not possible to tell anymore whether
a squash is in progress or not: there is no on-disk state anymore which
Gitaly could check. Because of this, Gitaly has deprecated the RPC and
will eventually remove it.

Remove this check on GitLab's side to adapt to this change: it doesn't
work anymore anyway. If it needs to resurface, the check would instead
need to be implemented in GitLab itself, where any ongoing squashes
would need to be recorded as part of the database.

Changelog: removed
parent 647312f4
......@@ -1835,13 +1835,6 @@ class MergeRequest < ApplicationRecord
Ability.allowed?(user, :push_code, source_project)
end
def squash_in_progress?
# The source project can be deleted
return false unless source_project
source_project.repository.squash_in_progress?(id)
end
def find_actual_head_pipeline
all_pipelines.for_sha_or_source_sha(diff_head_sha).first
end
......
......@@ -2,8 +2,6 @@
module MergeRequests
class SquashService < MergeRequests::BaseService
SquashInProgressError = Class.new(RuntimeError)
def execute
# If performing a squash would result in no change, then
# immediately return a success message without performing a squash
......@@ -13,14 +11,7 @@ module MergeRequests
return error(s_('MergeRequests|This project does not allow squashing commits when merge requests are accepted.')) if squash_forbidden?
if squash_in_progress?
return error(s_('MergeRequests|Squash task canceled: another squash is already in progress.'))
end
squash! || error(s_('MergeRequests|Failed to squash. Should be done manually.'))
rescue SquashInProgressError
error(s_('MergeRequests|An error occurred while checking whether another squash is in progress.'))
end
private
......@@ -35,14 +26,6 @@ module MergeRequests
false
end
def squash_in_progress?
merge_request.squash_in_progress?
rescue StandardError => e
log_error(exception: e, message: 'Failed to check squash in progress')
raise SquashInProgressError, e.message
end
def squash_forbidden?
target_project.squash_never?
end
......
......@@ -869,12 +869,6 @@ module Gitlab
end
end
def squash_in_progress?(squash_id)
wrapped_gitaly_errors do
gitaly_repository_client.squash_in_progress?(squash_id)
end
end
def bundle_to_disk(save_path)
wrapped_gitaly_errors do
gitaly_repository_client.create_bundle(save_path)
......
......@@ -155,23 +155,6 @@ module Gitlab
)
end
def squash_in_progress?(squash_id)
request = Gitaly::IsSquashInProgressRequest.new(
repository: @gitaly_repo,
squash_id: squash_id.to_s
)
response = GitalyClient.call(
@storage,
:repository_service,
:is_squash_in_progress,
request,
timeout: GitalyClient.fast_timeout
)
response.in_progress
end
def fetch_source_branch(source_repository, source_branch, local_ref)
request = Gitaly::FetchSourceBranchRequest.new(
repository: @gitaly_repo,
......
......@@ -21063,9 +21063,6 @@ msgstr ""
msgid "MergeRequestDiffs|Select comment starting line"
msgstr ""
msgid "MergeRequests|An error occurred while checking whether another squash is in progress."
msgstr ""
msgid "MergeRequests|An error occurred while saving the draft comment."
msgstr ""
......@@ -21078,9 +21075,6 @@ msgstr ""
msgid "MergeRequests|Saving the comment failed"
msgstr ""
msgid "MergeRequests|Squash task canceled: another squash is already in progress."
msgstr ""
msgid "MergeRequests|This project does not allow squashing commits when merge requests are accepted."
msgstr ""
......
......@@ -195,19 +195,6 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
end
end
describe '#squash_in_progress?' do
let(:squash_id) { 1 }
it 'sends a repository_squash_in_progress message' do
expect_any_instance_of(Gitaly::RepositoryService::Stub)
.to receive(:is_squash_in_progress)
.with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
.and_return(double(in_progress: true))
client.squash_in_progress?(squash_id)
end
end
describe '#calculate_checksum' do
it 'sends a calculate_checksum message' do
expect_any_instance_of(Gitaly::RepositoryService::Stub)
......
......@@ -151,43 +151,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
end
end
describe '#squash_in_progress?' do
let(:repo_path) do
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
subject.source_project.repository.path
end
end
let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") }
before do
system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master))
end
it 'returns true when there is a current squash directory' do
expect(subject.squash_in_progress?).to be_truthy
end
it 'returns false when there is no squash directory' do
FileUtils.rm_rf(squash_path)
expect(subject.squash_in_progress?).to be_falsey
end
it 'returns false when the squash directory has expired' do
time = 20.minutes.ago.to_time
File.utime(time, time, squash_path)
expect(subject.squash_in_progress?).to be_falsey
end
it 'returns false when the source project has been removed' do
allow(subject).to receive(:source_project).and_return(nil)
expect(subject.squash_in_progress?).to be_falsey
end
end
describe '#squash?' do
let(:merge_request) { build(:merge_request, squash: squash) }
......
......@@ -402,21 +402,6 @@ RSpec.describe MergeRequests::MergeService do
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
it 'logs and saves error if there is a squash in progress' do
error_message = 'another squash is already in progress'
allow_any_instance_of(MergeRequest).to receive(:squash_in_progress?).and_return(true)
merge_request.update!(squash: true)
service.execute(merge_request)
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
it 'logs and saves error if there is an PreReceiveError exception' do
error_message = 'error message'
......
......@@ -194,23 +194,6 @@ RSpec.describe MergeRequests::SquashService do
expect(service.execute).to match(status: :error, message: a_string_including('squash'))
end
end
context 'with an error in squash in progress check' do
before do
allow(repository).to receive(:squash_in_progress?)
.and_raise(Gitlab::Git::Repository::GitError, error)
end
it 'logs the stage and output' do
expect(service).to receive(:log_error).with(exception: an_instance_of(Gitlab::Git::Repository::GitError), message: 'Failed to check squash in progress')
service.execute
end
it 'returns an error' do
expect(service.execute).to match(status: :error, message: 'An error occurred while checking whether another squash is in progress.')
end
end
end
context 'when any other exception is thrown' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment