Support pipelines. #299

georgebrock · 2017-05-21T04:05:06Z

Two commands combined with the pipe character (|) will be run in parallel with the standard output of the first command connected to the standard input of the second command via an IO.pipe.

georgebrock · 2017-05-21T22:07:49Z

The first commit on this branch works on macOS, but not on Linux (e.g. on CI).

The difference seems to be how sh(1) handles SIGINT when run with -c. On Ubuntu Linux, which is the distribution CI uses and the one I've done manual testing on, /bin/sh appears to be dash(1). I can't reproduce the problem with FreeBSD's sh(1) or macOS's (which is GNU bash running in a compatibility mode).

To experiment, I wrote a little Ruby script that does nothing until it gets a SIGINT:

# test.rb
begin
  sleep
rescue Interrupt
  puts 'SIGINT!'
end

When I spawn a new process that runs this script directly, I can easily send it an interrupt:

irb(main):001:0> p = Process.spawn('ruby2.0', 'test.rb')
=> 8594
irb(main):002:0> Process.kill('INT', p)
=> 1
SIGINT!
irb(main):003:0> Process.wait(p)
=> 8594

However, when I spawn a new process that runs the script in a shell, I can't interrupt it:

irb(main):004:0> p = Process.spawn('sh', '-c', 'ruby2.0 test.rb')
=> 8661
irb(main):005:0> Process.kill('INT', p)
=> 1
irb(main):006:0> Process.wait(p)
^CSIGINT!
IRB::Abort: abort then interrupt!
        from (irb):6:in `call'
        from (irb):6:in `wait'
        from (irb):6
        from /usr/bin/irb2.0:12:in `<main>'

Note that in this case SIGINT! is only output when I press ctrlc.

The only vaguely useful reference to this I've found so far is a line in some Docker documentation:

… will be started as a subcommand of /bin/sh -c, which does not pass signals.

houndci-bot · 2019-03-03T15:46:12Z

spec/units/parser_spec.rb

+ it 'parses two commands combined with |' do
+ result = parse(tokens(
+ [:WORD, 'log'], [:PIPE], [:WORD, '!wc'], [:EOS],
+ ))


Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

houndci-bot · 2019-03-03T15:46:12Z

spec/units/parser_spec.rb

@@ -169,6 +169,14 @@
 expect(result).to be_a(Gitsh::Commands::Tree::Multi)
 end

+ it 'parses two commands combined with |' do
+ result = parse(tokens(
+ [:WORD, 'log'], [:PIPE], [:WORD, '!wc'], [:EOS],


Layout/FirstParameterIndentation: Indent the first parameter one step more than tokens(.

houndci-bot · 2019-03-03T15:46:12Z

spec/units/commands/pipeline_spec.rb

+ def build_env
+ Gitsh::Environment.new(
+ input_stream: instance_double(IO, read: ""),
+ output_stream: StringIO.new


Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

houndci-bot · 2019-03-03T15:46:13Z

spec/units/commands/pipeline_spec.rb

+
+ def build_env
+ Gitsh::Environment.new(
+ input_stream: instance_double(IO, read: ""),


Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2019-03-03T15:46:13Z

spec/units/commands/pipeline_spec.rb

+ end
+ end
+
+ def create_command_double(value=true)


Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment.

houndci-bot · 2019-03-03T15:46:13Z

spec/units/commands/pipeline_spec.rb

+ context 'when the left command fails' do
+ it 'returns false' do
+ left_command = create_command_double(false) { '' }
+ right_command = create_command_double { |input| input.upcase }


Style/SymbolProc: Pass &:upcase as an argument to create_command_double instead of a block.

houndci-bot · 2019-03-03T15:46:13Z

spec/units/commands/pipeline_spec.rb

+ describe '#execute' do
+ it 'pipes output of left command to right command' do
+ left_command = create_command_double { 'string' }
+ right_command = create_command_double { |input| input.upcase }


Style/SymbolProc: Pass &:upcase as an argument to create_command_double instead of a block.

houndci-bot · 2019-03-03T15:46:13Z

lib/gitsh/parser.rb

@@ -36,6 +37,7 @@ class Parser < RLTK::Parser
 clause('.commands SEMICOLON .commands') { |c1, c2| Commands::Tree::Multi.new(c1, c2) }
 clause('.commands OR .commands') { |c1, c2| Commands::Tree::Or.new(c1, c2) }
 clause('.commands AND .commands') { |c1, c2| Commands::Tree::And.new(c1, c2) }
+ clause('.commands PIPE .commands') { |c1, c2| Commands::Pipeline.new(c1, c2) }


Metrics/LineLength: Line is too long. [84/80]

houndci-bot · 2019-03-03T15:46:14Z

spec/units/parser_spec.rb

+ it 'parses two commands combined with |' do
+ result = parse(tokens(
+ [:WORD, 'log'], [:PIPE], [:WORD, '!wc'], [:EOS],
+ ))


Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/parser_spec.rb

@@ -169,6 +169,14 @@
 expect(result).to be_a(Gitsh::Commands::Tree::Multi)
 end

+ it 'parses two commands combined with |' do
+ result = parse(tokens(
+ [:WORD, 'log'], [:PIPE], [:WORD, '!wc'], [:EOS],


Layout/FirstParameterIndentation: Indent the first parameter one step more than tokens(.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/commands/pipeline_spec.rb

+ def build_env
+ Gitsh::Environment.new(
+ input_stream: instance_double(IO, read: ""),
+ output_stream: StringIO.new


Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/commands/pipeline_spec.rb

+
+ def build_env
+ Gitsh::Environment.new(
+ input_stream: instance_double(IO, read: ""),


Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/commands/pipeline_spec.rb

+ end
+ end
+
+ def create_command_double(value=true)


Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/commands/pipeline_spec.rb

+ context 'when the left command fails' do
+ it 'returns false' do
+ left_command = create_command_double(false) { '' }
+ right_command = create_command_double { |input| input.upcase }


Style/SymbolProc: Pass &:upcase as an argument to create_command_double instead of a block.

houndci-bot · 2019-03-03T15:46:15Z

spec/units/commands/pipeline_spec.rb

+ describe '#execute' do
+ it 'pipes output of left command to right command' do
+ left_command = create_command_double { 'string' }
+ right_command = create_command_double { |input| input.upcase }


Style/SymbolProc: Pass &:upcase as an argument to create_command_double instead of a block.

houndci-bot · 2019-03-03T15:46:16Z

lib/gitsh/parser.rb

@@ -36,6 +37,7 @@ class Parser < RLTK::Parser
 clause('.commands SEMICOLON .commands') { |c1, c2| Commands::Tree::Multi.new(c1, c2) }
 clause('.commands OR .commands') { |c1, c2| Commands::Tree::Or.new(c1, c2) }
 clause('.commands AND .commands') { |c1, c2| Commands::Tree::And.new(c1, c2) }
+ clause('.commands PIPE .commands') { |c1, c2| Commands::Pipeline.new(c1, c2) }


Metrics/LineLength: Line is too long. [84/80]

sharplet · 2019-03-09T15:44:37Z

Here’s another possibly relevant snippet of documentation from OpenBSD’s sh(1) man page:

SIGINT

If a shell is interactive and in command line editing mode, editing is terminated on the current line and the command being edited is not entered into command history. Otherwise the signal is caught but no action is taken.

Would it make sense for gitsh to catch SIGINT and manually send ~~SIGKILL~~ SIGTERM to child shell processes in a pipeline?

sharplet · 2019-03-09T17:52:22Z

Ok, I had a play around with this and I think I got it to work: https://gist.github.com/sharplet/5dc8bd4239e09f68e8f009dbfe48f9ff#file-subshell-swift-L23.

This sets up a signal handler for SIGINT that calls kill(0, SIGTERM), terminating the shell and its children. I tried sending SIGTERM to just the pid of the shell, but it doesn’t also terminate child processes.

Using 0 as the first argument to kill(2) sends the signal to all process in the same process group. I’m not sure if that’s too blunt an instrument, but if gitsh isn’t running any other child processes, then terminating everything that’s currently running seems reasonable to me.

Two commands combined with the pipe character (`|`) will be run in parallel with the standard output of the first command connected to the standard input of the second command via an `IO.pipe`.

georgebrock mentioned this pull request May 21, 2017

Support pipelines #263

Closed

georgebrock force-pushed the pipelines branch 2 times, most recently from 6ba89fb to 5e5b312 Compare May 29, 2017 14:39

georgebrock force-pushed the pipelines branch from 5e5b312 to ce79e2a Compare June 21, 2017 13:42

georgebrock mentioned this pull request Jul 3, 2017

Wildcards don't seem to work correctly #145

Open

georgebrock force-pushed the pipelines branch from ce79e2a to b890232 Compare March 3, 2019 15:45

houndci-bot reviewed Mar 3, 2019

View reviewed changes

georgebrock added this to the v0.15 milestone Mar 3, 2019

Support pipelines.

6ef39b3

Two commands combined with the pipe character (`|`) will be run in parallel with the standard output of the first command connected to the standard input of the second command via an `IO.pipe`.

georgebrock force-pushed the pipelines branch from b890232 to 6ef39b3 Compare March 10, 2019 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support pipelines. #299

Support pipelines. #299

georgebrock commented May 21, 2017

georgebrock commented May 21, 2017

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

houndci-bot Mar 3, 2019

sharplet commented Mar 9, 2019 •

edited

Loading

sharplet commented Mar 9, 2019

Support pipelines. #299

Are you sure you want to change the base?

Support pipelines. #299

Conversation

georgebrock commented May 21, 2017

georgebrock commented May 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharplet commented Mar 9, 2019 • edited Loading

sharplet commented Mar 9, 2019

sharplet commented Mar 9, 2019 •

edited

Loading