[PATCH] Scan for \r in addition to \n when reading shbang lines
Peter Harris
On Sat, Mar 1, 2008 Steffen Prohaska <proh...@zib.de> wrote:
>
> Hi,
> I uploaded the installer Git-preview20080301.exe, which includes a
> recent master of official Git, includes git svn, and has
> core.autocrlf=true as its default.
I downloaded and installed this version of git, used it to clone
git://repo.or.cz/msysgit.git, rebuilt the installer, and wondered why
"git commit" was failing with errors about vi.
After some digging, it turns out that "vi" in msysgit.git is a shell
script, and git was trying to launch "sh\r" instead of "sh", since the
vi script was autocrlf'd.
This patch fixes the problem.
compat/mingw.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 32bb5f3..bf8d60d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -424,8 +424,8 @@ static const char *parse_interpreter(const char *cmd)
if (buf[0] != '#' || buf[1] != '!')
return NULL;
buf[n] = '\0';
- p = strchr(buf, '\n');
- if (!p)
+ p = buf + strcspn(buf, "\r\n");
+ if (!*p)
return NULL;
*p = '\0';
--
1.5.3.7
Steffen Prohaska
On Mar 2, 2008, at 10:30 PM, Peter Harris wrote:
> On Sat, Mar 1, 2008 Steffen Prohaska <proh...@zib.de> wrote:
>>
>> Hi,
>> I uploaded the installer Git-preview20080301.exe, which includes a
>> recent master of official Git, includes git svn, and has
>> core.autocrlf=true as its default.
>
> I downloaded and installed this version of git, used it to clone
> git://repo.or.cz/msysgit.git, rebuilt the installer, and wondered why
> "git commit" was failing with errors about vi.
>
> After some digging, it turns out that "vi" in msysgit.git is a shell
> script, and git was trying to launch "sh\r" instead of "sh", since the
> vi script was autocrlf'd.
You should force the msysgit repository to core.autocrlf = false (or
maybe core.autocrlf = input). Even though msysgit runs on Windows it
is not a real cross-platform project. MinGW emulates a Unix
environment and thus you should override core.autocrlf.
> This patch fixes the problem.
I don't think this fix is needed.
Steffen
Johannes Schindelin
While I agree that MinGW itself (and thus the scripts that come with Git)
should be LF-only, it seems that many, many people actually like CR/LF.
And if they write hooks, chances are that they use a CR/LF-enamoured
editor to write them.
So, I think this patch is needed _and_ corelf = input is needed in /git.
Ciao,
Dscho
Steffen Prohaska
Ok, makes sense.
Steffen
Peter Harris
>
> On Sun, 2 Mar 2008, Steffen Prohaska wrote:
>
> > On Mar 2, 2008, at 10:30 PM, Peter Harris wrote:
> >
> > > On Sat, Mar 1, 2008 Steffen Prohaska <proh...@zib.de> wrote:
> > > >
> > >
> > > After some digging, it turns out that "vi" in msysgit.git is a shell
> > > script, and git was trying to launch "sh\r" instead of "sh", since the
> > > vi script was autocrlf'd.
> >
> > You should force the msysgit repository to core.autocrlf = false
>
> should be LF-only
In that case, shouldn't there be a .gitattributes files with "* -crlf"
in 4msysgit (and the .gitattributes file already in msysgit modified
to say * instead of msys.bat and cmd/*.cmd)?
Peter Harris
Steffen Prohaska
On Mar 3, 2008, at 4:58 PM, Johannes Schindelin wrote:
> Hi,
> Thanks! I completely forgot about crlf in .gitattributes. I like it.
>
> Steffen, do you agree?
In principle yes. However, .gitattributes might not work
properly when switching branches if the file .gitattributes
itself changes. Because of this, I try to avoid .gitattributes
as much as possible.
The only way that always works for me is to set core.autocrlf
before the first checkout. I stopped using clone but instead always
"clone" manually, that is
mkdir x
cd x
git init
git config core.autocrlf <you-choice-here>
git remote add origin <URL>
git fetch origin
git checkout ...
This sequence reliably works. Any other procedure, including
.gitattributes sometimes lead to unexpected results for me.
But after everyone has the right .gitattributes in his work tree
and the .gitattributes do not change, everything should work. So
I think we should tweak .gitattributes as proposed.
Steffen
Johannes Schindelin
On Mon, 3 Mar 2008, Peter Harris wrote:
Thanks! I completely forgot about crlf in .gitattributes. I like it.
Steffen, do you agree?
Ciao,
Dscho
Steffen Prohaska
On Mar 2, 2008, at 10:30 PM, Peter Harris wrote:
> ---
>
> On Sat, Mar 1, 2008 Steffen Prohaska <proh...@zib.de> wrote:
>>
>> Hi,
>> I uploaded the installer Git-preview20080301.exe, which includes a
>> recent master of official Git, includes git svn, and has
>> core.autocrlf=true as its default.
>
> I downloaded and installed this version of git, used it to clone
> git://repo.or.cz/msysgit.git, rebuilt the installer, and wondered why
> "git commit" was failing with errors about vi.
>
> After some digging, it turns out that "vi" in msysgit.git is a shell
> script, and git was trying to launch "sh\r" instead of "sh", since the
> vi script was autocrlf'd.
>
> This patch fixes the problem.
I applied the patch and pushed it to 4msysgit/pha/shbang, which is
based on mingw/master.
I merged pha/shbang to 4msysgit/devel.
Steffen