From f4f8270c9fe5906e54e70aafadc375f7a2c869b0 Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Mon, 13 Aug 2018 12:01:59 +0800 Subject: [PATCH] Allow resolving git submodule url for subgroup projects using relative path --- app/helpers/submodule_helper.rb | 35 +++++---- .../37356-relative-submodule-link.yml | 5 ++ spec/helpers/submodule_helper_spec.rb | 75 ++++++++++++++----- 3 files changed, 79 insertions(+), 36 deletions(-) create mode 100644 changelogs/unreleased/37356-relative-submodule-link.yml diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index ebfde993456..ec2cf2b16c0 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -64,8 +64,7 @@ module SubmoduleHelper end def relative_self_url?(url) - # (./)?(../repo.git) || (./)?(../../project/repo.git) ) - url =~ %r{\A((\./)?(\.\./))(?!(\.\.)|(.*/)).*(\.git)?\z} || url =~ %r{\A((\./)?(\.\./){2})(?!(\.\.))([^/]*)/(?!(\.\.)|(.*/)).*(\.git)?\z} + url.start_with?('../', './') end def standard_links(host, namespace, project, commit) @@ -73,25 +72,29 @@ module SubmoduleHelper [base, [base, '/tree/', commit].join('')] end - def relative_self_links(url, commit, project) - url.rstrip! - # Map relative links to a namespace and project - # For example: - # ../bar.git -> same namespace, repo bar - # ../foo/bar.git -> namespace foo, repo bar - # ../../foo/bar/baz.git -> namespace bar, repo baz - components = url.split('/') - base = components.pop.gsub(/.git$/, '') - namespace = components.pop.gsub(/^\.\.$/, '') + def relative_self_links(relative_path, commit, project) + relative_path.rstrip! + absolute_project_path = "/" + project.full_path - if namespace.empty? - namespace = project.namespace.full_path + # Resolve `relative_path` to target path + # Assuming `absolute_project_path` is `/g1/p1`: + # ../p2.git -> /g1/p2 + # ../g2/p3.git -> /g1/g2/p3 + # ../../g3/g4/p4.git -> /g3/g4/p4 + submodule_project_path = File.absolute_path(relative_path, absolute_project_path) + target_namespace_path = File.dirname(submodule_project_path) + + if target_namespace_path == '/' || target_namespace_path.start_with?(absolute_project_path) + return [nil, nil] end + target_namespace_path.sub!(%r{^/}, '') + submodule_base = File.basename(submodule_project_path, '.git') + begin [ - namespace_project_path(namespace, base), - namespace_project_tree_path(namespace, base, commit) + namespace_project_path(target_namespace_path, submodule_base), + namespace_project_tree_path(target_namespace_path, submodule_base, commit) ] rescue ActionController::UrlGenerationError [nil, nil] diff --git a/changelogs/unreleased/37356-relative-submodule-link.yml b/changelogs/unreleased/37356-relative-submodule-link.yml new file mode 100644 index 00000000000..99d1577609d --- /dev/null +++ b/changelogs/unreleased/37356-relative-submodule-link.yml @@ -0,0 +1,5 @@ +--- +title: Fix git submodule link for subgroup projects with relative path +merge_request: 21154 +author: +type: fixed diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index a64f8a11ef2..8662cadc7a0 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -162,42 +162,77 @@ describe SubmoduleHelper do end context 'submodules with relative links' do - let(:group) { create(:group, name: "Master Project", path: "master-project") } + let(:group) { create(:group, name: "top group", path: "top-group") } let(:project) { create(:project, group: group) } - let(:commit_id) { sample_commit[:id] } + let(:repo) { double(:repo, project: project) } - it 'one level down' do - result = relative_self_links('../test.git', commit_id, project) - expect(result).to eq(["/#{group.path}/test", "/#{group.path}/test/tree/#{commit_id}"]) + def expect_relative_link_to_resolve_to(relative_path, expected_path) + allow(repo).to receive(:submodule_url_for).and_return(relative_path) + + result = submodule_links(submodule_item) + + expect(result).to eq([expected_path, "#{expected_path}/tree/#{submodule_item.id}"]) end - it 'with trailing whitespace' do - result = relative_self_links('../test.git ', commit_id, project) - expect(result).to eq(["/#{group.path}/test", "/#{group.path}/test/tree/#{commit_id}"]) + it 'handles project under same group' do + expect_relative_link_to_resolve_to('../test.git', "/#{group.path}/test") end - it 'two levels down' do - result = relative_self_links('../../test.git', commit_id, project) - expect(result).to eq(["/#{group.path}/test", "/#{group.path}/test/tree/#{commit_id}"]) + it 'handles trailing whitespace' do + expect_relative_link_to_resolve_to('../test.git ', "/#{group.path}/test") end - it 'one level down with namespace and repo' do - result = relative_self_links('../foobar/test.git', commit_id, project) - expect(result).to eq(["/foobar/test", "/foobar/test/tree/#{commit_id}"]) + it 'handles project under another top group' do + expect_relative_link_to_resolve_to('../../baz/test.git ', "/baz/test") end - it 'two levels down with namespace and repo' do - result = relative_self_links('../foobar/baz/test.git', commit_id, project) - expect(result).to eq(["/baz/test", "/baz/test/tree/#{commit_id}"]) + context 'repo path resolves to be located at root (namespace absent)' do + it 'returns nil' do + allow(repo).to receive(:submodule_url_for).and_return('../../test.git') + + result = submodule_links(submodule_item) + + expect(result).to eq([nil, nil]) + end + end + + context 'repo path resolves to be located underneath current project path' do + it 'returns nil because it is not possible to have repo nested under another repo' do + allow(repo).to receive(:submodule_url_for).and_return('./test.git') + + result = submodule_links(submodule_item) + + expect(result).to eq([nil, nil]) + end + end + + context 'subgroup' do + let(:sub_group) { create(:group, parent: group, name: "sub group", path: "sub-group") } + let(:sub_project) { create(:project, group: sub_group) } + + context 'project in sub group' do + let(:project) { sub_project } + + it "handles referencing ancestor group's project" do + expect_relative_link_to_resolve_to('../../../top-group/test.git', "/#{group.path}/test") + end + end + + it "handles referencing descendent group's project" do + expect_relative_link_to_resolve_to('../sub-group/test.git', "/top-group/sub-group/test") + end + + it "handles referencing another top group's project" do + expect_relative_link_to_resolve_to('../../frontend/css/test.git', "/frontend/css/test") + end end context 'personal project' do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } - it 'one level down with personal project' do - result = relative_self_links('../test.git', commit_id, project) - expect(result).to eq(["/#{user.username}/test", "/#{user.username}/test/tree/#{commit_id}"]) + it 'handles referencing another personal project' do + expect_relative_link_to_resolve_to('../test.git', "/#{user.username}/test") end end end