Merge branch 'sh-include-worktree-stderr' into 'master'

Include stderr in output of worktree operations

See merge request gitlab-org/gitaly!1787

4 files changed, 33 insertions, 4 deletions

diff --git a/changelogs/unreleased/sh-include-worktree-stderr.yml b/changelogs/unreleased/sh-include-worktree-stderr.yml
new file mode 100644
index 000000000..2d3329b04
--- /dev/null
+++ b/changelogs/unreleased/sh-include-worktree-stderr.yml
@@ -0,0 +1,5 @@
+---
+title: Include stderr in output of worktree operations
+merge_request: 1787
+author:
+type: fixed
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index d443ceb77..78feb3555 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -877,21 +877,21 @@ module Gitlab
         # checkout files in by a changeset but that changeset only adds files.
         if sparse_checkout_files
           # Create worktree without checking out
-          run_git!(base_args + ['--no-checkout', worktree.path], env: env)
+          run_git!(base_args + ['--no-checkout', worktree.path], env: env, include_stderr: true)
           worktree_git_path = run_git!(%w[rev-parse --git-dir], chdir: worktree.path).chomp
 
           configure_sparse_checkout(worktree_git_path, sparse_checkout_files)
 
           # After sparse checkout configuration, checkout `branch` in worktree
-          run_git!(%W[checkout --detach #{branch}], chdir: worktree.path, env: env)
+          run_git!(%W[checkout --detach #{branch}], chdir: worktree.path, env: env, include_stderr: true)
         else
           # Create worktree and checkout `branch` in it
-          run_git!(base_args + [worktree.path, branch], env: env)
+          run_git!(base_args + [worktree.path, branch], env: env, include_stderr: true)
         end
 
         yield
       ensure
-        run_git(%W[worktree remove -f #{worktree.name}])
+        run_git(%W[worktree remove -f #{worktree.name}], include_stderr: true)
       end
 
       # Adding a worktree means checking out the repository. For large repos,
diff --git a/ruby/spec/lib/gitlab/git/popen_spec.rb b/ruby/spec/lib/gitlab/git/popen_spec.rb
index 8b09dd313..efa3ed922 100644
--- a/ruby/spec/lib/gitlab/git/popen_spec.rb
+++ b/ruby/spec/lib/gitlab/git/popen_spec.rb
@@ -39,6 +39,15 @@ describe 'Gitlab::Git::Popen' do
       it { expect(output).to eq('') }
     end
 
+    context 'when stderr is included' do
+      let(:result) { klass.new.popen(['ruby', '-e', 'warn "hello world"'], path, include_stderr: true) }
+      let(:output) { result.first }
+      let(:status) { result.last }
+
+      it { expect(status).to eq(0) }
+      it { expect(output).to eq("hello world\n") }
+    end
+
     context 'unsafe string command' do
       it 'raises an error when it gets called with a string argument' do
         expect { klass.new.popen('ls', path) }.to raise_error(RuntimeError)
diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb
index 216666e60..979f17a04 100644
--- a/ruby/spec/lib/gitlab/git/repository_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_spec.rb
@@ -702,6 +702,21 @@ describe Gitlab::Git::Repository do # rubocop:disable Metrics/BlockLength
           subject
         end
       end
+
+      describe 'when worktree throws an error' do
+        before do
+          allow(repository).to receive(:run_git).and_return(['ok', 0])
+          expect(repository).to receive(:run_git)
+            .with(array_including('worktree'), chdir: anything, env: anything, nice: false, include_stderr: true, lazy_block: anything)
+            .and_return(['error', 1])
+        end
+
+        it 'includes the stderr output' do
+          expect do
+            subject
+          end.to raise_error(described_class::GitError, 'error')
+        end
+      end
     end
 
     describe 'with an ASCII-8BIT diff' do