rsync: filter() method + delete=True leads to unexpected results

[ctheune]

I'm using an RSync subclass that implements filter() to ignore some parts of a large tree, specifically things where the remote side has a local version like a virtualenv that should not be touched at all, however, it currently always deletes the remote version ...

I'd be happy to improve the code but need to know whether in general filter()=False + delete=True would expect a filtered item to be removed or left alone on the remote side ... my personal favorite would be to have it ignore the deletions on the remote side and eventually adding a delete_filtered option. This would be in sync with the way regular rsync works.

[RonnyPfannschmidt]

at first glance this looks like a missed case, i think its fine to go with your intent, alto it would be nice if @hpk42 or @arigo where to remember the original intent (which likely came from running pypy testruns via ssh on multiple systems)

i remember that classically rsync has practically been used to copy new code (so the delete by default may have been correct for just that use case)

depending on the structure it may be sensible to give the rsync with correct behaviour a new class name and deprecating the current one (as else it would have to default to the current destructive behaviour

this may very well data back to the time where execnet, xdist and pytest where in the same package

[ctheune]

Yeah, that sounds about right. rsync --delete will do what you want if you don't use exclusion lists, but once you start using exclusion lists it becomes a bit of a gamble whether the default mechanism in rsync in that situation is what you meant.

For us it took about 10 years to discover this situation and now we're stuck with two bad choices. Even noticing that this "filter" + "delete" creates a number of choices that one doesn't directly consider means that people might already be running (similarly to us) with a "broken" strategy that just never materialized to be wrong because of the specific use cases. ;)

I'm happy to consider adding a new class once we're clear about how we want new and old behaviours to interact. The most important change will be on the remote side anyways.