From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got ignore some .{cvs,git}ignore files
To: Sebastien Marie <semarie@online.fr>
Cc: gameoftrees@openbsd.org
Date: Tue, 23 Jun 2020 10:19:10 +0200

On Tue, Jun 23, 2020 at 08:59:23AM +0200, Sebastien Marie wrote:
> Hi,
> 
> I have a buggy behaviour (or an unexpected feature) with .cvsignore files.
> 
> I slightly modified the regress for test_status_cvsignore to expose it.
> 
> In the (modified) regress test, the files in worktree are (for what I want to
> expose):
> 
> /.cvsignore
> /foo
> /epsilon/foo
> 
> The .cvsignore contains "foo" as ignored. "/foo" and "/epsilon/foo" are
> unversionned.
> 
> if `got status' is fine, running `got status epsilon' is wrong ("/epsilon/foo"
> is showed as unversionned instead being ignored).

If you ignore **/foo instead of 'foo' in .cvsignore, then epsilon/foo
will be ignored when 'got status' is run from the root of the work tree:

$ got st
?  .cvsignore
?  epsilon/.cvsignore
?  epsilon/boo
?  epsilon/foo
?  foop
$ echo **/foo > .cvsignore
$ got st
?  .cvsignore
?  epsilon/.cvsignore
?  epsilon/boo
?  foo
?  foop
$

> For what I see from the behaviour, in the first command, "/" directory is
> walked, so "/.cvsignore" is read (and "foo" pattern considered for ignorelist),
> whereas in the second command, the "/" directory is not walked so "/.cvsignore"
> isn't considered, resulting "/epsilon/foo" to be reported as unversionned.

I think you are missing one important detail here: Patterns are supposed
to be matched relative to the location of the .cvsignore file. Consider:

$ echo foo > .cvsignore
$ got st
?  .cvsignore
?  epsilon/boo
?  epsilon/foo
?  foop
$ echo foo > epsilon/.cvsignore
$ got st
?  .cvsignore
?  epsilon/.cvsignore
?  epsilon/bar
?  epsilon/boo
?  epsilon/moo
?  foop
$ got st epsilon
?  epsilon/.cvsignore
?  epsilon/bar
?  epsilon/boo
?  epsilon/moo
$

> test_status_cvsignore --- /tmp/got-test-status_cvsignore-t9lqQcrr/stdout.expected       Tue Jun 23 08:42:30 2020
> +++ /tmp/got-test-status_cvsignore-t9lqQcrr/stdout      Tue Jun 23 08:42:30 2020
> @@ -1,4 +1,5 @@
>  ?  .cvsignore
>  ?  epsilon/.cvsignore
>  ?  epsilon/boo
> +?  epsilon/foo
>  ?  foop
> test failed; leaving test data in /tmp/got-test-status_cvsignore-t9lqQcrr
> test_status_gitignore ok
> 
> 
> Please note that it could be also exposed with got repository itself:
> 
> $ cd got
> $ cat .gitignore
> **/obj
> $ got st
> M  regress/cmdline/status.sh
> $ got st libexec/got-fetch-pack
> ?  libexec/got-fetch-pack/obj/error.d
> ?  libexec/got-fetch-pack/obj/error.o
> ?  libexec/got-fetch-pack/obj/got-fetch-pack
> ?  libexec/got-fetch-pack/obj/got-fetch-pack.d
> ?  libexec/got-fetch-pack/obj/got-fetch-pack.o
> ?  libexec/got-fetch-pack/obj/inflate.d
> ?  libexec/got-fetch-pack/obj/inflate.o
> ?  libexec/got-fetch-pack/obj/object_parse.d
> ?  libexec/got-fetch-pack/obj/object_parse.o
> ?  libexec/got-fetch-pack/obj/path.d
> ?  libexec/got-fetch-pack/obj/path.o
> ?  libexec/got-fetch-pack/obj/privsep.d
> ?  libexec/got-fetch-pack/obj/privsep.o
> ?  libexec/got-fetch-pack/obj/sha1.d
> ?  libexec/got-fetch-pack/obj/sha1.o
> 
> the "obj" directory is shown whereas present in .gitignore

Yes, this is a bug. The evaluation of ignore lists should not depend on
the status path argument. However, this is not the problem which your test
case patch is trying to reproduce, is it? Our ignore test cases do not
cover the case of running status with a path argument! We should fix this.

> diff c34ec41754978e41240e8aedd4e5d3a9e6362635 /home/semarie/repos/openbsd/got
> blob - 5abbce9475bb8bd2f1cbb3e33113883173026f03
> file + regress/cmdline/status.sh
> --- regress/cmdline/status.sh
> +++ regress/cmdline/status.sh
> @@ -496,6 +496,7 @@ function test_status_cvsignore {
>  
>  	echo "unversioned file" > $testroot/wt/foo
>  	echo "unversioned file" > $testroot/wt/foop
> +	echo "unversioned file" > $testroot/wt/epsilon/foo
>  	echo "unversioned file" > $testroot/wt/epsilon/bar
>  	echo "unversioned file" > $testroot/wt/epsilon/boo
>  	echo "unversioned file" > $testroot/wt/epsilon/moo

As explained above, your suggested test case will pass if we do this instead:

diff 2c2d5c5f4cce08cb69d03aea1a1c747fd27f9e39 /home/stsp/src/got
blob - 5abbce9475bb8bd2f1cbb3e33113883173026f03
file + regress/cmdline/status.sh
--- regress/cmdline/status.sh
+++ regress/cmdline/status.sh
@@ -496,10 +496,11 @@ function test_status_cvsignore {
 
 	echo "unversioned file" > $testroot/wt/foo
 	echo "unversioned file" > $testroot/wt/foop
+	echo "unversioned file" > $testroot/wt/epsilon/foo
 	echo "unversioned file" > $testroot/wt/epsilon/bar
 	echo "unversioned file" > $testroot/wt/epsilon/boo
 	echo "unversioned file" > $testroot/wt/epsilon/moo
-	echo "foo" > $testroot/wt/.cvsignore
+	echo "**/foo" > $testroot/wt/.cvsignore
 	echo "bar" > $testroot/wt/epsilon/.cvsignore
 	echo "moo" >> $testroot/wt/epsilon/.cvsignore