Subject: bug#54035: closed (Re: bug#54035: Patch for easier use in scripting pipelines)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 24 Feb 2022 19:10:02 +0000

Your bug report

#54035: Patch for easier use in scripting pipelines

which was filed against the grep package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
54035: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=54035
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#54035: Patch for easier use in scripting pipelines
From: Paul Eggert <[email-masked]>
To: Ulrich Eckhardt <[email-masked]>
Cc: [email-masked]
Date: Thu, 24 Feb 2022 11:09:05 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

On 2/23/22 23:11, Ulrich Eckhardt wrote:

> Just for my understanding, grep stops reading
> when it finds the first match and then the shell closes the output
> stream of cat. That in turn causes cat to fail (exit code 141, meaning
> SIGPIPE), because it can't write the rest of the data that it wants,
> right?

Right.

> I think that short reads (which could cause SIGPIPE) and the
> non-error exit code 1 deserve mention there. I'll take a look and
> perhaps file another patch.

I installed the attached to try to document this better.

From 89de21bd2525088cf5ce12395515ca1d8ef582a2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 24 Feb 2022 11:07:14 -0800
Subject: [PATCH] doc: mention issues with set -e

* doc/grep.texi (Usage, Performance): Mention early exits (Bug#54035).
---
 doc/grep.texi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/doc/grep.texi b/doc/grep.texi
index 37ef839..ebbefda 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -1862,6 +1862,22 @@ Use the special file name @samp{-}:
 cat /etc/passwd | grep 'alain' - /etc/motd
 @end example
 
+@item
+Why can't I combine the shell's @samp{set -e} with @command{grep}?
+
+The @command{grep} command follows the convention of programs like
+@command{cmp} and @command{diff} where an exit status of 1 is not an
+error.  The shell command @samp{set -e} causes the shell to exit if
+any subcommand exits with nonzero status, and this will cause the
+shell to exit merely because @command{grep} selected no lines,
+which is ordinarily not what you want.
+
+There is a related problem with Bash's @command{set -e -o pipefail}.
+Since @command{grep} does not always read all its input, a command
+outputting to a pipe read by @command{grep} can fail when
+@command{grep} exits before reading all its input, and the command's
+failure can cause Bash to exit.
+
 @item
 Why is this back-reference failing?
 
@@ -1998,6 +2014,14 @@ needing to read the zeros.  This optimization is not available if the
 Directory Selection}), unless the @option{-z} (@option{--null-data})
 option is also used (@pxref{Other Options}).
 
+@cindex pipelines and reading
+For efficiency @command{grep} does not always read all its input.
+For example, the shell command @samp{sed '/^...$/d' | grep -q X} can
+cause @command{grep} to exit immediately after reading a line
+containing @samp{X}, without bothering to read the rest of its input data.
+This in turn can cause @command{sed} to exit with a nonzero status because
+@command{sed} cannot write to its output pipe after @command{grep} exits.
+
 For more about the algorithms used by @command{grep} and about
 related string matching algorithms, see:
 
-- 
2.32.0

Subject: Patch for easier use in scripting pipelines
From: Ulrich Eckhardt <[email-masked]>
To: [email-masked]
Date: Thu, 17 Feb 2022 08:57:10 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Greetings!

The attached patch add a `--pipe` option to grep. When used, grep
only exits with with nonzero status on error. In particular, it
doesn't signal "match" / "no match" through the exit code.

Here's an example using Bash:

  # enable automatic error handling
  set -eo pipefail
  # grep for issues in a logfile to produce a report
  cat logfile | grep issue | sort --unique

If grep doesn't find "issue" in its input (which is not an error,
obviously), it exits with status 1. Bash interprets this nonzero exit
code as an error and terminates with an error itself.

In order to fix that bug in the above script, you currently have to
replace `grep ...` with `grep ... || [ $? = 1 ]`, which is not really
readable. As alternative, I've implemented a `--pipe` option, which
only returns nonzero on actual errors, but not when there is no match.
This is a bit of a complementary option to `--quiet`.

Open tasks here:
 * FSF paperwork is not finished, so obviously the patch can't be
   applied yet.
 * Should I add a `-p` to complement the long `--pipe`?
 * Should I call it `--pipe` at all? The other alternative I came up
   with was `--filter`. I don't really like either of them very much.

Cheers!

