diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-12 20:28:21 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-12 20:28:21 +0000 |
commit | b956605f9e5f95ded853d3458f0f132dbb5b892a (patch) | |
tree | dd1596c99ab5766e8749f45acb1e5a0d22e1a4d1 | |
parent | 66dd61ef0f1f98f860bdef5aa2cdb1c6f2e1f83f (diff) | |
parent | b02c21df5cc0dcc795f71f8c11d05d61dc2ad897 (diff) | |
download | gitlab-ce-b956605f9e5f95ded853d3458f0f132dbb5b892a.tar.gz |
Merge branch 'ambiguous-sha' into 'master'fondev
Fix ambiguous sha problem with mentioned commit
Before: write in database only 6 chars of commit sha. This cause to `Ambiguous SHA1 prefix` exception.
- [x] write full commit sha in db.
- [x] Standardise usage of sha truncation: 8 characters everywhere.
- [x] prevent exception when ambiguous sha requested in markdown
Fixes #1644
See merge request !1171
21 files changed, 48 insertions, 27 deletions
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index cab2984a4c4..0e0532b65b2 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -120,4 +120,8 @@ module CommitsHelper class: 'commit-short-id') end end + + def truncate_sha(sha) + Commit.truncate_sha(sha) + end end diff --git a/app/models/commit.rb b/app/models/commit.rb index a1343b65c72..212229649fc 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -19,13 +19,24 @@ class Commit class << self def decorate(commits) - commits.map { |c| self.new(c) } + commits.map do |commit| + if commit.kind_of?(Commit) + commit + else + self.new(commit) + end + end end # Calculate number of lines to render for diffs def diff_line_count(diffs) diffs.reduce(0) { |sum, d| sum + d.diff.lines.count } end + + # Truncate sha to 8 characters + def truncate_sha(sha) + sha[0..7] + end end attr_accessor :raw @@ -111,7 +122,7 @@ class Commit # Mentionable override. def gfm_reference - "commit #{sha[0..5]}" + "commit #{id}" end def method_missing(m, *args, &block) @@ -124,6 +135,11 @@ class Commit super end + # Truncate sha to 8 characters + def short_id + @raw.short_id(7) + end + def parents @parents ||= Commit.decorate(super) end diff --git a/app/models/event.rb b/app/models/event.rb index 9e296c00281..c0b126713a6 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -266,7 +266,7 @@ class Event < ActiveRecord::Base end def note_short_commit_id - note_commit_id[0..8] + Commit.truncate_sha(note_commit_id) end def note_commit? diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 409e82ed1ef..a71122d5e07 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 last_commit_short_sha - @last_commit_short_sha ||= last_commit.sha[0..10] + @last_commit_short_sha ||= last_commit.short_id end private diff --git a/app/models/repository.rb b/app/models/repository.rb index 339e485e6d2..93994123a90 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -30,6 +30,8 @@ class Repository commit = Gitlab::Git::Commit.find(raw_repository, id) commit = Commit.new(commit) if commit commit + rescue Rugged::OdbError => ex + nil end def commits(ref, path = nil, limit = nil, offset = nil, skip_merges = false) diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index 0e03e116e7d..f0c34def145 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -1,5 +1,5 @@ %li.commit .commit-row-title - = link_to commit[:id][0..8], project_commit_path(project, commit[:id]), class: "commit_short_id", alt: '' + = link_to truncate_sha(commit[:id]), project_commit_path(project, commit[:id]), class: "commit_short_id", alt: '' = gfm event_commit_title(commit[:message]), project diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index 17228c430ca..2b63519edac 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -2,7 +2,7 @@ - event.commits.first(15).each do |commit| %p %strong= commit[:author][:name] - = link_to "(##{commit[:id][0...8]})", project_commit_path(event.project, id: commit[:id]) + = link_to "(##{truncate_sha(commit[:id])})", project_commit_path(event.project, id: commit[:id]) %i at = commit[:timestamp].to_time.to_s(:short) diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 1bca64c7d50..b912b5e092f 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -22,4 +22,4 @@ - if event.commits_count > 2 %span ... and #{event.commits_count - 2} more commits. = link_to project_compare_path(event.project, from: event.commit_from, to: event.commit_to) do - %strong Compare → #{event.commit_from[0..7]}...#{event.commit_to[0..7]} + %strong Compare → #{truncate_sha(event.commit_from)}...#{truncate_sha(event.commit_to)} diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index e5cde488c3c..bdf02c6285d 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -15,7 +15,7 @@ %tr %td.blame-commit %span.commit - = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(@project, commit), class: "commit_short_id" = commit_author_link(commit, avatar: true, size: 16) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 0b6b6af4f90..e149f017f84 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -35,7 +35,7 @@ .commit-info-row %span.cgray= pluralize(@commit.parents.count, "parent") - @commit.parents.each do |parent| - = link_to parent.id[0...10], project_commit_path(@project, parent) + = link_to parent.short_id, project_commit_path(@project, parent) - if @branches.any? .commit-info-row diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 68852ba973f..1eb17f760dc 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,6 +1,6 @@ %li.commit.js-toggle-container .commit-row-title - = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(project, commit), class: "commit_short_id" %span.str-truncated = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" diff --git a/app/views/projects/commits/_inline_commit.html.haml b/app/views/projects/commits/_inline_commit.html.haml index b36369b4285..574599aa2d2 100644 --- a/app/views/projects/commits/_inline_commit.html.haml +++ b/app/views/projects/commits/_inline_commit.html.haml @@ -1,6 +1,6 @@ %li.commit.inline-commit .commit-row-title - = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(project, commit), class: "commit_short_id" %span.str-truncated = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" diff --git a/app/views/projects/tree/_submodule_item.html.haml b/app/views/projects/tree/_submodule_item.html.haml index a8ec9df2c8f..46e9be4af83 100644 --- a/app/views/projects/tree/_submodule_item.html.haml +++ b/app/views/projects/tree/_submodule_item.html.haml @@ -7,8 +7,8 @@ @ %span.monospace - if commit.nil? - #{submodule_item.id[0..10]} + #{truncate_sha(submodule_item.id)} - else - = link_to "#{submodule_item.id[0..10]}", commit + = link_to "#{truncate_sha(submodule_item.id)}", commit %td %td.hidden-xs diff --git a/app/views/projects/wikis/history.html.haml b/app/views/projects/wikis/history.html.haml index d3a66c48c9b..ef4b8f74714 100644 --- a/app/views/projects/wikis/history.html.haml +++ b/app/views/projects/wikis/history.html.haml @@ -17,7 +17,7 @@ %tr %td = link_to project_wiki_path(@project, @page, version_id: commit.id) do - = commit.id[0..10] + = truncate_sha(commit.id) %td = commit.author.name %td diff --git a/app/views/search/results/_note.html.haml b/app/views/search/results/_note.html.haml index f2327cd69cc..a44a4542df5 100644 --- a/app/views/search/results/_note.html.haml +++ b/app/views/search/results/_note.html.haml @@ -10,7 +10,7 @@ = project.name_with_namespace · = link_to project_commit_path(project, note.commit_id, anchor: dom_id(note)) do - Commit #{note.commit_id[0..8]} + Commit #{truncate_sha(note.commit_id)} - else = link_to project do = project.name_with_namespace diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index c054e0e8282..935f313e298 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -8,7 +8,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps commit = @project.repository.commit page.should have_content(@project.name) page.should have_content(commit.message[0..20]) - page.should have_content(commit.id.to_s[0..5]) + page.should have_content(commit.short_id) end step 'I click atom feed link' do diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index c009568977d..fae0cec53a6 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -111,7 +111,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I click on the commit in the merge request' do within '.mr-commits' do - click_link sample_commit.id[0..8] + click_link Commit.truncate_sha(sample_commit.id) end end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 328942d32b0..61751a82369 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -60,7 +60,7 @@ describe GitlabMarkdownHelper do end it "should link using a short id" do - actual = "Backported from #{commit.short_id(6)}" + actual = "Backported from #{commit.short_id}" gfm(actual).should match(expected) end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 6f201adc4e8..a6ec44da4be 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -75,7 +75,7 @@ eos it_behaves_like 'a mentionable' do let(:subject) { commit } let(:mauthor) { create :user, email: commit.author_email } - let(:backref_text) { "commit #{subject.sha[0..5]}" } + let(:backref_text) { "commit #{subject.id}" } let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } } # Include the subject in the repository stub. diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index eeecd714a28..2d839e9611b 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -228,7 +228,7 @@ describe Note do it { should be_valid } its(:noteable) { should == issue } - its(:note) { should == "_mentioned in commit #{commit.sha[0..5]}_" } + its(:note) { should == "_mentioned in commit #{commit.sha}_" } end context 'merge request from an issue' do @@ -267,7 +267,7 @@ describe Note do its(:noteable_type) { should == "Commit" } its(:noteable_id) { should be_nil } its(:commit_id) { should == commit.id } - its(:note) { should == "_mentioned in commit #{parent_commit.id[0...6]}_" } + its(:note) { should == "_mentioned in commit #{parent_commit.id}_" } end end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 692834c9f29..ebd74206699 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -30,15 +30,15 @@ def common_mentionable_setup "!#{mentioned_mr.iid}, " + "#{ext_proj.path_with_namespace}##{ext_issue.iid}, " + "#{ext_proj.path_with_namespace}!#{ext_mr.iid}, " + - "#{ext_proj.path_with_namespace}@#{ext_commit.id[0..5]}, " + - "#{mentioned_commit.sha[0..5]} and itself as #{backref_text}" + "#{ext_proj.path_with_namespace}@#{ext_commit.short_id}, " + + "#{mentioned_commit.sha[0..10]} and itself as #{backref_text}" end before do # Wire the project's repository to return the mentioned commit, and +nil+ for any # unrecognized commits. - commitmap = { '123456' => mentioned_commit } - extra_commits.each { |c| commitmap[c.sha[0..5]] = c } + commitmap = { '1234567890a' => mentioned_commit } + extra_commits.each { |c| commitmap[c.short_id] = c } mproject.repository.stub(:commit) { |sha| commitmap[sha] } set_mentionable_text.call(ref_string) end @@ -54,7 +54,6 @@ shared_examples 'a mentionable' do it "extracts references from its reference property" do # De-duplicate and omit itself refs = subject.references(mproject) - refs.should have(6).items refs.should include(mentioned_issue) refs.should include(mentioned_mr) @@ -90,7 +89,7 @@ shared_examples 'an editable mentionable' do it 'creates new cross-reference notes when the mentionable text is edited' do new_text = "still mentions ##{mentioned_issue.iid}, " + - "#{mentioned_commit.sha[0..5]}, " + + "#{mentioned_commit.sha[0..10]}, " + "#{ext_issue.iid}, " + "new refs: ##{other_issue.iid}, " + "#{ext_proj.path_with_namespace}##{other_ext_issue.iid}" |