Re: [PATCH] mingw: allow hooks to be .exe files

From: Stefan Beller <sbeller@google.com>
Date: Jan 27, 2017, 18:24 UTC
Message-ID: <CAGZ79kaa5WJmZkyFROfkfNb3++t37qAuAebKJTon2iD2bh+sWw@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1701271127520.3469@virtualbox>

On Fri, Jan 27, 2017 at 2:29 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 35 quoted lines

> Hi Junio,
>
> On Thu, 26 Jan 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > On Wed, 25 Jan 2017, Jeff King wrote:
>> >
>> >> On Wed, Jan 25, 2017 at 05:58:42PM +0100, Johannes Schindelin wrote:
>> >>
>> >> > -        if (access(path.buf, X_OK) < 0)
>> >> > +        if (access(path.buf, X_OK) < 0) {
>> >> > +#ifdef STRIP_EXTENSION
>> >> > +                strbuf_addstr(&path, ".exe");
>> >>
>> >> I think STRIP_EXTENSION is a string.  Should this line be:
>> >>
>> >>   strbuf_addstr(&path, STRIP_EXTENSION);
>> >
>> > Yep.
>> >
>> > v2 coming,
>> > Johannes
>>
>> I think I've already tweaked it out when I queued the original one.
>
> After digging, I found your SQUASH commit. I had not known about that.
>
> In any case, I much rather prefer to have the final version of any patch
> or patch series I contribute to be identical between what you commit and
> what I sent to the mailing list. We do disagree from time to time, and I
> would like to have the opportunity of reviewing how you tweak my changes.
>
> Ciao,
> Johannes

I saw the queued up commit and that lead me to this thread here. The commit message, while technically correct, seems a bit off. This is because the commit message only talks about .exe extensions, but the change in code doesn't even have the string "exe" mentioned once.

With this discussion here, it is easy for me to connect the dots, but it would be nice to have the full picture in the commit message. This is what I would write:

    mingw: allow hooks to be .exe files

    Executable files in Windows need to have the extension '.exe', otherwise
    they do not work. Extend the hooks to not just look at the hard coded names,
    but also at the extended names via the custom STRIP_EXTENSION, which
    is defined as '.exe' in Windows.

Stefan