aureliojargas / txt2regex

Regex wizard for the terminal, written in Bash
https://aurelio.net/projects/txt2regex/
GNU General Public License v2.0
183 stars 26 forks source link

Migrate: Travis CI → GitHub Actions #9

Closed aureliojargas closed 3 years ago

aureliojargas commented 3 years ago

Besides adding the GitHub Actions configuration, I've had to fix a test (see the second commit):

The previous code fails when running in GitHub Actions:

[FAILED #39, line 384] txt2regex --foo | head -n 3 | sed '3 s/:.*//'
@@ -1,3 +1,21 @@
+./txt2regex.sh: line 115: printf: write error: Broken pipe
+./txt2regex.sh: line 117: printf: write error: Broken pipe
+./txt2regex.sh: line 119: printf: write error: Broken pipe
+./txt2regex.sh: line 121: printf: write error: Broken pipe
+./txt2regex.sh: line 122: printf: write error: Broken pipe
+./txt2regex.sh: line 123: printf: write error: Broken pipe
 --foo: invalid option

 usage

Even when changing head -n 3 to sed 3q it failed with the same error.

When doing a single sed after the first pipe it worked fine once. In the next run it failed again, with the same error. I don't know what's happening, to be honest.

Now trying with a temporary file, to avoid piping the first command. Running 3x, all of them passed. I guess we have a winner.

aureliojargas commented 3 years ago

After some research, I think now I understand the Broken pipe problem.

In my problematic command txt2regex --foo | head -n 3 | sed '3 s/:.*//, after reading the third line, head is done and just exits, since it doesn't need to read any extra lines. The kernel will then signal SIGPIPE to txt2regex, so it can be aware that the pipe is no more and there's no point in trying to write to it anymore. This is the default behavior, when there's no custom trap for SIGPIPE.

But the GitHub Actions runner traps SIGPIPE, ignoring it. This can be verified by running trap with no arguments, this is what shows up as output:

trap -- '' SIGPIPE

So in this case, SIGPIPE is still signaled against txt2regex by the kernel when head early exits, but since the signal is trapped and ignored by the GitHub Actions runner, it will be ignored by the txt2regex process and it will try to send new lines to stdout (which is pipe-connected to head stdin -- which is now gone). Then this fails and then the error write error: Broken pipe is reported. In other words, the printf inside txt2regex tried to print to a closed file descriptor.

The best solution would be to "undo" the GitHub Actions trap and just have a working SIGPIPE. But it looks like once set like that, children cannot change it.

This excellent article (it's in Japanese, but the Google translation is decent) explains everything in details and gives some alternatives for fixes:

Thanks Mario Domenech Goulart and Michael Ho for helping and teaching me on that.

Update at 2022-09-21: I'm testing this issue in this sandbox PR: https://github.com/aureliojargas/github-actions-sandbox/pull/4

aureliojargas commented 1 year ago

The mentioned PR #13 upgraded the CI image to Ubuntu 22.04 so we can use the env --default-signal=PIPE trick to fix the issue for good 🎉