diff options
author | Stan Hu <stanhu@gmail.com> | 2015-04-13 08:27:45 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2015-04-28 06:42:38 +0300 |
commit | 72a7febeada2c58c98caee8bb7ce18886a7c0868 (patch) | |
tree | 22cde467a3b87e93babb526b154003ffad12fef6 | |
parent | 9f443f42578f8f995415f3d0b9aa9ee8aebeff0b (diff) |
Fix "Revspec not found" errors when viewing diffs in a forked project with submodules
Closes #1413
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/helpers/diff_helper.rb | 4 | ||||
-rw-r--r-- | app/helpers/submodule_helper.rb | 4 | ||||
-rw-r--r-- | app/views/projects/diffs/_file.html.haml | 2 | ||||
-rw-r--r-- | spec/controllers/merge_requests_controller_spec.rb | 20 | ||||
-rw-r--r-- | spec/factories/projects.rb | 8 | ||||
-rw-r--r-- | spec/support/test_env.rb | 53 |
7 files changed, 80 insertions, 12 deletions
diff --git a/CHANGELOG b/CHANGELOG index 97a469513c8..a6bfedf283a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 7.11.0 (unreleased) - Fix "Cannot move project" error message from popping up after a successful transfer (Stan Hu) - Redirect to sign in page after signing out. - Fix "Hello @username." references not working by no longer allowing usernames to end in period. + - Fix "Revspec not found" errors when viewing diffs in a forked project with submodules (Stan Hu) - - Fix broken file browsing with relative submodule in personal projects (Stan Hu) - Add "Reply quoting selected text" shortcut key (`r`) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 4f42972a4dd..162778ade58 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -140,8 +140,8 @@ module DiffHelper end end - def submodule_link(blob, ref) - tree, commit = submodule_links(blob, ref) + def submodule_link(blob, ref, repository = @repository) + tree, commit = submodule_links(blob, ref, repository) commit_id = if commit.nil? blob.id[0..10] else diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index e13d4eaf101..6def7793dc3 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -2,8 +2,8 @@ module SubmoduleHelper include Gitlab::ShellAdapter # links to files listing for submodule if submodule is a project on this server - def submodule_links(submodule_item, ref = nil) - url = @repository.submodule_url_for(ref, submodule_item.path) + def submodule_links(submodule_item, ref = nil, repository = @repository) + url = repository.submodule_url_for(ref, submodule_item.path) return url, nil unless url =~ /([^\/:]+)\/([^\/]+\.git)\Z/ diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 672a6635321..d4b019780f5 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -11,7 +11,7 @@ = view_file_btn(@commit.parent_id, diff_file, project) - elsif diff_file.diff.submodule? - submodule_item = project.repository.blob_at(@commit.id, diff_file.file_path) - = submodule_link(submodule_item, @commit.id) + = submodule_link(submodule_item, @commit.id, project.repository) - else %span - if diff_file.renamed_file diff --git a/spec/controllers/merge_requests_controller_spec.rb b/spec/controllers/merge_requests_controller_spec.rb index d6f56ed33d6..c94ef1629ae 100644 --- a/spec/controllers/merge_requests_controller_spec.rb +++ b/spec/controllers/merge_requests_controller_spec.rb @@ -78,4 +78,24 @@ describe Projects::MergeRequestsController do end end end + + context '#diffs with forked projects with submodules' do + render_views + let(:project) { create(:project) } + let(:fork_project) { create(:forked_project_with_submodules) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: fork_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) } + + before do + fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) + fork_project.save + merge_request.reload + end + + it '#diffs' do + get(:diffs, namespace_id: project.namespace.to_param, + project_id: project.to_param, id: merge_request.iid, format: 'json') + expect(response).to be_success + expect(response.body).to have_content('Subproject commit') + end + end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 0899a7603fc..57fa079d753 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -76,6 +76,14 @@ FactoryGirl.define do end end + factory :forked_project_with_submodules, parent: :empty_project do + path { 'forked-gitlabhq' } + + after :create do |project| + TestEnv.copy_forked_repo_with_submodules(project) + end + end + factory :redmine_project, parent: :project do after :create do |project| project.create_redmine_service( diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 44d70e741b2..d1844bd2b87 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -14,6 +14,10 @@ module TestEnv 'master' => '5937ac0' } + FORKED_BRANCH_SHA = BRANCH_SHA.merge({ + 'add-submodule-version-bump' => '3f547c08' + }) + # Test environment # # See gitlab.yml.example test section for paths @@ -31,6 +35,8 @@ module TestEnv # Create repository for FactoryGirl.create(:project) setup_factory_repo + # Create repository for FactoryGirl.create(:forked_project_with_submodules) + setup_forked_repo end def disable_mailer @@ -61,14 +67,26 @@ module TestEnv end def setup_factory_repo - clone_url = "https://gitlab.com/gitlab-org/#{factory_repo_name}.git" + setup_repo(factory_repo_path, factory_repo_path_bare, factory_repo_name, + BRANCH_SHA) + end + + # This repo has a submodule commit that is not present in the main test + # repository. + def setup_forked_repo + setup_repo(forked_repo_path, forked_repo_path_bare, forked_repo_name, + FORKED_BRANCH_SHA) + end + + def setup_repo(repo_path, repo_path_bare, repo_name, branch_sha) + clone_url = "https://gitlab.com/gitlab-org/#{repo_name}.git" - unless File.directory?(factory_repo_path) - system(*%W(git clone -q #{clone_url} #{factory_repo_path})) + unless File.directory?(repo_path) + system(*%W(git clone -q #{clone_url} #{repo_path})) end - Dir.chdir(factory_repo_path) do - BRANCH_SHA.each do |branch, sha| + Dir.chdir(repo_path) do + branch_sha.each do |branch, sha| # Try to reset without fetching to avoid using the network. reset = %W(git update-ref refs/heads/#{branch} #{sha}) unless system(*reset) @@ -85,7 +103,7 @@ module TestEnv end # We must copy bare repositories because we will push to them. - system(git_env, *%W(git clone -q --bare #{factory_repo_path} #{factory_repo_path_bare})) + system(git_env, *%W(git clone -q --bare #{repo_path} #{repo_path_bare})) end def copy_repo(project) @@ -100,6 +118,14 @@ module TestEnv Gitlab.config.gitlab_shell.repos_path end + def copy_forked_repo_with_submodules(project) + base_repo_path = File.expand_path(forked_repo_path_bare) + target_repo_path = File.expand_path(repos_path + "/#{project.namespace.path}/#{project.path}.git") + FileUtils.mkdir_p(target_repo_path) + FileUtils.cp_r("#{base_repo_path}/.", target_repo_path) + FileUtils.chmod_R 0755, target_repo_path + end + private def factory_repo_path @@ -114,9 +140,22 @@ module TestEnv 'gitlab-test' end + def forked_repo_path + @forked_repo_path ||= Rails.root.join('tmp', 'tests', forked_repo_name) + end + + def forked_repo_path_bare + "#{forked_repo_path}_bare" + end + + def forked_repo_name + 'gitlab-test-fork' + end + + # Prevent developer git configurations from being persisted to test # repositories def git_env - {'GIT_TEMPLATE_DIR' => ''} + { 'GIT_TEMPLATE_DIR' => '' } end end |