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

Remove FAT-inspired DST workaround #1046

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gdt
Copy link
Collaborator

@gdt gdt commented Jun 10, 2024

In addition, clean up related dead code.

gdt added 2 commits June 10, 2024 09:00
This was introduced in adf68c9 in
2009.  The theory is that on Windows with FAT, filesystem times are in
an undetermined timezone (a design defect) and as DST happens, the
times of files already there change.  While this is true, ignoring an
hour change also ignores valid changes on filesystems that store times
correctly (in UTC, as the major example).

Ignoring an hour is not enough; if the host's timezone changes by 3
hours (or anything not 1), then times in FAT move by that amount.
It's also too much, in that the workaround ignores time changes on
filesystems with a stable time representation.

Therefore, remove the workaround, and anyone who is using FAT,
changing timezones, and syncing times (expected to be very few) can
propose how to deal with this in a way that doesn't adversely affect
others.

Fixes bcpierce00#72.
@gdt gdt marked this pull request as draft June 10, 2024 13:17
@gdt
Copy link
Collaborator Author

gdt commented Jun 10, 2024

Someone pointed out to me that the time check is also used for update detection, and that this will mean that people syncing to/from FAT will have a full scan/hash every time the timezone changes (DST or a zone change). Of course, a subset of these cases (changes that aren't 1h) result in rescans now.

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.

1 participant