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

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

[Karsten Blees]

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.

Re: t0008-ignores failure

[Junio C Hamano]

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.

-- 
-- 
*** 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.

Re: t0008-ignores failure

[Jeff King]

On Wed, May 29, 2013 at 07:21:51PM -0700, Junio C Hamano wrote:

> 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.

Re: t0008-ignores failure

[Jeff King]

On Wed, May 29, 2013 at 10:52:58PM -0400, Jeff King wrote:

> 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.

Re: Re: t0008-ignores failure

[Johannes Sixt]

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.

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

[Johannes Schindelin]

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

-- 
-- 
*** 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.

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

[Pat Thoyts]

On 30 May 2013 16:15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 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.

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

[Johannes Schindelin]

Hi Pat,

On Thu, 30 May 2013, Pat Thoyts wrote:

> 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.