Re: [PATCH v4 6/8] cmake: support for building git on windows with mingw

From: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
Date: Jun 18, 2020, 20:40 UTC
Message-ID: <CAKiG+9UqvrxyMQufuG=oAP6Yo7+YSoicwqZBJjsZZFQT-Bscjw@mail.gmail.com>
In-Reply-To: <xmqqk104nnll.fsf@gitster.c.googlers.com>

On Fri, Jun 19, 2020 at 1:38 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

>
> Sibi Siddharthan <sibisiddharthan.github@gmail.com> writes:
>
> > Since this patch series has been merged with pu, I didn't know whether
> > I should wait till the patch gets merged into 'next' or do the change
> > immediately.
>
> Ah, OK.  Being in 'pu' does not mean no more than that the patch was
> sent to the list and I happened to have seen it.  If it were ready
> to be merged to 'next', I may have marked it as such in the "What's
> cooking" report, but otherwise, the default is to be polished until
> it gets ready.
>
> > One more thing, there is an issue with the scripts' permissions when
> > run in Linux. They don't have execute permissions.
>
> What script?  Your scripts you add in the patch series?  What is the
> reason why they lack execute permissions?  Forgot to "chmod +x"?
>
> It sounds like, in addition to issues pointed out during the review
> cycle, you have identified more issues that can be solved to make
> the series more complete yourself, which is a very good thing.  It
> is hard for anyone to review one's own patches and find issues, and
> you seem to be getting the hang of it.  These are all good thing to
> address before the topic becomes ready for 'next'.
>

Danh was the one who pointed this out, credit goes to him. The reason I deferred modifying in PATCH v4 was because there was no easy way(cross platform) to change file permissions. The workaround is to juggle the files to a temporary directory, and then copy them to where they are intended to be with the required permissions. This added quite a bit of code. Since Windows platform was the priority, I did not address this issue.

I know that this issue needs to be addressed to make the script more complete, so will have a UNIX conditional block for addressing this issue.

Thank You, Sibi Siddharthan

Show 8 quoted lines

> And there is no need to hurry.  If you do not want to waste time in
> repeated rewrite and review cycle, the best way may be to go slowly
> and carefully to avoid "this was known to be suboptimal even when I
> wrote it, but I didn't bother fixing it before sending it out, but
> it was noticed during the review so I have to update it and send a
> new round".
>
> Thanks.