GNU bug report logs - #17494
grep bug report: "GREP_OPTIONS" allows a user to break shell script

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17494 in the body.
You can then email your comments to 17494 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Nadav Har'El" <nyh <at> cloudius-systems.com>
To: bug-grep <at> gnu.org
Cc: Osv Dev <osv-dev <at> googlegroups.com>
Subject: grep bug report: "GREP_OPTIONS" allows a user to break shell script
Date: Thu, 15 May 2014 01:01:24 +0300

[Message part 1 (text/plain, inline)]

Hi,

An unsuspecting user who reads the grep(1) manual page and discovers
the GREP_OPTIONS environment variable, might thing it is a cool idea to
set it. For example, set GREP_OPTIONS=-n to always get numbers on the
output, great isn't it?

Well, it's actually a really bad idea to export GREP_OPTIONS=-n, or anything
similar, because this will effect not only the user's interactive work in
his shell,
but also likely break any shell-script, makefile, and so on, which happens
to
use "grep" and assume that it behaves normally. Imagine what kind a mess
a user could cause by setting GREP_OPTIONS=-v :-)

Instead of setting GREP_OPTIONS hoping it will only effect his interactive
session, what the user should have done is to use an *alias*, e.g.,

    alias grep='grep -n'

In fact on my Fedora installation, there is already a similar alias by
default:

    alias grep='grep --color=auto'

These aliases (when put in .bashrc or similar) are safe - they only effect
the user's interactive shell, and *not* shell scripts or makefiles, which
use the normal grep as they expect.

So I think it would be best if you drop the GREP_OPTIONS feature
completely. Or if for some reason you really don't want to drop it,
please at least explain in the manual page why it is dangerous
and aliases should be preferred.

An example of the kind of damage the availability of this option can do
is that today, a user of an open-source project I'm working on (OSv),
asked us to replace every use of "grep" in our makefile to calls to
"GREP_OPTIONS= grep", so that grep will continue to work normally
even for people who set GREP_OPTIONS.

A final remark: Other command-line utilities, such as "ls", "rm", "cat"
and so on, also don't have a .._OPTIONS variable of the sort grep has.
Imagine what kind of havoc users could cause to shell scripts if they
could do RM_OPTIONS=-i. Instead, what users have been doing for
many years is to alias ls, rm, and so on. They can change "rm" to "rm -i"
for themselves, but it won't mess with non-interactive shell-scripts.
There is no reason, I think, why grep needs to be any different.

Thanks,
Nadav.

-- 
Nadav Har'El
nyh <at> cloudius-systems.com

[Message part 2 (text/html, inline)]

Message #8 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Nadav Har'El <nyh <at> cloudius-systems.com>, 17494 <at> debbugs.gnu.org
Cc: Osv Dev <osv-dev <at> googlegroups.com>
Subject: Re: bug#17494: grep bug report: "GREP_OPTIONS" allows a user to break
 shell script
Date: Wed, 14 May 2014 22:08:05 -0700

Nadav Har'El wrote:
> So I think it would be best if you drop the GREP_OPTIONS feature
> completely.

I tend to agree.  Hindsight is wonderful of course, but GREP_OPTIONS 
should never have been added, as it's a real minefield.  But how would 
we minimize breakage of existing usage that relies on GREP_OPTIONS?  If 
we removed it, we'd probably need a transition period, and have 
documentation of how to get the old behavior if you want it, etc.

Message #11 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: "Nadav Har'El" <nyh <at> cloudius-systems.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17494 <at> debbugs.gnu.org, Osv Dev <osv-dev <at> googlegroups.com>
Subject: Re: bug#17494: grep bug report: "GREP_OPTIONS" allows a user to break
 shell script
Date: Thu, 15 May 2014 10:18:14 +0300

[Message part 1 (text/plain, inline)]

On Thu, May 15, 2014 at 8:08 AM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:

> Nadav Har'El wrote:
>
>> So I think it would be best if you drop the GREP_OPTIONS feature
>> completely.
>>
>
> I tend to agree.  Hindsight is wonderful of course, but GREP_OPTIONS
> should never have been added, as it's a real minefield.


Yes, as they say, hindsight is always 20/20 ;-)


>  But how would we minimize breakage of existing usage that relies on
> GREP_OPTIONS?  If we removed it, we'd probably need a transition period,
> and have documentation of how to get the old behavior if you want it, etc.
>

