Looks good, although at present I don't think it handles this case:

$HOME/**/*
!$HOME/local/bin

i.e. the current logic excludes $HOME/local and so the negate for $HOME/local/bin is missed.

This solution needs special handling of the childMayMatch combined with negate case.

I've worked some logic which appears to work and will add tomorrow. It needs to consider both exclude and include cases.

Yes, you are right, this is not handled correctly. It should be:

$HOME/**/*
!$HOME/local
$HOME/local/*
!$HOME/local/bin

Not very user-friendly. But on par with git (though for git, this kind of case is not very common).

Understood, thanks, although I think highly preferable to handle this as per the two-line formulation. Thoughts?

Also, this PR does not currently handle restore presently (same filter code used in cmd_restore.go).

e.g. this should restore local/bin files starting with 'a' through 'm' but restores nothing

restic restore -v --include '/local/bin' --include '!/local/bin/[a-m]*' latest

For this example, yes. But what about:

$HOME/build
test.*
!test.c

Should we include $HOME/build/something/something/test.c (and then walk $HOME/build)? I mean, just having one unqualified/relative exception would mean we have to walk everything because it can be anywhere.

As for restore, I think you want --exclude '/local/bin/*' --exclude '!/local/bin/[a-m]*'. Except if you have already implemented your additional logic?

Regarding general logic, I think you are right. The additional terms are better than the requirement to walk additional subtrees. Thanks.

In regards to the restore, I'm presuming you mean --include not --exclude. But in any case the modification of the term to /local/bin/* does not help.

I believe the issue is that the childMayMatch should not reset in the same way as the primary match term - it should retain the behaviour of triggering to true if any pattern indicates that a child may match.

So

childMayMatch = childMayMatch && !c || c && !negate

becomes

childMayMatch = childMayMatch || c && !negate

This works for my restore case. Thoughts?

I also suggest another expansion of the filter tests is warranted - a few extra cases and capturing the childMayMatch return value in the tests. See danielsharvey@7cb1815

Thoughts?

You said e.g. this should restore local/bin files starting with 'a' through 'm' but restores nothing, but you have --include "!local/bin/[a-m]*", so it should not restore files starts with 'a' through 'm', right? So, I am a bit confused by the example.

But you are right, there is something different about restore. Just using a negative pattern is enough to make it fail. We should add tests for the this as there is currently none.

You said e.g. this should restore local/bin files starting with 'a' through 'm' but restores nothing, but you have --include "!local/bin/[a-m]*", so it should not restore files starts with 'a' through 'm', right? So, I am a bit confused by the example.

Argh! Sorry - the test restore (that works with the change I suggest) is to exclude 'a' through 'm' and include 'n' through 'z'.

But you are right, there is something different about restore. Just using a negative pattern is enough to make it fail. We should add tests for the this as there is currently none.

Yes. The tests I suggest in my commit danielsharvey@7cb1815 and danielsharvey@6a4938a that capture this and the other checks we've been discussing are:

{[]string{"/**/*", "!/foo", "/foo/*", "!/foo/bar"}, "/foo/other/test.go", true, true},
{[]string{"/**/*", "!/foo", "/foo/*", "!/foo/bar"}, "/foo/bar/test.go", false, false},
{[]string{"/**/*", "!/foo", "/foo/*", "!/foo/bar"}, "/foo/bar/test.go/child", false, false},
{[]string{"/**/*", "!/foo", "/foo/*", "!/foo/bar", "/foo/bar/test*"}, "/foo/bar/test.go/child", true, true},
{[]string{"/foo/bar/*", "!/foo/bar/[a-m]*"}, "/foo", false, true},
{[]string{"/foo/**/test.c"}, "/foo/bar/foo/bar/test.c", true, true},
{[]string{"/foo/*/test.c"}, "/foo/bar/foo/bar/test.c", false, false},

But, I still had not properly considered the childMayMatch case in the negation case. I think the logic needs to be:

matched = matched && !m || m && !negate

if negate {
  // a matching negation will 'cover' child matches
  if m && c {
    childMayMatch = false
  }
} else {
  childMayMatch = childMayMatch || c
}

Hey!

Sorry for the late answer, I was a bit busy lately. So you were right. I have added a commit with tests for childMayMatch before the change and I have updated the commit for the change to use your logic (but I have formatted to make it look like the logic for updating match). I agree with the results of the tests. This seems to work now as expected.

Thanks, looks good to me.

Thanks @danielsharvey for the review! I have added the documentation and the changelog entry. The PR should be ready for a formal review.

Codecov Report

Merging #2311 into master will decrease coverage by 4.25%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2311      +/-   ##
==========================================
- Coverage   51.09%   46.84%   -4.26%     
==========================================
  Files         178      178              
  Lines       14546    14550       +4     
==========================================
- Hits         7433     6816     -617     
- Misses       6042     6720     +678     
+ Partials     1071     1014      -57

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bd5db4...86f53ab. Read the comment docs.

@fd0 Is there any process that we should follow to progress PRs like this one? I have been using this for some time and would love to see it merged. Thanks!

Following up on the comments by @MichaelEischer would be the process, but in the end it boils down to the core developers' time constraints. There are many other changes that are being worked on, patience is needed :)

I have rebased the commit and use the second approach, suggested by @MichaelEischer

Thanks @rawtaz, I appreciate the challenge of volunteer efforts. And apologies, I had missed that @MichaelEischer's comments had not been fully responded to (now done).

To anyone else; Has anyone else tested this in production? Please let us know :)

I'm not "anyone else", but just to add clarity to my use cases (in regular use by me):

1. I've attached one of my standard backups, which runs off restic ... --exclude-file ~/restic-excludes-negative ~/

2. I also have a continuous backup script which uses a file watcher to detect filesystem updates, and then run a differential backup every 15 minutes based upon a dynamically generated exclude file similiar to 1 above.

Attachment: restic-excludes-negative

If there is still hope to get this merged, I can try another rebase...

I did have a look, but there are too many changes to easily rebase. If anyone wants to give it a try, they are welcome to take over this PR.

I understand, thanks for your work!

@MichaelEischer @fd0 Thanks for all the work. Just checking in - any assistance required to finalise this?

Does the new exclamation mark functionality work for inclusions using --files-from? The documentation only mentions using it for exclusions, not inclusions.

No, the negative patterns are only available for exclusions. A negative pattern in files-from would actually be an exclude, so it should just work to specify it as such?

@MichaelEischer Are we able to represent any sort of ordering with this approach?

Meaning, can I specify the following?

• Exclude /foo but
• Include /foo/one, /foo/two

If I remember correctly, the exclusion of the parent directory will override any inclusion of the children directories.

@cowwoc You absolutely can.

My backup selectively backs up using this mechanism:

restic <...> --exclude-file ~/backup/readme/restic-excludes-negative ~/

My excludes file looks like this:

/*
!/Users
/Users/*
!/Users/daniel

/$HOME/*

!$HOME/Desktop
!$HOME/Documents
!$HOME/Downloads

# ...

Note the pattern:

1. Exclude everything
2. "Unexclude" the folder you want to include
3. Exclude everything (all contents - *) in that folder
4. Unexclude the contents of the folder that you want to include
5. Repeat