Uli
>From 02f00ec50e820b9320af66ee7bb47bab36184f96 Mon Sep 17 00:00:00 2001
From: Ulrich Eckhardt <[email-masked]>
Date: Mon, 14 Feb 2022 19:23:10 +0100
Subject: [PATCH 1/4] grep: implement `--pipe` option

* src/grep.c: Implement `--pipe` option, which does not indicate
matches or lack thereof in the exit code but only filters the input
into the output.
---
 src/grep.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/grep.c b/src/grep.c
index 9c933e4..2260d7d 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -498,7 +498,8 @@ enum
   INCLUDE_OPTION,
   LINE_BUFFERED_OPTION,
   LABEL_OPTION,
-  NO_IGNORE_CASE_OPTION
+  NO_IGNORE_CASE_OPTION,
+  PIPE_OPTION
 };
 
 /* Long options equivalences. */
@@ -543,6 +544,7 @@ static struct option const long_options[] =
   {"null", no_argument, NULL, 'Z'},
   {"null-data", no_argument, NULL, 'z'},
   {"only-matching", no_argument, NULL, 'o'},
+  {"pipe", no_argument, NULL, PIPE_OPTION},
   {"quiet", no_argument, NULL, 'q'},
   {"recursive", no_argument, NULL, 'r'},
   {"dereference-recursive", no_argument, NULL, 'R'},
@@ -1078,6 +1080,7 @@ static enum
 static int out_file;
 
 static int filename_mask;	/* If zero, output nulls after filenames.  */
+static bool out_pipe;		/* --pipe: Don't set exit status depending on match. */
 static bool out_quiet;		/* Suppress all normal output. */
 static bool out_invert;		/* Print nonmatching stuff. */
 static bool out_line;		/* Print line numbers. */
