Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore force = newer/older if mtimes are equal #970

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

tleedjarv
Copy link
Contributor

@tleedjarv tleedjarv commented Oct 3, 2023

force = newer/older is defined (and documented as such) only for changes where mtimes are different between the replicas. If mtimes are equal then root 2 is always forced over root 1.

This produces unexpected results for changes that don't change mtime (basically all metadata changes). Since force = newer/older is undefined for such changes (and, importantly, is very difficult to define), this patch just ignores the preference for non-conflicting changes if mtimes are equal. For conflicting changes, the current default of forcing root 2 is kept unchanged.

There is further discussion and background in the mailing list.

@gdt
Copy link
Collaborator

gdt commented Oct 3, 2023

I object to forcing root 2 if there are conflicting changes. The user has asked for newer to be forced, but there is no newer, and thus no basis to force at all. What they are trying to do, and what they actually want, is hard to guess, so I think we should just not. Yes, I realize this leads to interactive, but I think that's better than doing something unexpected that is basically bug-compatible with before. Preferring root2 over root1 in a tie I find bizarre in the first place and it smells like fallthrough instead of intended.

@tleedjarv
Copy link
Contributor Author

I have no issue with your objection and it's only a minor change to the patch but I can't help but be reminded of https://xkcd.com/1172/ (of course, the PR in its current form does the same, so...).

@gdt
Copy link
Collaborator

gdt commented Oct 3, 2023

The xkcd is amusing. However, I feel that force=newer is a kludge to start with, and doesn't really belong in a sync tool. If you want rsync instead of unison, it's much simpler.

force = newer/older preference works based on the difference of mtime in
each replica. If mtimes are equal then it always propagates from root 2
to root 1. This yields unexpected results when propagating changes that
don't update the mtime (that's basically all metadata updates), with
different roots treated clearly differently.

This patch makes it explicit that newer/older are only defined for when
mtimes differ, ignoring the force = newer/older preference (and other
related preferences, such as prefer and *partial) if mtimes are equal.
@tleedjarv
Copy link
Contributor Author

Pushed the new fix which completely ignores "newer" and "older" for equal mtimes.

@gdt
Copy link
Collaborator

gdt commented Oct 3, 2023

Thanks. Will hold off until 16Z on 5 October for discussion and then merge as soon as I realize it's after that :-)

@gdt gdt merged commit 91f5b47 into bcpierce00:master Oct 9, 2023
37 checks passed
@tleedjarv tleedjarv deleted the force-newer-older branch October 9, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants