Skip to content

Commit 363eb3e

Browse files
committed
Support passing standard input stream to execute helper
We want to support running existing git hooks that developers have written. In order to do that, we need to support passing a standard input stream, since some hooks use that as the mechanism to pass input. This modifies the work we did in 317047a and 82d5220 to support long argument lists by changing the signature of the `execute` helper to accept an options hash, requiring that splittable arguments now be passed using the `args` option so that the `input` option can also be passed if necessary. This also has the added benefit of having our `CommandSplitter` support passing standard input streams, though there's probably not many situations where that would actually be necessary.
1 parent 1f5f5b3 commit 363eb3e

10 files changed

+84
-36
lines changed

lib/overcommit/command_splitter.rb

+6-4
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,19 @@ def stderr
5151
end
5252

5353
class << self
54-
def execute(args, splittable_args)
55-
if splittable_args.empty?
54+
def execute(initial_args, options)
55+
options = options.dup
56+
57+
if (splittable_args = (options.delete(:args) { [] })).empty?
5658
raise Overcommit::Exceptions::InvalidCommandArgs,
5759
'Must specify list of arguments to split on'
5860
end
5961

6062
# Execute each chunk of arguments in serial. We don't parallelize (yet)
6163
# since in theory we want to support parallelization at the hook level
6264
# and not within individual hooks.
63-
results = extract_argument_lists(args, splittable_args).map do |arg_list|
64-
Overcommit::Subprocess.spawn(arg_list)
65+
results = extract_argument_lists(initial_args, splittable_args).map do |arg_list|
66+
Overcommit::Subprocess.spawn(arg_list, options)
6567
end
6668

6769
Result.new(results.map(&:status), results.map(&:stdout), results.map(&:stderr))

lib/overcommit/hook/base.rb

+6-3
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,14 @@ def in_path?(cmd)
9090
# for splitting up long file lists.
9191
#
9292
# @param cmd [Array<String>] command arguments
93-
# @param splittable_args [Array<String>] arguments that can be split up over
93+
# @param options [Hash]
94+
# @option options [Array<String>] :args arguments that can be split up over
9495
# multiple invocations (usually a list of files)
96+
# @option options [String] :input string to pass to process' standard input
97+
# stream
9598
# @return [#status,#stdout,#stderr] struct containing result of invocation
96-
def execute(cmd, splittable_args = nil)
97-
Overcommit::Utils.execute(cmd, splittable_args)
99+
def execute(cmd, options = {})
100+
Overcommit::Utils.execute(cmd, options)
98101
end
99102

100103
def execute_in_background(cmd)

lib/overcommit/hook/pre_commit/hard_tabs.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module Overcommit::Hook::PreCommit
22
# Checks for hard tabs in files.
33
class HardTabs < Base
44
def run
5-
result = execute(command, applicable_files)
5+
result = execute(command, args: applicable_files)
66

77
extract_messages(
88
result.stdout.split("\n"),

lib/overcommit/hook/pre_commit/local_paths_in_gemfile.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module Overcommit::Hook::PreCommit
22
# Checks for local paths in files and issues a warning
33
class LocalPathsInGemfile < Base
44
def run
5-
result = execute(command, applicable_files)
5+
result = execute(command, args: applicable_files)
66

77
unless result.stdout.empty?
88
return :warn, "Avoid pointing to local paths in Gemfiles:\n#{result.stdout}"

lib/overcommit/hook/pre_commit/merge_conflicts.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module Overcommit::Hook::PreCommit
22
# Checks for unresolved merge conflicts
33
class MergeConflicts < Base
44
def run
5-
result = execute(command, applicable_files)
5+
result = execute(command, args: applicable_files)
66

77
unless result.stdout.empty?
88
return :fail, "Merge conflict markers detected:\n#{result.stdout}"

lib/overcommit/hook/pre_commit/trailing_whitespace.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module Overcommit::Hook::PreCommit
22
# Checks for trailing whitespace in files.
33
class TrailingWhitespace < Base
44
def run
5-
result = execute(command, applicable_files)
5+
result = execute(command, args: applicable_files)
66

77
extract_messages(
88
result.stdout.split("\n"),

lib/overcommit/subprocess.rb

+11-1
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,22 @@ def success?
1919
class << self
2020
# Spawns a new process using the given array of arguments (the first
2121
# element is the command).
22-
def spawn(args)
22+
#
23+
# @param args [Array<String>]
24+
# @param options [Hash]
25+
# @option options [String] input string to pass via standard input stream
26+
# @return [Result]
27+
def spawn(args, options = {})
2328
process = ChildProcess.build(*args)
2429

2530
out, err = assign_output_streams(process)
2631

32+
process.duplex = true if options[:input] # Make stdin available if needed
2733
process.start
34+
if options[:input]
35+
process.io.stdin.puts(options[:input])
36+
process.io.stdin.close
37+
end
2838
process.wait
2939

3040
err.rewind

lib/overcommit/utils.rb

+15-14
Original file line numberDiff line numberDiff line change
@@ -146,32 +146,33 @@ def parent_command
146146
# This is intended to provide a centralized place to perform any checks or
147147
# filtering of the command before executing it.
148148
#
149-
# The `splittable_args` parameter provides a convenient way of splitting
150-
# up long argument lists which would otherwise exceed the maximum command
151-
# line length of the OS. It will break up the list into chunks and run the
152-
# command with the same prefix `args`, finally combining the output
153-
# together at the end.
149+
# The `args` option provides a convenient way of splitting up long
150+
# argument lists which would otherwise exceed the maximum command line
151+
# length of the OS. It will break up the list into chunks and run the
152+
# command with the same prefix `initial_args`, finally combining the
153+
# output together at the end.
154154
#
155155
# This requires that the external command you are running can have its
156156
# work split up in this way and still produce the same resultant output
157157
# when outputs of the individual commands are concatenated back together.
158158
#
159-
# @param args [Array<String>]
160-
# @param splittable_args [Array<String>]
159+
# @param initial_args [Array<String>]
160+
# @param options [Hash]
161+
# @option options [Array<String>] :args long list of arguments to split up
161162
# @return [Overcommit::Subprocess::Result] status, stdout, and stderr
162-
def execute(args, splittable_args = nil)
163-
if args.include?('|')
163+
def execute(initial_args, options = {})
164+
if initial_args.include?('|')
164165
raise Overcommit::Exceptions::InvalidCommandArgs,
165166
'Cannot pipe commands with the `execute` helper'
166167
end
167168

168169
result =
169-
if splittable_args
170-
debug(args.join(' ') + " ... (#{splittable_args.length} splittable args)")
171-
Overcommit::CommandSplitter.execute(args, splittable_args)
170+
if splittable_args = options[:args]
171+
debug(initial_args.join(' ') + " ... (#{splittable_args.length} splittable args)")
172+
Overcommit::CommandSplitter.execute(initial_args, options)
172173
else
173-
debug(args.join(' '))
174-
Overcommit::Subprocess.spawn(args)
174+
debug(initial_args.join(' '))
175+
Overcommit::Subprocess.spawn(initial_args, options)
175176
end
176177

177178
debug("EXIT STATUS: #{result.status}")

spec/overcommit/command_splitter_spec.rb

+29-8
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
describe '.execute' do
55
let(:args_prefix) { %w[cmd] }
66
let(:max_command_length) { 10 }
7+
let(:options) { { args: splittable_args } }
78

8-
subject { described_class.execute(args_prefix, splittable_args) }
9+
subject { described_class.execute(args_prefix, options) }
910

1011
before do
1112
described_class.stub(:max_command_length).and_return(max_command_length)
@@ -35,8 +36,10 @@
3536
let(:splittable_args) { %w[1 2 3 4 5 6 7 8] }
3637

3738
it 'executes two commands with the appropriately split arguments' do
38-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 1 2 3 4 5 6 7])
39-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 8])
39+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 1 2 3 4 5 6 7],
40+
hash_excluding(:args))
41+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 8],
42+
hash_excluding(:args))
4043
subject
4144
end
4245

@@ -55,7 +58,7 @@
5558
context 'when one command fails' do
5659
before do
5760
Overcommit::Subprocess.stub(:spawn).
58-
with(%w[cmd 8]).
61+
with(%w[cmd 8], anything).
5962
and_return(Overcommit::Subprocess::Result.new(2, 'whoa', 'bad error'))
6063
end
6164

@@ -75,10 +78,14 @@
7578
let(:splittable_args) { 15.times.map { |i| (i + 1).to_s } }
7679

7780
it 'executes multiple commands with the appropriately split arguments' do
78-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 1 2 3 4 5 6 7])
79-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 8 9 10 11])
80-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 12 13 14])
81-
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 15])
81+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 1 2 3 4 5 6 7],
82+
hash_excluding(:args))
83+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 8 9 10 11],
84+
hash_excluding(:args))
85+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 12 13 14],
86+
hash_excluding(:args))
87+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cmd 15],
88+
hash_excluding(:args))
8289
subject
8390
end
8491
end
@@ -101,5 +108,19 @@
101108
expect { subject }.to raise_error Overcommit::Exceptions::InvalidCommandArgs
102109
end
103110
end
111+
112+
context 'with a standard input stream specified' do
113+
let(:max_command_length) { 5 }
114+
let(:args_prefix) { %w[cat] }
115+
let(:options) { { args: %w[- - -], input: 'Hello' } }
116+
117+
it 'passes the same standard input to each command' do
118+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cat - -],
119+
hash_including(input: 'Hello'))
120+
Overcommit::Subprocess.should_receive(:spawn).with(%w[cat -],
121+
hash_including(input: 'Hello'))
122+
subject
123+
end
124+
end
104125
end
105126
end

spec/overcommit/utils_spec.rb

+13-2
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,26 @@
174174
end
175175
end
176176

177+
context 'when given an input stream' do
178+
let(:arguments) { ['cat', '-'] }
179+
let(:input) { 'Hello world' }
180+
181+
subject { described_class.execute(arguments, input: input) }
182+
183+
it 'passes the input to the standard input stream' do
184+
subject.stdout.should == "Hello world\n"
185+
end
186+
end
187+
177188
context 'when given a list of arguments to execute in chunks' do
178189
let(:arguments) { ['echo'] }
179190
let(:splittable_args) { %w[1 2 3] }
180191

181-
subject { described_class.execute(arguments, splittable_args) }
192+
subject { described_class.execute(arguments, args: splittable_args) }
182193

183194
it 'invokes CommandSplitter.execute' do
184195
Overcommit::CommandSplitter.should_receive(:execute).
185-
with(arguments, splittable_args).
196+
with(arguments, args: splittable_args).
186197
and_return(double(status: 0, stdout: '', stderr: ''))
187198
subject
188199
end

0 commit comments

Comments
 (0)