I think deprecating and un-recommending this option in the manual page will
be a good start (and I just now noticed that
https://savannah.gnu.org/bugs/?32501 asked for the same thing). I think
that the only reason people use this variable is that they read through the
manual page, and thought it would be cool to set it to something like
"--with-color -n". So the manual page should be clear that it's not cool to
use it, and what the alternative is.

I don't mind that people use such an option if they are well aware of what
they are doing, and if they are aware that it is their environment which is
broken, not the shell scripts they use, and don't expect every shell script
which uses grep to begin with "unset GREP_OPTIONS".

Here is a patch I propose to the grep(1) manual page:

diff --git a/grep.1.orig b/grep.1
index fb99665..9d2a8d3 100644
--- a/grep.1.orig
+++ b/grep.1
@@ -875,6 +875,21 @@ 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.
+
+This variable is deprecated, and should be avoided. Setting it changes not
+only the behavior of
+.B grep
+in interactive shell sessions, but may also break shell scripts and
makefiles
+which assume that
+.B grep
+behaves normally. A user that wishes to alter the default
+.B grep
+behavior only for interactive sessions should instead use the
+.I alias
+feature provided by the shell, e.g.,
+.EX
+alias grep='grep --color=auto -n'
+.EE
 .TP
 .B GREP_COLOR
 This variable specifies the color used to highlight matched (non-empty)
text.




-- 
Nadav Har'El
nyh <at> cloudius-systems.com

[Message part 2 (text/html, inline)]

Message #14 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17494 <at> debbugs.gnu.org, Osv Dev <osv-dev <at> googlegroups.com>,
 Nadav Har'El <nyh <at> cloudius-systems.com>
Subject: Re: bug#17494: grep bug report: "GREP_OPTIONS" allows a user to break
 shell script
Date: Thu, 15 May 2014 21:53:15 +0900

I also think GREP_OPTIONS should be removed, but I'm unwilling that I
won't be able to ignore the directory without a command line option.

I use GREP_OPTION='-d skip' to ignore directories by default in grep 2.11
or laster. (They were ignored without a option in 2.10 or former and
other imprements of grep on Solaris, HP-UX, etc.)

Even if I define an alias "grep='grep -d skip'", when "find . | xargs grep"
doesn't ignore directories.

Norihiro

Message #17 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: "Nadav Har'El" <nyh <at> cloudius-systems.com>
To: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
Cc: 17494 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>,
 Osv Dev <osv-dev <at> googlegroups.com>
Subject: Re: bug#17494: grep bug report: "GREP_OPTIONS" allows a user to break
 shell script
Date: Thu, 15 May 2014 16:02:02 +0300

[Message part 1 (text/plain, inline)]

On Thu, May 15, 2014 at 3:53 PM, Norihiro Tanaka <noritnk <at> kcn.ne.jp> wrote:

>
> Even if I define an alias "grep='grep -d skip'", when "find . | xargs grep"
> doesn't ignore directories.
>

Since find does output directories, and clearly you don't want to grep on
them, why not just do something like

find . ! -type d | xargs grep

To be even safer and work with any file name (containing even newlines),
you actually need to do

find . ! -type d -print0 | xargs -0 grep

So maybe you just want to alias this whole thing :-)

I don't think this a reason to keep GREP_OPTIONS, but like I said I don't
mind if it is kept if at least people are warned about how it might break
shell scripts.


-- 
Nadav Har'El
nyh <at> cloudius-systems.com

[Message part 2 (text/html, inline)]

Message #22 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Nadav Har'El <nyh <at> cloudius-systems.com>, 17494 <at> debbugs.gnu.org
Subject: Re: "GREP_OPTIONS" allows a user to break shell script
Date: Thu, 11 Sep 2014 14:21:54 -0700

[Message part 1 (text/plain, inline)]

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?

[0001-grep-make-GREP_OPTIONS-obsolescent.patch (text/x-patch, attachment)]

Message #27 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17494 <at> debbugs.gnu.org, Nadav Har'El <nyh <at> cloudius-systems.com>
Subject: Re: bug#17494: "GREP_OPTIONS" allows a user to break shell script
Date: Fri, 12 Sep 2014 08:24:15 -0700

On Thu, Sep 11, 2014 at 2:21 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> 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?

That looks fine. Long overdue. Thank you for writing it.

Message #32 received at 17494-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>
Cc: 17494-done <at> debbugs.gnu.org, Nadav Har'El <nyh <at> cloudius-systems.com>
Subject: Re: bug#17494: "GREP_OPTIONS" allows a user to break shell script
Date: Fri, 12 Sep 2014 09:05:51 -0700

Jim Meyering wrote:

> That looks fine. Long overdue. Thank you for writing it.

You're welcome, and I applied it and am marking this as done.  Other GNU 
utilities probably need similar changes at some point.

Message #35 received at 17494 <at> debbugs.gnu.org (full text, mbox):

From: "Nadav Har'El" <nyh <at> cloudius-systems.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17494 <at> debbugs.gnu.org
Subject: Re: "GREP_OPTIONS" allows a user to break shell script
Date: Sun, 14 Sep 2014 00:37:49 +0300

[Message part 1 (text/plain, inline)]

On Fri, Sep 12, 2014 at 12:21 AM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:

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

Looks good to me. However, it seems to me that if grep prints this
"warning:" every time grep is used, we don't just deprecate this feature
(with a promise to remove it later), we actually make this feature unusable
immediately, because nobody wants to see this message on every invocation
of grep.

But I'm personally fine with this change, if you are.

Thanks,
Nadav.



-- 
Nadav Har'El
nyh <at> cloudius-systems.com

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.