diff options
| -rw-r--r-- | app/models/concerns/repository_mirroring.rb | 31 | ||||
| -rw-r--r-- | app/models/merge_request_diff.rb | 2 | ||||
| -rw-r--r-- | lib/github/import.rb | 5 | ||||
| -rw-r--r-- | lib/github/representation/pull_request.rb | 46 |
4 files changed, 46 insertions, 38 deletions
diff --git a/app/models/concerns/repository_mirroring.rb b/app/models/concerns/repository_mirroring.rb index 5758c52297f..6d84d8dc342 100644 --- a/app/models/concerns/repository_mirroring.rb +++ b/app/models/concerns/repository_mirroring.rb @@ -1,36 +1,39 @@ module RepositoryMirroring - IMPORT_REFS = %w[+refs/pull/*/head:refs/merge-requests/*/head +refs/heads/*:refs/heads/* +refs/tags/*:refs/tags/*].freeze + IMPORT_REFS = %w[ + +refs/heads/*:refs/heads/* + +refs/tags/*:refs/tags/* + ].freeze def set_remote_as_mirror(name) # This is used to define repository as equivalent as "git clone --mirror" - config["remote.#{name}.fetch"] = 'refs/*:refs/*' - config["remote.#{name}.mirror"] = true - config["remote.#{name}.prune"] = true + raw_repository.rugged.config["remote.#{name}.fetch"] = 'refs/*:refs/*' + raw_repository.rugged.config["remote.#{name}.mirror"] = true + raw_repository.rugged.config["remote.#{name}.prune"] = true end - def set_import_remote_as_mirror(name) + def set_import_remote_as_mirror(remote_name) # Add first fetch with Rugged so it does not create its own. - config["remote.#{name}.fetch"] = IMPORT_REFS.first + raw_repository.rugged.config["remote.#{remote_name}.fetch"] = IMPORT_REFS.first - IMPORT_REFS.drop(1).each do |ref| - run_git(%W[config --add remote.#{name}.fetch #{ref}]) + IMPORT_REFS.drop(1).each do |refspec| + add_remote_fetch_config(remote_name, refspec) end - config["remote.#{name}.mirror"] = true - config["remote.#{name}.prune"] = true + raw_repository.rugged.config["remote.#{remote_name}.mirror"] = true + raw_repository.rugged.config["remote.#{remote_name}.prune"] = true rescue Rugged::ConfigError # Ignore multivar errors when the config already exist # TODO: refactor/fix this end + def add_remote_fetch_config(remote_name, refspec) + run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}]) + end + def fetch_mirror(remote, url) add_remote(remote, url) set_remote_as_mirror(remote) fetch_remote(remote, forced: true) remove_remote(remote) end - - def config - raw_repository.rugged.config - end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index cd20df8404a..68c87210772 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -55,7 +55,7 @@ class MergeRequestDiff < ActiveRecord::Base end def ensure_commit_shas - merge_request.fetch_ref unless merge_request.source_branch_head + merge_request.fetch_ref self.start_commit_sha ||= merge_request.target_branch_sha self.head_commit_sha ||= merge_request.source_branch_sha diff --git a/lib/github/import.rb b/lib/github/import.rb index 47ed9acadc4..2a7a50ecc01 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -57,6 +57,7 @@ module Github project.ensure_repository project.repository.add_remote('github', repo_url) project.repository.set_import_remote_as_mirror('github') + project.repository.add_remote_fetch_config('github', '+refs/pull/*/head:refs/merge-requests/*/head') project.repository.fetch_remote('github', forced: true) rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error => e error(:project, repo_url, e.message) @@ -140,7 +141,7 @@ module Github response.body.each do |raw| pull_request = Github::Representation::PullRequest.new(raw, options.merge(project: project)) merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) - next unless merge_request.new_record? && pull_request.valid? && merge_request.source_branch_head + next unless merge_request.new_record? && pull_request.valid? begin pull_request.restore_branches! @@ -173,6 +174,8 @@ module Github fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote) rescue => e error(:pull_request, pull_request.url, e.message) + ensure + pull_request.remove_tmp_branches! end end diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb index ec030ade38a..684cf54e154 100644 --- a/lib/github/representation/pull_request.rb +++ b/lib/github/representation/pull_request.rb @@ -9,20 +9,13 @@ module Github end def source_branch_name - @source_branch_name ||= - if opened? && project.repository.branch_exists?(source_branch.ref) - source_branch.ref - elsif cross_project? && opened? - source_branch_name_prefixed - else - source_branch_ref - end + @source_branch_name ||= source_branch_exists? ? source_branch_ref : tmp_source_branch end def source_branch_exists? return @source_branch_exists if defined?(@source_branch_exists) - @source_branch_exists = project.repository.branch_exists?(source_branch_name) + @source_branch_exists = !cross_project? && source_branch.exists? end def target_project @@ -30,7 +23,7 @@ module Github end def target_branch_name - @target_branch_name ||= target_branch_exists? ? target_branch_ref : target_branch_name_prefixed + @target_branch_name ||= target_branch_exists? ? target_branch_ref : tmp_target_branch end def target_branch_exists? @@ -57,6 +50,13 @@ module Github restore_target_branch! end + def remove_tmp_branches! + return if opened? + + remove_tmp_source_branch! + remove_tmp_target_branch! + end + private def project @@ -67,22 +67,10 @@ module Github @source_branch ||= Representation::Branch.new(raw['head'], repository: project.repository) end - def source_branch_ref - "refs/merge-requests/#{iid}/head" - end - - def source_branch_name_prefixed - "gh-#{target_branch_short_sha}/#{iid}/#{source_branch_user}/#{source_branch.ref}" - end - def target_branch @target_branch ||= Representation::Branch.new(raw['base'], repository: project.repository) end - def target_branch_name_prefixed - "gl-#{target_branch_short_sha}/#{iid}/#{target_branch_user}/#{target_branch_ref}" - end - def cross_project? return true if source_branch_repo.nil? @@ -100,6 +88,20 @@ module Github target_branch.restore!(target_branch_name) end + + def remove_tmp_source_branch! + # We should remove the source/target branches only if they were + # restored. Otherwise, we'll remove branches like 'master' that + # target_branch_exists? returns true. In other words, we need + # to clean up only the restored branches that (source|target)_branch_exists? + # returns false for the first time it has been called, because of + # this that is important to memoize these values. + source_branch.remove!(source_branch_name) unless source_branch_exists? + end + + def remove_tmp_target_branch! + target_branch.remove!(target_branch_name) unless target_branch_exists? + end end end end |