@@ -2017,6 +2020,7 @@ Output control:\n\
 "));
       printf (_("\
   -o, --only-matching       show only nonempty parts of lines that match\n\
+      --pipe                only filter, do not set exit value depending on found matches\n\
   -q, --quiet, --silent     suppress all normal output\n\
       --binary-files=TYPE   assume that binary files are TYPE;\n\
                             TYPE is 'binary', 'text', or 'without-match'\n\
@@ -2698,6 +2702,10 @@ main (int argc, char **argv)
         only_matching = true;
         break;
 
+      case PIPE_OPTION:
+        out_pipe = true;
+        break;
+
       case 'q':
         exit_on_match = true;
         exit_failure = 0;
@@ -3008,5 +3016,9 @@ main (int argc, char **argv)
     status &= grep_command_line_arg (*files++);
   while (*files != NULL);
 
-  return errseen ? EXIT_TROUBLE : status;
+  if (errseen)
+    return EXIT_TROUBLE;
+  if (out_pipe)
+    return EXIT_SUCCESS;
+  return status;
 }
-- 
2.32.0

>From e930e55fd34f02bd9959148c1d43a3fe521e2169 Mon Sep 17 00:00:00 2001
From: Ulrich Eckhardt <[email-masked]>
Date: Mon, 14 Feb 2022 21:36:39 +0100
Subject: [PATCH 2/4] tests: add tests for --pipe

* tests/pipe: Add new test file exercising the `--pipe` option.
* tests/Makefile.am: Register new test file.
---
 tests/Makefile.am |  1 +
 tests/pipe        | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100755 tests/pipe

diff --git a/tests/Makefile.am b/tests/Makefile.am
index a211cb6..ae8ffc8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -148,6 +148,7 @@ TESTS =						\
   pcre-w					\
   pcre-wx-backref				\
   pcre-z					\
+  pipe						\
   posix-bracket					\
   prefix-of-multibyte				\
   proc						\
diff --git a/tests/pipe b/tests/pipe
new file mode 100755
index 0000000..9c789bb
--- /dev/null
+++ b/tests/pipe
@@ -0,0 +1,42 @@
+#!/bin/sh
+# tests for --pipe commandline option
+# A missing or found match does not influence the exit status.
+
+. "${srcdir=.}/init.sh"
+path_prepend_ ../src
+
+require_timeout_
+
+failures=0
+
+# should return 0 even though it doesn't find a match in stdin
+echo -n '' | LC_ALL=C timeout 10s grep --pipe abcd
+if test $? -ne 0 ; then
+    echo 'Status: Wrong status code, test #1 failed'
+    failures=1
+fi
+
+# should return 0 even though it doesn't find a match in the input file
+echo -n '' > in
+LC_ALL=C timeout 10s grep --pipe abcd in
+if test $? -ne 0 ; then
+    echo 'Status: Wrong status code, test #2 failed'
+    failures=1
+fi
+
+# should return 0 even though it doesn't find a match in stdin
+echo -n '' | LC_ALL=C timeout 10s grep --invert-match --pipe abcd
+if test $? -ne 0 ; then
+    echo 'Status: Wrong status code, test #3 failed'
+    failures=1
+fi
+
+# should return 0 even though it doesn't find a non-match in the input file
+echo -n '' > in
+LC_ALL=C timeout 10s grep --invert-match --pipe abcd in
+if test $? -ne 0 ; then
+    echo 'Status: Wrong status code, test #4 failed'
+    failures=1
+fi
+
+Exit $failures
-- 
2.32.0

>From 7bcc7fb748be62bf87be15e5d1f78d54b6c10823 Mon Sep 17 00:00:00 2001
From: Ulrich Eckhardt <[email-masked]>
Date: Tue, 15 Feb 2022 20:46:12 +0100
Subject: [PATCH 3/4] doc: update manpage

* doc/grep.in.1: Add --pipe manpage documentation.
---
 doc/grep.in.1 | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/doc/grep.in.1 b/doc/grep.in.1
index 372b892..89aa5fc 100644
--- a/doc/grep.in.1
+++ b/doc/grep.in.1
@@ -357,6 +357,14 @@ non-matching lines.
 Print only the matched (non-empty) parts of a matching line,
 with each such part on a separate output line.
 .TP
+.B \-\^\-pipe
+For use in pipelines. Just apply the configured input filters but
+don't set any exit status depending on found matches. Not matching
+the search pattern is not always an error, so don't report it like
+one. The use is shell scripting (e.g. Bash's `set -o pipefail`),
+which otherwise requires explicitly checking for exit codes 0 or 1
+in the middle of a pipeline.
+.TP
 .BR \-q ", " \-\^\-quiet ", " \-\^\-silent
 Quiet; do not write anything to standard output.
 Exit immediately with zero status if any match is found,
@@ -971,7 +979,11 @@ or
 or
 .B \-\^\-silent
 is used and a line is selected, the exit status is 0 even if an error
-occurred.
+occurred.  With the
+.B \-\^\-pipe
+option, the exit status is 2 when an error occurred and 0 otherwise.
+Other @command{grep} implementations may exit with status greater than
+2 on error.
 .
 .SH ENVIRONMENT
 The behavior of
-- 
2.32.0

>From 4fd80ac450dc2429dd03e803c9a8a729307ba069 Mon Sep 17 00:00:00 2001
From: Ulrich Eckhardt <[email-masked]>
Date: Tue, 15 Feb 2022 20:57:12 +0100
Subject: [PATCH 4/4] doc: update texinfo

* doc/grep.texi: Add --pipe documentation.
---
 doc/grep.texi | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/grep.texi b/doc/grep.texi
index 37ef839..b2a98bd 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -387,6 +387,16 @@ Output lines use the same delimiters as input, and delimiters are null
 bytes if @option{-z} (@option{--null-data}) is also used (@pxref{Other
 Options}).
 
+@item --pipe
+@opindex --pipe
+@cindex --pipe
+For use in pipelines. Just apply the configured input filters but
+don't set any exit status depending on found matches. Not matching
+the search pattern is not always an error, so don't report it like
+one. The use is shell scripting (e.g. Bash's `set -o pipefail`),
+which otherwise requires explicitly checking for exit codes 0 or 1
+in the middle of a pipeline.
+
 @item -q
 @itemx --quiet
 @itemx --silent
@@ -1093,8 +1103,9 @@ Normally the exit status is 0 if a line is selected, 1 if no lines
 were selected, and 2 if an error occurred.  However, if the
 @option{-q} or @option{--quiet} or @option{--silent} option is used
 and a line is selected, the exit status is 0 even if an error
-occurred.  Other @command{grep} implementations may exit with status
-greater than 2 on error.
+occurred.  With the @option{--pipe} option, the exit status is 2
+when an error occurred and 0 otherwise. Other @command{grep}
+implementations may exit with status greater than 2 on error.
 
 @node grep Programs
 @section @command{grep} Programs
-- 
2.32.0