From 8c2b1f250e0447547b2972fab6be7ba5304788e9 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 31 Oct 2016 14:09:39 +0000 Subject: Merge branch '22271-drone-tag-pipeline-build' into 'master' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix lightweight tags not processed correctly by GitTagPushService Closes #22271 See merge request !6532 Signed-off-by: Rémy Coutable --- CHANGELOG.md | 1 + Gemfile | 2 +- Gemfile.lock | 6 +- app/controllers/projects/tags_controller.rb | 2 +- app/models/repository.rb | 24 +-- app/services/delete_branch_service.rb | 2 +- app/services/delete_tag_service.rb | 2 +- app/services/git_tag_push_service.rb | 4 +- app/views/projects/branches/_branch.html.haml | 2 +- .../projects/issues/_related_branches.html.haml | 2 +- app/views/projects/tags/_tag.html.haml | 2 +- lib/api/entities.rb | 4 +- lib/gitlab/data_builder/push.rb | 2 +- spec/finders/branches_finder_spec.rb | 2 +- spec/finders/tags_finder_spec.rb | 2 +- spec/models/repository_spec.rb | 14 +- spec/services/git_tag_push_service_spec.rb | 173 +++++++++++++++------ .../issues/_related_branches.html.haml_spec.rb | 2 +- 18 files changed, 161 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c7c57012e..f4cfa363b63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Honour issue and merge request visibility in their respective finders. - Disable reference Markdown for unavailable features. +- Fix lightweight tags not processed correctly by GitTagPushService. !6532 - Allow owners to fetch source code in CI builds. !6943 - Reduce the overhead to calculate number of open/closed issues and merge requests within the group or project. !7123 - Fix builds tab visibility. !7178 diff --git a/Gemfile b/Gemfile index 46245ab62d1..5a95fdc82a7 100644 --- a/Gemfile +++ b/Gemfile @@ -51,7 +51,7 @@ gem 'browser', '~> 2.2' # Extracting information from a git repository # Provide access to Gitlab::Git library -gem 'gitlab_git', '~> 10.6.8' +gem 'gitlab_git', '~> 10.7.0' # LDAP Auth # GitLab fork with several improvements to original library. For full list of changes diff --git a/Gemfile.lock b/Gemfile.lock index 442184b9228..26c4dc0d23f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -283,7 +283,7 @@ GEM mime-types (>= 1.16, < 3) posix-spawn (~> 0.3) gitlab-markup (1.5.0) - gitlab_git (10.6.8) + gitlab_git (10.7.0) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) @@ -867,7 +867,7 @@ DEPENDENCIES github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.0) - gitlab_git (~> 10.6.8) + gitlab_git (~> 10.7.0) gitlab_omniauth-ldap (~> 1.2.1) gollum-lib (~> 4.2) gollum-rugged_adapter (~> 0.4.2) @@ -994,4 +994,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.13.2 + 1.13.5 diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 8fea20cefef..953091492ae 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -23,7 +23,7 @@ class Projects::TagsController < Projects::ApplicationController return render_404 unless @tag @release = @project.releases.find_or_initialize_by(tag: @tag.name) - @commit = @repository.commit(@tag.target) + @commit = @repository.commit(@tag.dereferenced_target) end def create diff --git a/app/models/repository.rb b/app/models/repository.rb index b6653d18530..fe4b73e9aa4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -178,7 +178,7 @@ class Repository before_remove_branch branch = find_branch(branch_name) - oldrev = branch.try(:target).try(:id) + oldrev = branch.try(:dereferenced_target).try(:id) newrev = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name @@ -294,10 +294,10 @@ class Repository # Rugged seems to throw a `ReferenceError` when given branch_names rather # than SHA-1 hashes number_commits_behind = raw_repository. - count_commits_between(branch.target.sha, root_ref_hash) + count_commits_between(branch.dereferenced_target.sha, root_ref_hash) number_commits_ahead = raw_repository. - count_commits_between(root_ref_hash, branch.target.sha) + count_commits_between(root_ref_hash, branch.dereferenced_target.sha) { behind: number_commits_behind, ahead: number_commits_ahead } end @@ -679,11 +679,11 @@ class Repository branches.sort_by(&:name) when 'updated_desc' branches.sort do |a, b| - commit(b.target).committed_date <=> commit(a.target).committed_date + commit(b.dereferenced_target).committed_date <=> commit(a.dereferenced_target).committed_date end when 'updated_asc' branches.sort do |a, b| - commit(a.target).committed_date <=> commit(b.target).committed_date + commit(a.dereferenced_target).committed_date <=> commit(b.dereferenced_target).committed_date end else branches @@ -858,7 +858,7 @@ class Repository branch = find_branch(ref) if branch - last_commit = branch.target + last_commit = branch.dereferenced_target index.read_tree(last_commit.raw_commit.tree) parents = [last_commit.sha] end @@ -945,7 +945,7 @@ class Repository end def revert(user, commit, base_branch, revert_tree_id = nil) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha revert_tree_id ||= check_revert_content(commit, base_branch) return false unless revert_tree_id @@ -962,7 +962,7 @@ class Repository end def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) return false unless cherry_pick_tree_id @@ -991,7 +991,7 @@ class Repository end def check_revert_content(commit, base_branch) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha args = [commit.id, source_sha] args << { mainline: 1 } if commit.merge_commit? @@ -1005,7 +1005,7 @@ class Repository end def check_cherry_pick_content(commit, base_branch) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha args = [commit.id, source_sha] args << 1 if commit.merge_commit? @@ -1078,7 +1078,7 @@ class Repository if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil? oldrev = Gitlab::Git::BLANK_SHA else - oldrev = rugged.merge_base(newrev, target_branch.target.sha) + oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha) end GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do @@ -1138,7 +1138,7 @@ class Repository end def tags_sorted_by_committed_date - tags.sort_by { |tag| tag.target.committed_date } + tags.sort_by { |tag| tag.dereferenced_target.committed_date } end def keep_around_ref_name(sha) diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 918eddaa53a..3e5dd4ebb86 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -42,7 +42,7 @@ class DeleteBranchService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - branch.target.sha, + branch.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index d0cb151a010..d824406cb49 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -36,7 +36,7 @@ class DeleteTagService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - tag.target.sha, + tag.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index e6002b03b93..20a4445bddf 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -27,8 +27,8 @@ class GitTagPushService < BaseService tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) - if tag && tag.object_sha == params[:newrev] - commit = project.commit(tag.target) + if tag && tag.target == params[:newrev] + commit = project.commit(tag.dereferenced_target) commits = [commit].compact message = tag.message end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 4480b2f22c3..ed30ca7fbd4 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,4 +1,4 @@ -- commit = @repository.commit(branch.target) +- commit = @repository.commit(branch.dereferenced_target) - bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 - diverging_commit_counts = @repository.diverging_commit_counts(branch) - number_commits_behind = diverging_commit_counts[:behind] diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml index 44683c8bcdb..1892ebb512f 100644 --- a/app/views/projects/issues/_related_branches.html.haml +++ b/app/views/projects/issues/_related_branches.html.haml @@ -4,7 +4,7 @@ %ul.unstyled-list.related-merge-requests - @related_branches.each do |branch| %li - - target = @project.repository.find_branch(branch).target + - target = @project.repository.find_branch(branch).dereferenced_target - pipeline = @project.pipeline_for(branch, target.sha) if target - if pipeline %span.related-branch-ci-status diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml index 05fccb4f976..c42641afea0 100644 --- a/app/views/projects/tags/_tag.html.haml +++ b/app/views/projects/tags/_tag.html.haml @@ -1,4 +1,4 @@ -- commit = @repository.commit(tag.target) +- commit = @repository.commit(tag.dereferenced_target) - release = @releases.find { |release| release.tag == tag.name } %li %div diff --git a/lib/api/entities.rb b/lib/api/entities.rb index feaa0c213bf..ab9d2d54f4b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -138,7 +138,7 @@ module API expose :name expose :commit do |repo_branch, options| - options[:project].repository.commit(repo_branch.target) + options[:project].repository.commit(repo_branch.dereferenced_target) end expose :protected do |repo_branch, options| @@ -523,7 +523,7 @@ module API expose :name, :message expose :commit do |repo_tag, options| - options[:project].repository.commit(repo_tag.target) + options[:project].repository.commit(repo_tag.dereferenced_target) end expose :release, using: Entities::Release do |repo_tag, options| diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index 4f81863da35..d76aa38f741 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -83,7 +83,7 @@ module Gitlab tag = repository.find_tag(tag_name) if tag - commit = repository.commit(tag.target) + commit = repository.commit(tag.dereferenced_target) commit.try(:sha) end else diff --git a/spec/finders/branches_finder_spec.rb b/spec/finders/branches_finder_spec.rb index 6fce11de30f..db60c01db0d 100644 --- a/spec/finders/branches_finder_spec.rb +++ b/spec/finders/branches_finder_spec.rb @@ -21,7 +21,7 @@ describe BranchesFinder do result = branches_finder.execute recently_updated_branch = repository.branches.max do |a, b| - repository.commit(a.target).committed_date <=> repository.commit(b.target).committed_date + repository.commit(a.dereferenced_target).committed_date <=> repository.commit(b.dereferenced_target).committed_date end expect(result.first.name).to eq(recently_updated_branch.name) diff --git a/spec/finders/tags_finder_spec.rb b/spec/finders/tags_finder_spec.rb index 2ac810e478a..98b42e264dc 100644 --- a/spec/finders/tags_finder_spec.rb +++ b/spec/finders/tags_finder_spec.rb @@ -20,7 +20,7 @@ describe TagsFinder do result = tags_finder.execute recently_updated_tag = repository.tags.max do |a, b| - repository.commit(a.target).committed_date <=> repository.commit(b.target).committed_date + repository.commit(a.dereferenced_target).committed_date <=> repository.commit(b.dereferenced_target).committed_date end expect(result.first.name).to eq(recently_updated_tag.name) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 187a1bf2d79..19b3e7e470d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -68,8 +68,8 @@ describe Repository, models: true do double_first = double(committed_date: Time.now) double_last = double(committed_date: Time.now - 1.second) - allow(tag_a).to receive(:target).and_return(double_first) - allow(tag_b).to receive(:target).and_return(double_last) + allow(tag_a).to receive(:dereferenced_target).and_return(double_first) + allow(tag_b).to receive(:dereferenced_target).and_return(double_last) allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end @@ -83,8 +83,8 @@ describe Repository, models: true do double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) - allow(tag_a).to receive(:target).and_return(double_last) - allow(tag_b).to receive(:target).and_return(double_first) + allow(tag_a).to receive(:dereferenced_target).and_return(double_last) + allow(tag_b).to receive(:dereferenced_target).and_return(double_first) allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end @@ -632,9 +632,9 @@ describe Repository, models: true do context "when the branch wasn't empty" do it 'updates the head' do - expect(repository.find_branch('feature').target.id).to eq(old_rev) + expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) repository.update_branch_with_hooks(user, 'feature') { new_rev } - expect(repository.find_branch('feature').target.id).to eq(new_rev) + expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) end end end @@ -659,7 +659,7 @@ describe Repository, models: true do context 'when the update would remove commits from the target branch' do it 'raises an exception' do branch = 'master' - old_rev = repository.find_branch(branch).target.sha + old_rev = repository.find_branch(branch).dereferenced_target.sha # The 'master' branch is NOT an ancestor of new_rev. expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev) diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index a4fcd44882d..0879e3ab4c8 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -37,65 +37,138 @@ describe GitTagPushService, services: true do end describe "Git Tag Push Data" do - before do - service.execute - @push_data = service.push_data - @tag_name = Gitlab::Git.ref_name(ref) - @tag = project.repository.find_tag(@tag_name) - @commit = project.commit(@tag.target) - end - subject { @push_data } + let(:tag) { project.repository.find_tag(tag_name) } + let(:commit) { tag.dereferenced_target } - it { is_expected.to include(object_kind: 'tag_push') } - it { is_expected.to include(ref: ref) } - it { is_expected.to include(before: oldrev) } - it { is_expected.to include(after: newrev) } - it { is_expected.to include(message: @tag.message) } - it { is_expected.to include(user_id: user.id) } - it { is_expected.to include(user_name: user.name) } - it { is_expected.to include(project_id: project.id) } - - context "with repository data" do - subject { @push_data[:repository] } - - it { is_expected.to include(name: project.name) } - it { is_expected.to include(url: project.url_to_repo) } - it { is_expected.to include(description: project.description) } - it { is_expected.to include(homepage: project.web_url) } - end + context 'annotated tag' do + let(:tag_name) { Gitlab::Git.ref_name(ref) } - context "with commits" do - subject { @push_data[:commits] } + before do + service.execute + @push_data = service.push_data + end - it { is_expected.to be_an(Array) } - it 'has 1 element' do - expect(subject.size).to eq(1) + it { is_expected.to include(object_kind: 'tag_push') } + it { is_expected.to include(ref: ref) } + it { is_expected.to include(before: oldrev) } + it { is_expected.to include(after: newrev) } + it { is_expected.to include(message: tag.message) } + it { is_expected.to include(user_id: user.id) } + it { is_expected.to include(user_name: user.name) } + it { is_expected.to include(project_id: project.id) } + + context "with repository data" do + subject { @push_data[:repository] } + + it { is_expected.to include(name: project.name) } + it { is_expected.to include(url: project.url_to_repo) } + it { is_expected.to include(description: project.description) } + it { is_expected.to include(homepage: project.web_url) } end - context "the commit" do - subject { @push_data[:commits].first } - - it { is_expected.to include(id: @commit.id) } - it { is_expected.to include(message: @commit.safe_message) } - it { is_expected.to include(timestamp: @commit.date.xmlschema) } - it do - is_expected.to include( - url: [ - Gitlab.config.gitlab.url, - project.namespace.to_param, - project.to_param, - 'commit', - @commit.id - ].join('/') - ) + context "with commits" do + subject { @push_data[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) + end + + context "the commit" do + subject { @push_data[:commits].first } + + it { is_expected.to include(id: commit.id) } + it { is_expected.to include(message: commit.safe_message) } + it { is_expected.to include(timestamp: commit.date.xmlschema) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + commit.id + ].join('/') + ) + end + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { is_expected.to include(name: commit.author_name) } + it { is_expected.to include(email: commit.author_email) } + end end + end + end - context "with a author" do - subject { @push_data[:commits].first[:author] } + context 'lightweight tag' do + let(:tag_name) { 'light-tag' } + let(:newrev) { '5937ac0a7beb003549fc5fd26fc247adbce4a52e' } + let(:ref) { "refs/tags/light-tag" } + + before do + # Create the lightweight tag + project.repository.raw_repository.rugged.tags.create(tag_name, newrev) + + # Clear tag list cache + project.repository.expire_tags_cache + + service.execute + @push_data = service.push_data + end + + it { is_expected.to include(object_kind: 'tag_push') } + it { is_expected.to include(ref: ref) } + it { is_expected.to include(before: oldrev) } + it { is_expected.to include(after: newrev) } + it { is_expected.to include(message: tag.message) } + it { is_expected.to include(user_id: user.id) } + it { is_expected.to include(user_name: user.name) } + it { is_expected.to include(project_id: project.id) } + + context "with repository data" do + subject { @push_data[:repository] } + + it { is_expected.to include(name: project.name) } + it { is_expected.to include(url: project.url_to_repo) } + it { is_expected.to include(description: project.description) } + it { is_expected.to include(homepage: project.web_url) } + end + + context "with commits" do + subject { @push_data[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) + end - it { is_expected.to include(name: @commit.author_name) } - it { is_expected.to include(email: @commit.author_email) } + context "the commit" do + subject { @push_data[:commits].first } + + it { is_expected.to include(id: commit.id) } + it { is_expected.to include(message: commit.safe_message) } + it { is_expected.to include(timestamp: commit.date.xmlschema) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + commit.id + ].join('/') + ) + end + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { is_expected.to include(name: commit.author_name) } + it { is_expected.to include(email: commit.author_email) } + end end end end diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb index c8a3d02d8fd..889d9a38887 100644 --- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb +++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb @@ -5,7 +5,7 @@ describe 'projects/issues/_related_branches' do let(:project) { create(:project) } let(:branch) { project.repository.find_branch('feature') } - let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.target.id, ref: 'feature') } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.dereferenced_target.id, ref: 'feature') } before do assign(:project, project) -- cgit v1.2.3