[patch] "which"-builtin writes diagnostics to stdout

[patch] "which"-builtin writes diagnostics to stdout

[Timo Buhrmester]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

When invoking the `which` builtin for something that does not exist, like:

| % which doesnotexist
| doesnotexist not found

the "doesnotexist not found" message goes to standard output, rather than standard error.

This is of course wrong, and particularly nasty in command-substitution, where the diagnostic message might end up in a variable, be given to an external program as an arguemnt or even be be fed through a pipeline.

The below (and also attached) patch fixes that.


Cheers,

Timo Buhrmester

---------8<------------------------------------------------

diff --git a/Src/builtin.c b/Src/builtin.c
index 97022ad..774390a 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3578,8 +3578,9 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 		}
 	    }
 	    if (!informed && (wd || v || csh)) {
-		zputs(*argv, stdout);
-		puts(wd ? ": none" : " not found");
+		zputs(*argv, stderr);
+		fputs(wd ? ": none" : " not found", stderr);
+		fputc('\n', stderr);
 		returnval = 1;
 	    }
 	    popheap();
@@ -3598,8 +3599,11 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    informed = 1;
 	} else {
 	    /* Not found at all. */
-	    if (v || csh || wd)
-		zputs(*argv, stdout), puts(wd ? ": none" : " not found");
+	    if (v || csh || wd) {
+		zputs(*argv, stderr);
+		fputs(wd ? ": none" : " not found", stderr);
+		fputc('\n', stderr);
+	    }
 	    returnval = 1;
 	}
     }

[-- Attachment #2: zsh-which-stderr.patch --]
[-- Type: text/plain, Size: 847 bytes --]

diff --git a/Src/builtin.c b/Src/builtin.c
index 97022ad..774390a 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3578,8 +3578,9 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 		}
 	    }
 	    if (!informed && (wd || v || csh)) {
-		zputs(*argv, stdout);
-		puts(wd ? ": none" : " not found");
+		zputs(*argv, stderr);
+		fputs(wd ? ": none" : " not found", stderr);
+		fputc('\n', stderr);
 		returnval = 1;
 	    }
 	    popheap();
@@ -3598,8 +3599,11 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    informed = 1;
 	} else {
 	    /* Not found at all. */
-	    if (v || csh || wd)
-		zputs(*argv, stdout), puts(wd ? ": none" : " not found");
+	    if (v || csh || wd) {
+		zputs(*argv, stderr);
+		fputs(wd ? ": none" : " not found", stderr);
+		fputc('\n', stderr);
+	    }
 	    returnval = 1;
 	}
     }

Re: [patch] "which"-builtin writes diagnostics to stdout

[Bart Schaefer]

On Sat, Aug 29, 2015 at 8:06 PM, Timo Buhrmester <fstd.lkml@gmail.com> wrote:
> When invoking the `which` builtin for something that does not exist, like:
>
> | % which doesnotexist
> | doesnotexist not found
>
> the "doesnotexist not found" message goes to standard output, rather than standard error.

Believe it or not, this is intentional, because the original "which"
builtin from csh does that.  Try it in tcsh, for example.

Re: [patch] "which"-builtin writes diagnostics to stdout

[Timo Buhrmester]

> > | % which doesnotexist
> > | doesnotexist not found
> >
> > the "doesnotexist not found" message goes to standard output, rather than standard error.
> 
> Believe it or not, this is intentional, because the original "which"
> builtin from csh does that.  Try it in tcsh, for example.
Uh, okay.

Out of curiosity, what reason would zsh (being more bourne-ish than csh-ish, as far as I can tell) have to maintain this particular csh glitch rather than fixing it?
How many csh scripts that run on zsh are out in the wild?  How many csh scripts are out in the wild at all? :-)

Timo Buhrmester

Re: [patch] "which"-builtin writes diagnostics to stdout

[Bart Schaefer]

