Re: Incorrect recursive behaviour of --exclude-ignore, --exclude-vcs-ignores

[Christian Schoenebeck]

On Sonntag, 12. September 2021 17:47:23 CEST Kirill Pushkaryov wrote:
> Hello!
> 
> I've previously posted this here in July 2020, but got no response at
> all and the problems are still present, so I decided to remind about this.

First off: I am not involved in tar, so I can't tell why nobody responded.

> I consider the issues to be important, because inadvertent omission of
> files can be very damaging, e.g. to backups.
> 
> The original text follows.
> 
> I've found that --exclude-ignore option and --exclude-vcs-ignores with
> .cvsignore act recursively, which contradicts the manual. A trivial
> patch is attached.
[...]
> 
> Current result:
> 
>     test/
>     test/.ign
>     test/a/
> 
> Expected result: test/a/b must not be excluded.
> 
> The same for --exclude-vcs-ignores:
> 
>     mkdir test test/a
>     touch test/b test/a/b
>     echo b > test/.cvsignore
>     tar -cvf test.tar --exclude-vcs-ignores test
> 
> Current result:
> 
>     test/
>     test/a/
>     test/.cvsignore
> 
> Expected result: test/a/b must not be excluded.

Yes, agreed, looks like a bug.

> I suspect a bug in info_attach_exclist() (exclist.c:116):
> 
>        ent = xmalloc (sizeof (*ent));
>        ent->excluded = ex;
>        ent->flags = file->flags == EXCL_DEFAULT
>                     ? file->flags : vcsfile->flags;
>        ent->prev = tail;
>        ent->next = NULL;
> 
> Here file is an exclusion file added by some options. The currently
> possible values of file->flags are as follows:
> 
> a) EXCL_DEFAULT corresponds to --exclude-vcs-ignores option and marks
> standard VCS exclusion files. Such entries are created by
> exclude_vcs_ignores();
> 
> b) EXCL_RECURSIVE corresponds to --exclude-ignore-recursive;
> 
> c) EXCL_NON_RECURSIVE corresponds to --exclude-ignore.
> 
> Currently neither EXCL_RECURSIVE nor EXCL_NON_RECURSIVE are propagated
> from file->flags to ent->flags. Moreover, EXCL_NON_RECURSIVE flag in
> vcsfile->flags of .cvsignore entry is dropped too, because for
> .cvsignore file->flags == EXCL_DEFAULT.
> 
> It looks like the condition in the code above is just erroneously
> inverted. I fixed this and the options started to work as expected.

Well, I just had a glimpse on the relevant code section, what I could imagine 
is that as maintainer you might be worried that it might just move the problem 
to another option combination.

Probably you could increase the chance for somebody to process your patch by 
accompanying it with a test case. In the tests directory there are already a 
bunch of similar exclude-* tests, so it should not be hard to add one for your 
case.

Best regards,
Christian Schoenebeck