git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Frans Klaver" <fransklaver@gmail.com>
To: "Jonathan Nieder" <jrnieder@gmail.com>
Cc: git@vger.kernel.org, "Junio C. Hamano" <gitster@pobox.com>,
	"Johannes Sixt" <j6t@kdbg.org>
Subject: Re: [PATCH 3/5] run-command: Elaborate execvp error checking
Date: Wed, 25 Jan 2012 23:48:36 +0100	[thread overview]
Message-ID: <op.v8nwbai60aolir@keputer.lokaal> (raw)
In-Reply-To: <20120125192222.GH1824@burratino>

On Wed, 25 Jan 2012 20:22:22 +0100, Jonathan Nieder <jrnieder@gmail.com>  
wrote:

> Frans Klaver wrote:
>> Jonathan Nieder wrote:
>
>>> Could you give an example?
>>
>> The case that triggered me to work on this. I had an incorrect entry
>> in my PATH and some aliasing tests failed. The generated command
>> output was something like
>>
>> fatal: script: Access Denied
>
> Sorry for the lack of clarity.  I meant that a (precise) "before and
> after" example could make the commit message a lot easier to
> understand.

Ah I see. I'll add something along those lines.


> [...]
>>> What happens on Windows?
>>
>> I haven't changed anything on the windows side, so that probably
>> sticks to the old behavior.
>
> This was mostly a comment on the change description --- unless I look
> at the patch, if I try this out on Windows after reading the changelog
> I would end up utterly confused.  For patch 5/5, it also brings up
> worries about consistency --- if systems are going to be relying on a
> missing #! interpreter being treated differently from a missing script
> for the sake of silent_exec_failure, do the same considerations apply
> on Windows, too?

I'm actually not sure if scripts would be relying on this. There is of  
course a good chance that people actually will rely on it, regardless of  
what we think. If there are consistency concerns on different platforms  
I'd probably have to work on that as well. Mentioning that windows isn't  
affected by these changes would be a start though.

> Perhaps it's more along the lines of "this is not supposed to happen
> in practice, and when it does, humans will find it easier to debug if
> we error out hard instead of falling back to the 'if the command does
> not exist' behavior (e.g., by trying an alias next)".  In other words,
> maybe this is intended as an optional nicety rather than something
> scripts would ever rely on.

Exactly. My concern was primarily the human interaction, so getting at  
least some pointer to the cause of the error. Would that be nice to have  
on windows as well? It probably would.

  reply	other threads:[~2012-01-25 22:48 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-24 22:32 [PATCH 0/6 v3] Add execvp failure diagnostics Frans Klaver
2012-01-24 22:32 ` [PATCH 1/5] t0061: Fix incorrect indentation Frans Klaver
2012-01-24 22:39   ` Junio C Hamano
2012-01-24 22:40   ` Jonathan Nieder
2012-01-25  6:27     ` Frans Klaver
2012-01-25  7:00       ` Junio C Hamano
2012-01-25  7:08         ` Frans Klaver
2012-01-25  8:08       ` Frans Klaver
2012-01-24 22:32 ` [PATCH 2/5] t0061: Add tests Frans Klaver
2012-01-24 22:56   ` Jonathan Nieder
2012-01-25  6:47     ` Frans Klaver
2012-01-24 22:32 ` [PATCH 3/5] run-command: Elaborate execvp error checking Frans Klaver
2012-01-24 23:22   ` Jonathan Nieder
2012-01-25  7:09     ` Frans Klaver
2012-01-25 19:22       ` Jonathan Nieder
2012-01-25 22:48         ` Frans Klaver [this message]
2012-01-25 19:03   ` Johannes Sixt
2012-01-25 22:59     ` Frans Klaver
2012-01-24 22:32 ` [PATCH 4/5] run-command: Warn if PATH entry cannot be searched Frans Klaver
2012-01-24 22:32 ` [PATCH 5/5] run-command: Error out if interpreter not found Frans Klaver
2012-01-24 23:24   ` Jonathan Nieder
2012-01-25  7:12     ` Frans Klaver
2012-01-25 18:55       ` Johannes Sixt
2012-01-25 23:09         ` Frans Klaver
2012-01-26 19:32         ` Junio C Hamano
2012-01-27  8:29           ` Frans Klaver
2012-01-27  8:48             ` Jonathan Nieder
2012-01-27  9:11               ` Frans Klaver
2012-01-27  9:41                 ` Jonathan Nieder
2012-01-27 11:46                   ` Frans Klaver
2012-02-04 21:31                   ` Frans Klaver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=op.v8nwbai60aolir@keputer.lokaal \
    --to=fransklaver@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).