Support pipelines. by georgebrock · Pull Request #299 · thoughtbot/gitsh

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

+    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


+    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

+  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

+
+  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

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


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

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

+    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

+  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

      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

+    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


+    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

+  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

+
+  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

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


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

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

+    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

+  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

      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

Uh oh!

Conversation

georgebrock commented May 21, 2017

Uh oh!

georgebrock commented May 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharplet commented Mar 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sharplet commented Mar 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sharplet commented Mar 9, 2019 •

edited

Loading