On Aug 30,  7:26am, Timo Buhrmester wrote:
} Subject: Re: [patch] "which"-builtin writes diagnostics to stdout
}
} > > | % which doesnotexist
} > > | doesnotexist not found
} > >
} > > the "doesnotexist not found" message goes to standard output,
} > > rather than standard error.
}
} Out of curiosity, what reason would zsh (being more bourne-ish than
} csh-ish, as far as I can tell) have to maintain this particular csh
} glitch rather than fixing it?

What makes you think it's a glitch?

I.e., "not found" here is not an error message!  It is the answer to the
question you asked, and you asked for the answer in csh format ("which"
is "whence -c").  If you don't want csh behavior, you should be using
"whence" directly so that you can omit the "-c" option.

Re: [patch] "which"-builtin writes diagnostics to stdout

[Mikael Magnusson]

On Tue, Sep 1, 2015 at 8:04 AM, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Aug 30,  7:26am, Timo Buhrmester wrote:
> } Subject: Re: [patch] "which"-builtin writes diagnostics to stdout
> }
> } > > | % which doesnotexist
> } > > | doesnotexist not found
> } > >
> } > > the "doesnotexist not found" message goes to standard output,
> } > > rather than standard error.
> }
> } Out of curiosity, what reason would zsh (being more bourne-ish than
> } csh-ish, as far as I can tell) have to maintain this particular csh
> } glitch rather than fixing it?
>
> What makes you think it's a glitch?
>
> I.e., "not found" here is not an error message!  It is the answer to the
> question you asked, and you asked for the answer in csh format ("which"
> is "whence -c").  If you don't want csh behavior, you should be using
> "whence" directly so that you can omit the "-c" option.

I guess the problem here is that in other shells, whence does not
exist, and which prints 'not found' to stderr. Zsh's which acts like
the csh variant even in the sh emulation, so finding the absolute path
of a command portably is pretty difficult. (In bash, both type and
which print 'not found' on stderr. In dash, which prints to stderr and
type prints to stdout. In zsh both print to stdout. type additionally
prints the extra 'foo is' first so it's less useful).

-- 
Mikael Magnusson

Re: [patch] "which"-builtin writes diagnostics to stdout

[Timo Buhrmester]

> } Out of curiosity, what reason would zsh (being more bourne-ish than
> } csh-ish, as far as I can tell) have to maintain this particular csh
> } glitch rather than fixing it?
> 
> What makes you think it's a glitch?
> 
> I.e., "not found" here is not an error message!  It is the answer to the
> question you asked
It seems to me like a diagnostic message intended for human consumption, mainly because there's an infinite number of ways it could answer the same thing (``I can' find <xyz>'' and ``no such <xyz>'', etc).  So unless on the other end of the pipeline you have a human, or a program that understands English, you'll have to deal with this as a special case.  Heck, I could even have a file named ``foobar not found'' sitting around, making things worse. :)

I obviously can't argue with behavior of ancient shells, but IMO there is exactly one correct, unambiguous and machine-readable answer to the ``which'' question:
The empty answer.  

> and you asked for the answer in csh format ("which" is "whence -c").
> If you don't want csh behavior, you should be using "whence" directly
> so that you can omit the "-c" option.
Okay, I didn't know about that.
Doesn't make my point less valid, but I'm glad there's a way out with `whence`.  Thanks.


Timo

Re: [patch] "which"-builtin writes diagnostics to stdout

[Peter Stephenson]

On Sat, 29 Aug 2015 22:15:38 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sat, Aug 29, 2015 at 8:06 PM, Timo Buhrmester <fstd.lkml@gmail.com> wrote:
> > When invoking the `which` builtin for something that does not exist, like:
> >
> > | % which doesnotexist
> > | doesnotexist not found
> >
> > the "doesnotexist not found" message goes to standard output, rather than standard error.
> 
> Believe it or not, this is intentional, because the original "which"
> builtin from csh does that.  Try it in tcsh, for example.

