threads / discuss / 33979

t0008-ignores failure (was: Git for Windows 1.8.3)

Subject: t0008-ignores failure (was: Git for Windows 1.8.3)

## tl;dr

8 messages between May 30, 2013 and May 30, 2013.

replies: 7people: 6as markdown or json

Karsten Blees· May 30, 2013, 01:13 UTC · lore
Am 25.05.2013 21:16, schrieb Pat Thoyts:
> On that note -- with this merge as it now stands I get the following
> test failures:
> 
> t0008-ignores.sh                     155, 158, 162, 164
These tests fail because they use absolute paths, e.g. "C:/.../global-excludes", which is then translated to "C<NUL>/.../global-excludes". Can be fixed like so:
--- 8< ---
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -5,7 +5,7 @@ test_description=check-ignore
 . ./test-lib.sh

 init_vars () {
-       global_excludes="$(pwd)/global-excludes"
+       global_excludes="global-excludes"
 }

 enable_global_excludes () {
---

However, this raises the question whether colon is such a good choice as separator in 'git-check-ignore --verbose' output.

':' conflicts at least with Windows absolute paths and ADS names, and also with URLs (in case someone finds 'git ls-files --exclude-from=http://git-tricks.foo/special-exclude-file' useful enough to implement :-)

I realize colon was chosen to mimic git-check-attr, however, check-attr prints relative paths only (I think?).

How about using TAB or '|' instead? AFAICT, these are typically not used in paths or glob patterns.

Cheers,
Karsten
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Junio C Hamano· May 30, 2013, 02:21 UTC · re: Karsten Blees · lore

Re: t0008-ignores failure

Karsten Blees <karsten.blees@gmail.com> writes:
Show 5 quoted lines
> I realize colon was chosen to mimic git-check-attr, however,
> check-attr prints relative paths only (I think?).
>
> How about using TAB or '|' instead? AFAICT, these are typically
> not used in paths or glob patterns.

The primary reason to use ':' in "check-ignore -v" is to mimic the output format of "grep -n".

Emacs users can then run the commands like check-attr/check-ignore with "M-x find-grep" (or "M-x compile"), the output format is recognized by the editor, and the user can jump around with \C-x` to view hits.

I do not use vim myself, but I would be mildly surprised if there isn't a similar feature there.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Jeff King· May 30, 2013, 02:52 UTC · re: Junio C Hamano · lore

Re: t0008-ignores failure

On Wed, May 29, 2013 at 07:21:51PM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
> > I realize colon was chosen to mimic git-check-attr, however,
> > check-attr prints relative paths only (I think?).
> >
> > How about using TAB or '|' instead? AFAICT, these are typically
> > not used in paths or glob patterns.
> 
> The primary reason to use ':' in "check-ignore -v" is to mimic the
> output format of "grep -n".
> 
> Emacs users can then run the commands like check-attr/check-ignore
> with "M-x find-grep" (or "M-x compile"), the output format is
> recognized by the editor, and the user can jump around with \C-x` to
> view hits.
> 
> I do not use vim myself, but I would be mildly surprised if there
> isn't a similar feature there.

It does (it is how my "git jump" command feeds marks to vim). Usually we would quote ambiguous pathnames, but I think we do not here to retain compatibility with that microformat. Readers that care about quoting should use "-z" to get unambiguous output. And indeed, it seems that check-ignore behaves reasonably in this case. The tests fail because the test script itself is lazy. It does:

  sed -e 's/      "/      /' -e 's/\\//' -e 's/"$//' expected-verbose | \
        tr ":\t\n" "\0" >expected-verbose0

which generates a bogus expectation; both the delimiter colons and any in the fields are converted, whereas only the former should be. Karsten's fix should work, or we could generate our expected output more carefully.

Long ago we switched to putting a space into our trash directory name to catch problems with such pathnames when we run the test suite. I wonder if we should do the same with ":". Doing this:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index ca6bdef..5d84705 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -600,7 +600,7 @@ fi
 fi
 
 # Test repository
-TRASH_DIRECTORY="trash directory.$(basename "$0" .sh)"
+TRASH_DIRECTORY="trash directory:$(basename "$0" .sh)"
 test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
 case "$TRASH_DIRECTORY" in
 /*) ;; # absolute path is good

reveals the breakage on Linux. And it seems that a lot of other tests
break, too. I haven't looked into them yet, though.

-Peff
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Jeff King· May 30, 2013, 02:55 UTC · re: Jeff King · lore

Re: t0008-ignores failure

On Wed, May 29, 2013 at 10:52:58PM -0400, Jeff King wrote:
Show 16 quoted lines
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ca6bdef..5d84705 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -600,7 +600,7 @@ fi
>  fi
>  
>  # Test repository
> -TRASH_DIRECTORY="trash directory.$(basename "$0" .sh)"
> +TRASH_DIRECTORY="trash directory:$(basename "$0" .sh)"
>  test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
>  case "$TRASH_DIRECTORY" in
>  /*) ;; # absolute path is good
> 
> reveals the breakage on Linux. And it seems that a lot of other tests
> break, too. I haven't looked into them yet, though.

Hrm. Just picking an example at random, t7006 fails because it uses --exec-path="`pwd`". And of course colons are meaningful in any PATH-like context. It would be nice to be able to handle that case cleanly, but I think we would be breaking compatibility.

So while it would be nice to work on paths with colons everywhere, I doubt it is worth the effort to start checking it through the whole test suite.

-Peff
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Johannes Sixt· May 30, 2013, 06:58 UTC · re: Jeff King · lore

Re: Re: t0008-ignores failure

Am 30.05.2013 04:55, schrieb Jeff King:
> So while it would be nice to work on paths with colons everywhere, I
> doubt it is worth the effort to start checking it through the whole test
> suite.

And on top of it, on Windows, you can't have a path component or file name that contains a colon...

-- Hannes
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Johannes Schindelin· May 30, 2013, 15:15 UTC · re: Karsten Blees · lore

Re: t0008-ignores failure (was: Git for Windows 1.8.3)

Hi Karsten,
On Thu, 30 May 2013, Karsten Blees wrote:
Show 21 quoted lines
> Am 25.05.2013 21:16, schrieb Pat Thoyts:
> > On that note -- with this merge as it now stands I get the following
> > test failures:
> > 
> > t0008-ignores.sh                     155, 158, 162, 164
> 
> These tests fail because they use absolute paths, e.g. "C:/.../global-excludes", which is then translated to "C<NUL>/.../global-excludes". Can be fixed like so:
> 
> --- 8< ---
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -5,7 +5,7 @@ test_description=check-ignore
>  . ./test-lib.sh
> 
>  init_vars () {
> -       global_excludes="$(pwd)/global-excludes"
> +       global_excludes="global-excludes"
>  }
> 
>  enable_global_excludes () {
> ---

Since I do not have time for the lengthy, undirected discussion upstream seems to want to start, let's make your change, but only conditional on MINGW?

Ciao, Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Pat Thoyts· May 30, 2013, 15:45 UTC · re: Johannes Schindelin · lore

Re: t0008-ignores failure (was: Git for Windows 1.8.3)

On 30 May 2013 16:15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 32 quoted lines
> Hi Karsten,
>
> On Thu, 30 May 2013, Karsten Blees wrote:
>
>> Am 25.05.2013 21:16, schrieb Pat Thoyts:
>> > On that note -- with this merge as it now stands I get the following
>> > test failures:
>> >
>> > t0008-ignores.sh                     155, 158, 162, 164
>>
>> These tests fail because they use absolute paths, e.g. "C:/.../global-excludes", which is then translated to "C<NUL>/.../global-excludes". Can be fixed like so:
>>
>> --- 8< ---
>> --- a/t/t0008-ignores.sh
>> +++ b/t/t0008-ignores.sh
>> @@ -5,7 +5,7 @@ test_description=check-ignore
>>  . ./test-lib.sh
>>
>>  init_vars () {
>> -       global_excludes="$(pwd)/global-excludes"
>> +       global_excludes="global-excludes"
>>  }
>>
>>  enable_global_excludes () {
>> ---
>
> Since I do not have time for the lengthy, undirected discussion upstream
> seems to want to start, let's make your change, but only conditional on
> MINGW?
>
> Ciao,
> Dscho

I was just testing this -- I've already wrapped the suggested fix within a "test_have_prereq MINGW" for our fork and committed it. This was an issue partly because was alias pwd to "pwd -W" and so always get Windows paths. It means the test here doesn't check absolute paths but I think we can live with that. I tried using $(builtin pwd) to avoid the "-W" but it didn't help and I still got C: style paths.

I also grabbed Karsten's patch "dir.c: fix ignore processing within not-ignored directories" as this appears to deal with a .gitignore regression in 1.8.3. We can carry this until the next merge with upstream.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Johannes Schindelin· May 30, 2013, 17:14 UTC · re: Pat Thoyts · lore

Re: t0008-ignores failure (was: Git for Windows 1.8.3)

Hi Pat,
On Thu, 30 May 2013, Pat Thoyts wrote:
Show 43 quoted lines
> On 30 May 2013 16:15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > On Thu, 30 May 2013, Karsten Blees wrote:
> >
> >> Am 25.05.2013 21:16, schrieb Pat Thoyts:
> >> > On that note -- with this merge as it now stands I get the following
> >> > test failures:
> >> >
> >> > t0008-ignores.sh                     155, 158, 162, 164
> >>
> >> These tests fail because they use absolute paths, e.g.
> >> "C:/.../global-excludes", which is then translated to
> >> "C<NUL>/.../global-excludes". Can be fixed like so:
> >>
> >> --- 8< ---
> >> --- a/t/t0008-ignores.sh
> >> +++ b/t/t0008-ignores.sh
> >> @@ -5,7 +5,7 @@ test_description=check-ignore
> >>  . ./test-lib.sh
> >>
> >>  init_vars () {
> >> -       global_excludes="$(pwd)/global-excludes"
> >> +       global_excludes="global-excludes"
> >>  }
> >>
> >>  enable_global_excludes () {
> >> ---
> >
> > Since I do not have time for the lengthy, undirected discussion upstream
> > seems to want to start, let's make your change, but only conditional on
> > MINGW?
> 
> I was just testing this -- I've already wrapped the suggested fix
> within a "test_have_prereq MINGW" for our fork and committed it. This
> was  an issue partly because was alias pwd to "pwd -W" and so always
> get Windows paths. It means the test here doesn't check absolute paths
> but I think we can live with that. I tried using $(builtin pwd) to
> avoid the "-W" but it didn't help and I still got C: style paths.
> 
> I also grabbed Karsten's patch "dir.c: fix ignore processing within
> not-ignored directories" as this appears to deal with a .gitignore
> regression in 1.8.3. We can carry this until the next merge with
> upstream.

Thanks! Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

← back to recent threads