Subject: Re: "GREP_OPTIONS" allows a user to break shell script

From: Paul Eggert <[email-masked]>
Date: Thu, 11 Sep 2014 14:21:54 -0700

Attached is a proposed patch that starts to address this longstanding 
problem by deprecating GREP_OPTIONS and warning if it's used.  We can 
remove the dangerous feature in a later release.  Comments?
>From be556580362361bbcbef5cd99e849bd99f6f0960 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 11 Sep 2014 14:16:48 -0700
Subject: [PATCH] grep: make GREP_OPTIONS obsolescent

* NEWS:
* doc/grep.in.1 (ENVIRONMENT_VARIABLES):
* doc/grep.texi (Environment Variables):
Document that GREP_OPTIONS is obsolescent now.
* src/grep.c (main): Warn if GREP_OPTIONS is used.
* tests/r-dot, tests/skip-device: Don't use GREP_OPTIONS.
---
 NEWS              |  3 +++
 doc/grep.in.1     | 18 ++++++------------
 doc/grep.texi     | 19 +++----------------
 src/grep.c        |  5 +++++
 tests/r-dot       |  3 +--
 tests/skip-device |  3 ---
 6 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/NEWS b/NEWS
index ca79525..05edd65 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,9 @@ GNU grep NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
+  The GREP_OPTIONS environment variable is now obsolescent, and grep
+  now warns if it is used.  Please use an alias or script instead.
+
   grep --exclude-dir='FOO/' now excludes the directory FOO.
   Previously, the trailing slash meant the option was ineffective.
 
diff --git a/doc/grep.in.1 b/doc/grep.in.1
index 63c6081..00d030a 100644
--- a/doc/grep.in.1
+++ b/doc/grep.in.1
@@ -851,19 +851,13 @@ was not compiled with national language support (\s-1NLS\s0).
 .B GREP_OPTIONS
 This variable specifies default options
 to be placed in front of any explicit options.
-For example, if
-.B GREP_OPTIONS
-is
-.BR "'\-\^\-binary-files=without-match \-\^\-directories=skip'" ,
-.B grep
-behaves as if the two options
-.B \-\^\-binary\-files=without-match
+As this causes problems when writing portable scripts,
+this feature will be removed in a future release of
+.BR grep ,
 and
-.B \-\^\-directories=skip
-had been specified before any explicit options.
-Option specifications are separated by whitespace.
-A backslash escapes the next character,
-so it can be used to specify an option containing whitespace or a backslash.
+.B grep
+warns if it is used.
+Please use an alias or script instead.
 .TP
 .B GREP_COLOR
 This variable specifies the color used to highlight matched (non-empty) text.
diff --git a/doc/grep.texi b/doc/grep.texi
index 02181b0..c8e4acd 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -824,22 +824,9 @@ the @code{terminfo} library.
 @cindex default options environment variable
 This variable specifies default options to be placed in front of any
 explicit options.
-For example, if @env{GREP_OPTIONS} is
-@samp{--binary-files=without-match --directories=skip}, @command{grep}
-behaves as if the two options @samp{--binary-files=without-match} and
-@samp{--directories=skip} had been specified before
-any explicit options.
-Option specifications are separated by
-whitespace.
-A backslash escapes the next character, so it can be used to
-specify an option containing whitespace or a backslash.
-
-The @env{GREP_OPTIONS} value does not affect whether @command{grep}
-without file operands searches standard input or the working
-directory; that is affected only by command-line options.  For
-example, the command @samp{grep PAT} searches standard input and the
-command @samp{grep -r PAT} searches the working directory, regardless
-of whether @env{GREP_OPTIONS} contains @option{-r}.
+As this causes problems when writing portable scripts, this feature
+will be removed in a future release of @command{grep}, and @command{grep}
+warns if it is used.  Please use an alias or script instead.
 
 @item GREP_COLOR
 @vindex GREP_COLOR @r{environment variable}
diff --git a/src/grep.c b/src/grep.c
index 6b930dc..1f801e9 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -1973,7 +1973,12 @@ main (int argc, char **argv)
   atexit (clean_up_stdout);
 
   last_recursive = 0;
+
   prepended = prepend_default_options (getenv ("GREP_OPTIONS"), &argc, &argv);
+  if (prepended)
+    error (0, 0, _("warning: GREP_OPTIONS is deprecated;"
+                   " please use an alias or script"));
+
   compile = matchers[0].compile;
   execute = matchers[0].execute;
 
diff --git a/tests/r-dot b/tests/r-dot
index a1d5c0a..29aedad 100755
--- a/tests/r-dot
+++ b/tests/r-dot
@@ -14,8 +14,7 @@ compare exp out || fail=1
 (cd dir && grep -r aaa < a) > out || fail=1
 compare exp out || fail=1
 
-echo aaa > exp || framework_failure_
-(cd dir && GREP_OPTIONS=-r grep aaa < a) > out || fail=1
+(cd dir && grep -r aaa *) > out || fail=1
 compare exp out || fail=1
 
 Exit $fail
diff --git a/tests/skip-device b/tests/skip-device
index 40645ea..32663fe 100755
--- a/tests/skip-device
+++ b/tests/skip-device
@@ -8,7 +8,4 @@
 echo foo | grep -D skip foo - || fail=1
 echo foo | grep --devices=skip foo || fail=1
 
-# It's more insidious when the skip option is via the envvar:
-echo foo | GREP_OPTIONS=--devices=skip grep foo || fail=1
-
 Exit $fail
-- 
1.9.3