This is an obvious candidate for hiding behind one of the options like
CSH_JUNKIE_STUFF_THAT_NO_ONE_COULD_CONCEIVABLY_WANT_ANY_MORE on the
grounds of consistency --- it's hard to see how sending the error to
stder could make anything other than a script/function deliberately set
up to expect csh output any more confused --- though I'm not that set on
the option name.

pws

Re: [patch] "which"-builtin writes diagnostics to stdout

[Bart Schaefer]

On Sep 1,  2:45pm, Peter Stephenson wrote:
}
} This is an obvious candidate for hiding behind one of the options like
} CSH_JUNKIE_STUFF_THAT_NO_ONE_COULD_CONCEIVABLY_WANT_ANY_MORE on the
} grounds of consistency

I know we avoid this in most cases, but I'd be OK with having this
"option" be (emulation == EMULATE_CSH).

I still think the suggestion that someone might expect $(which foo) to be
the empty string is equally unlikely, or for that matter that anyone who
used $(which foo) would *want* to see the error message.  The right thing
is to test the exit status of "which".

Re: [patch] "which"-builtin writes diagnostics to stdout

[Bart Schaefer]

On Sep 1,  8:12am, Mikael Magnusson wrote:
}
} I guess the problem here is that in other shells, whence does not
} exist, and which prints 'not found' to stderr.

As far as I can tell, in other shells "which notacommand" prints NOTHING
on stdout or stderr.

} Zsh's which acts like the csh variant even in the sh emulation, so
} finding the absolute path of a command portably is pretty difficult.

Really?  This seems to work pretty well in bash/ksh/zsh (I don't have
dash/ash to try):

    if absolute_path="$(which $1)"
    then echo "Absolute path of $1 is $absolute_path"
    fi

Re: [patch] "which"-builtin writes diagnostics to stdout

[Peter Stephenson]

On Tue, 1 Sep 2015 16:16:30 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sep 1,  8:12am, Mikael Magnusson wrote:
> }
> } I guess the problem here is that in other shells, whence does not
> } exist, and which prints 'not found' to stderr.
> 
> As far as I can tell, in other shells "which notacommand" prints NOTHING
> on stdout or stderr.

Another sidelight on zsh's behaviour illustrating the peculiarity of
this case...

% which true foo cat
true: shell built-in command
foo not found
/bin/cat

If "foo not found" went to stderr that would make the output overall a
bit of a mess; it's not clear there's a good argument for complicating
it like that.  At least this looks to me like a good argument against
changing it.

pws

Re: [patch] "which"-builtin writes diagnostics to stdout

[Vincent Lefevre]

On 2015-09-02 09:53:17 +0100, Peter Stephenson wrote:
> On Tue, 1 Sep 2015 16:16:30 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Sep 1,  8:12am, Mikael Magnusson wrote:
> > }
> > } I guess the problem here is that in other shells, whence does not
> > } exist, and which prints 'not found' to stderr.
> > 
> > As far as I can tell, in other shells "which notacommand" prints NOTHING
> > on stdout or stderr.

The other shells (bash, dash, ksh93, mksh, posh) do not have a "which"
builtin:

  which -foo

gives the following error under GNU/Linux (Debian):

Illegal option -f
Usage: /usr/bin/which [-a] args

and I confirm that "which notacommand" prints nothing. The "which"
builtin from BusyBox v1.22.1 behaves in a similar way.

Under zsh, one can do

  alias which="whence -p"

and get the same behavior, AFAIK.

> Another sidelight on zsh's behaviour illustrating the peculiarity of
> this case...
> 
> % which true foo cat
> true: shell built-in command
> foo not found
> /bin/cat
> 
> If "foo not found" went to stderr that would make the output overall a
> bit of a mess; it's not clear there's a good argument for complicating
> it like that.  At least this looks to me like a good argument against
> changing it.

I agree. And I hardly see how the output from "which" (when it mean
"whence -c") can be useful in a script. If the user wants the pathname
to some command, he would need "whence -p".

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)