Workhorse to send raw diff and patch for commits
Prior to this change, this was done through unicorn. In theory this could time out. Workhorse has been sending these raw patches and diffs for a long time and is stable in doing so. Added bonus is the fact that `Commit#to_patch` can be removed. `Commit#to_diff` too, which closes https://gitlab.com/gitlab-org/gitaly/issues/324 Closes https://gitlab.com/gitlab-org/gitaly/issues/1196
This commit is contained in:
parent
e844259574
commit
dfdd881510
|
@ -23,8 +23,12 @@ class Projects::CommitController < Projects::ApplicationController
|
||||||
|
|
||||||
respond_to do |format|
|
respond_to do |format|
|
||||||
format.html { render }
|
format.html { render }
|
||||||
format.diff { render text: @commit.to_diff }
|
format.diff do
|
||||||
format.patch { render text: @commit.to_patch }
|
send_git_diff(@project.repository, @commit.diff_refs)
|
||||||
|
end
|
||||||
|
format.patch do
|
||||||
|
send_git_patch(@project.repository, @commit.diff_refs)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Workhorse to send raw diff and patch for commits
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: other
|
|
@ -342,21 +342,6 @@ module Gitlab
|
||||||
parent_ids.first
|
parent_ids.first
|
||||||
end
|
end
|
||||||
|
|
||||||
# Shows the diff between the commit's parent and the commit.
|
|
||||||
#
|
|
||||||
# Cuts out the header and stats from #to_patch and returns only the diff.
|
|
||||||
#
|
|
||||||
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324
|
|
||||||
def to_diff
|
|
||||||
Gitlab::GitalyClient.migrate(:commit_patch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
|
|
||||||
if is_enabled
|
|
||||||
@repository.gitaly_commit_client.patch(id)
|
|
||||||
else
|
|
||||||
rugged_diff_from_parent.patch
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
# Returns a diff object for the changes from this commit's first parent.
|
# Returns a diff object for the changes from this commit's first parent.
|
||||||
# If there is no parent, then the diff is between this commit and an
|
# If there is no parent, then the diff is between this commit and an
|
||||||
# empty repo. See Repository#diff for keys allowed in the +options+
|
# empty repo. See Repository#diff for keys allowed in the +options+
|
||||||
|
@ -432,16 +417,6 @@ module Gitlab
|
||||||
Gitlab::Git::CommitStats.new(@repository, self)
|
Gitlab::Git::CommitStats.new(@repository, self)
|
||||||
end
|
end
|
||||||
|
|
||||||
def to_patch(options = {})
|
|
||||||
begin
|
|
||||||
rugged_commit.to_mbox(options)
|
|
||||||
rescue Rugged::InvalidError => ex
|
|
||||||
if ex.message =~ /commit \w+ is a merge commit/i
|
|
||||||
'Patch format is not currently supported for merge commits.'
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
# Get ref names collection
|
# Get ref names collection
|
||||||
#
|
#
|
||||||
# Ex.
|
# Ex.
|
||||||
|
|
|
@ -79,41 +79,18 @@ describe Projects::CommitController do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "as diff" do
|
describe "as diff" do
|
||||||
include_examples "export as", :diff
|
it "triggers workhorse to serve the request" do
|
||||||
let(:format) { :diff }
|
go(id: commit.id, format: :diff)
|
||||||
|
|
||||||
it "should really only be a git diff" do
|
expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:")
|
||||||
go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format)
|
|
||||||
|
|
||||||
expect(response.body).to start_with("diff --git")
|
|
||||||
end
|
|
||||||
|
|
||||||
it "is only be a git diff without whitespace changes" do
|
|
||||||
go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1)
|
|
||||||
|
|
||||||
expect(response.body).to start_with("diff --git")
|
|
||||||
|
|
||||||
# without whitespace option, there are more than 2 diff_splits for other formats
|
|
||||||
diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n")
|
|
||||||
expect(diff_splits.length).to be <= 2
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "as patch" do
|
describe "as patch" do
|
||||||
include_examples "export as", :patch
|
|
||||||
let(:format) { :patch }
|
|
||||||
let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') }
|
|
||||||
|
|
||||||
it "is a git email patch" do
|
|
||||||
go(id: commit2.id, format: format)
|
|
||||||
|
|
||||||
expect(response.body).to start_with("From #{commit2.id}")
|
|
||||||
end
|
|
||||||
|
|
||||||
it "contains a git diff" do
|
it "contains a git diff" do
|
||||||
go(id: commit2.id, format: format)
|
go(id: commit.id, format: :patch)
|
||||||
|
|
||||||
expect(response.body).to match(/^diff --git/)
|
expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -554,24 +554,10 @@ describe Gitlab::Git::Commit, seed_helper: true do
|
||||||
it_should_behave_like '#stats'
|
it_should_behave_like '#stats'
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#to_diff' do
|
|
||||||
subject { commit.to_diff }
|
|
||||||
|
|
||||||
it { is_expected.not_to include "From #{SeedRepo::Commit::ID}" }
|
|
||||||
it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#has_zero_stats?' do
|
describe '#has_zero_stats?' do
|
||||||
it { expect(commit.has_zero_stats?).to eq(false) }
|
it { expect(commit.has_zero_stats?).to eq(false) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#to_patch' do
|
|
||||||
subject { commit.to_patch }
|
|
||||||
|
|
||||||
it { is_expected.to include "From #{SeedRepo::Commit::ID}" }
|
|
||||||
it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#to_hash' do
|
describe '#to_hash' do
|
||||||
let(:hash) { commit.to_hash }
|
let(:hash) { commit.to_hash }
|
||||||
subject { hash }
|
subject { hash }
|
||||||
|
|
|
@ -182,7 +182,6 @@ eos
|
||||||
it { is_expected.to respond_to(:date) }
|
it { is_expected.to respond_to(:date) }
|
||||||
it { is_expected.to respond_to(:diffs) }
|
it { is_expected.to respond_to(:diffs) }
|
||||||
it { is_expected.to respond_to(:id) }
|
it { is_expected.to respond_to(:id) }
|
||||||
it { is_expected.to respond_to(:to_patch) }
|
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#closes_issues' do
|
describe '#closes_issues' do
|
||||||
|
|
Loading…
Reference in New Issue