Commit 72a7febe authored by Stan Hu's avatar Stan Hu

Fix "Revspec not found" errors when viewing diffs in a forked project with submodules

Closes #1413
parent 9f443f42
......@@ -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`)
......
......@@ -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
......
......@@ -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/
......
......@@ -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
......
......@@ -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
......@@ -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(
......
......@@ -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
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