On Fri, 27 Jan 2012 10:41:45 +0100, Jonathan Nieder <jrnieder@xxxxxxxxx>
wrote:
Frans Klaver wrote:
Just for my understanding: before a command is executed, a pager
(less/more or so) is started? We want to avoid starting the pager if
we won't be able to execute the command?
See [1] for an example of a recent patch touching the relevant
code path.
For example: if I run "git --paginate foo", foo is an alias for bar,
and the "[pager] bar" configuration is set to point to "otherpager",
then without this safety git launches the default pager in preparation
for running git-foo, receives ENOENT from execvp("git-foo"), and then
the pager has already been launched and it is too late to launch
otherpager instead.
Took me a while to catch your drift, but if I understand correctly, you're
thinking using some of the code to find out if starting the pager is a
good idea or not. If I factor out the part that finds a command in PATH,
there's the helper that with a fair amount of certainty, will predict
whether 'git foo' will fail with ENOENT or not. It would fix a possible
problem that is currently there. Obviously the only case we can catch, is
the command not actually existing. Although it is just one of the cases
ENOENT can be returned for, I think it is the only one git actually cares
about when checking for it.
On Fri, Jan 27, 2012 at 9:48 AM, Jonathan Nieder <jrnieder@xxxxxxxxx>
wrote:
I want to like (b), but the downside seems unacceptable.
The downside being: having to figure out what execvp is going to do?
That would be tantamount to writing your own execvp.
Exactly.
So as it seems, there are a few cases where we can fairly reliably predict
whether a command is or isn't going to be found. Unless I'm mistaken,
dashed externals are never shell built-ins and so we don't have to be able
to check for their existence. Then assuming that silent_exec_failure
really only cares about commands actually not existing, we can be fairly
naive about it. See if we can find it somewhere in PATH and if we can't
bail out. If we can, start the pager and everything execvp then returns
will be regarded a fatal error. In this case it would be a choice between
spawning the wrong pager, or having a quick browse through the file system.
That's part of why I was really grateful to Hannes for the reminder to
take a step back for a moment and consider whether it's worth it.
It may be a sensible reminder. I didn't understand that comment as such.
Maybe it's Hannes' style, I don't know.
Maybe there's another way or a more targetted way to take care of the
motivational original confusing scenario that leads to execvp errors.
(By the way, can you remind me which one that was?)
Been thinking about it and I doubt it. To find out whether EACCES is
returned due to a PATH issue, you have to go through all of those PATH
entries. So while you're at it, there's a lot more you can check and most
of those checks are fairly trivial to do.
I think I've worked through all your review comments. I'll address Hannes'
comments, create an RFC series and see where we end up.
Junio, care to be CC'd in that?
Thanks,
Frans
[1] http://thread.gmane.org/gmane.comp.version-control.git/179635
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html