bisync: normalize session name to non-canonical (and docs edits) - fixes #7423#1
Draft
nielash wants to merge 130 commits into
Draft
bisync: normalize session name to non-canonical (and docs edits) - fixes #7423#1nielash wants to merge 130 commits into
nielash wants to merge 130 commits into
Conversation
Apparently gcs doesn't return an S3 compatible result when using versions. In particular it doesn't return a NextKeyMarker - this means rclone loops and fetches the same page over and over again. This patch detects the problem and stops the infinite retries but it doesn't fix the underlying problem. See: https://forum.rclone.org/t/list-s3-versions-files-looping-bug/42974 See: https://issuetracker.google.com/u/0/issues/312292516
Without this, requests like PROPFIND, issued from a browser, fail.
…ails Before this change, if a multithread upload failed (let's say the source became unavailable) rclone would finalise the file first before aborting the transfer. This caused the partial file to be written which would overwrite any existing files. This was fixed by making sure we Abort the transfer before Close-ing it. This updates the docs to encourage calling of Abort before Close and updates writerAtChunkWriter to make sure that works properly. This also reworks the tests to detect this and to make sure we upload and download to each multi-thread capable backend (we were only downloading before which isn't a full test). Fixes rclone#7071
…-cutoff Before this change the b2 servers would complain as this was only a single part transfer. This was noticed by the new integration tests for server side chunked copy.
Before this change, streaming files an exact multiple of the chunk size would cause rclone to attempt to stream a 0 sized chunk which was rejected by the b2 servers. This bug was noticed by the new integration tests for chunked streaming.
This puts in a workaround for the tests also
This is a workaround to make the new multipart upload integration tests pass.
The following command will block for 60s(default) when the network is slow or unavailable: ``` rclone --contimeout 10s --low-level-retries 0 lsd dropbox: ``` This change will make it timeout after the expected 10s. Signed-off-by: rkonfj <rkonfj@gmail.com>
…clone#7455 Before this change serve s3 would return NoSuchKey errors when a non existent prefix was listed. This change fixes it to return an empty list like AWS does. This was discovered by the full integration tests.
Before this change overwriting an existing file with a 0 length file didn't update the file size. This change corrects the issue and makes sure the file is truncated properly. This was discovered by the full integration tests.
…2/ fork Before this change smb drives sometimes showed a fraction of the correct size using `rclone about`. This fixes the problem by switching the upstream library from github.com/hirochachacha/go-smb2 to github.com/cloudsoda/go-smb2 which has a fix for the problem. The new library passes the integration tests. Fixes rclone#6733
Before this change PartialUploads was not set. This is clearly wrong since incoming files are visible on the smb server. Setting PartialUploads fixes the multithread upload modtime problem as it uses the PartialUploads flag as an indication that it needs to set the modtime explicitly. This problem was detected by the new TestMultithreadCopy integration tests Fixes rclone#7411
Before this change ListR was unconditionally enabled on onedrive. This caused performance problems for some uses, so now the --onedrive-delta flag has to be supplied. Fixes rclone#7362
This was caused by trying to write to a non existent file, and changing the order of the cleanup fixed it. https://forum.rclone.org/t/rclone-v1-65-0-release/43100/18
Before this change the IP address of the server was used in the SMB connect request (see CloudSoda/go-smb2#18). The updated library now can pass the hostname instead. The update requires a small change in the dial method call. Fixes rclone#6672
Before this change, lsf's time format was hard-coded to "2006-01-02 15:04:05", regardless of the Fs's precision. After this change, a new optional --time-format flag is added to allow customizing the format (the default is unchanged). Examples: rclone lsf remote:path --format pt --time-format 'Jan 2, 2006 at 3:04pm (MST)' rclone lsf remote:path --format pt --time-format '2006-01-02 15:04:05.000000000' rclone lsf remote:path --format pt --time-format '2006-01-02T15:04:05.999999999Z07:00' rclone lsf remote:path --format pt --time-format RFC3339 rclone lsf remote:path --format pt --time-format DateOnly rclone lsf remote:path --format pt --time-format max --time-format max will automatically truncate '2006-01-02 15:04:05.000000000' to the maximum precision supported by the remote.
…--checkfile Before this change, --no-unicode-normalization and --ignore-case-sync were respected for rclone check but not for rclone check --checkfile, causing them to give different results. This change adds support for --checkfile so that the behavior is consistent.
normalizes unicode and ignores .DS_Store files to make testing possible on macOS
…p-dir -- fixes rclone#5690 fixes rclone#5685 Before this change, bisync handled copies and deletes in separate operations. After this change, they are combined in one sync operation, which is faster and also allows bisync to support --track-renames and --backup-dir. Bisync uses a --files-from filter containing only the paths bisync has determined need to be synced. Just like in sync (but in both directions), if a path is present on the dst but not the src, it's interpreted as a delete rather than a copy.
Logger instruments the Sync routine with a status report for each file pair, making it possible to output a list of the synced files, along with their attributes and sigil categorization (match/differ/missing/etc.) It is very customizable by passing in a custom LoggerFn, options, and io.Writers to be written to. Possible uses include: - allow sync to write path lists to a file, in the same format as rclone check - allow sync to output a --dest-after file using the same format flags as lsf - receive results as JSON when calling sync from an internal function - predict the post-sync state of the destination For usage examples, see bisync.WriteResults() or sync.SyncLoggerFn()
Allows rclone sync to accept the same output file flags as rclone check,
for the purpose of writing results to a file.
A new --dest-after option is also supported, which writes a list file using
the same ListFormat flags as lsf (including customizable options for hash,
modtime, etc.) Conceptually it is similar to rsync's --itemize-changes, but
not identical -- it should output an accurate list of what will be on the
destination after the sync.
Note that it has a few limitations, and certain scenarios
are not currently supported:
--max-duration / CutoffModeHard
--compare-dest / --copy-dest (because equal() is called multiple times for the
same file)
server-side moves of an entire dir at once (because we never get the individual
file objects in the dir)
High-level retries, because there would be dupes
Possibly some error scenarios that didn't come up on the tests
Note also that each file is logged during the sync, as opposed to after, so it
is most useful as a predictor of what SHOULD happen to each file
(which may or may not match what actually DID.)
Only rclone sync is currently supported -- support for copy and move may be
added in the future.
rclone#5676 Before this change, if there were changes to sync, bisync listed each path twice: once before the sync and once after. The second listing caused quite a lot of problems, in addition to making each run much slower and more expensive. A serious side-effect was that file changes could slip through undetected, if they happened to occur while a sync was running (between the first and second listing snapshots.) After this change, the second listing is eliminated by getting the underlying sync operation to report back a list of what it changed. Not only is this more efficient, but also much more robust to concurrent modifications. It should no longer be necessary to avoid make changes while it's running -- bisync will simply learn about those changes next time and handle them on the next run. Additionally, this also makes --check-sync usable again. For further discussion, see: https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=5.%20Final%20listings%20should%20be%20created%20from%20initial%20snapshot%20%2B%20deltas%2C%20not%20full%20re%2Dscans%2C%20to%20avoid%20errors%20if%20files%20changed%20during%20sync
Before this change, if --create-empty-src-dirs was specified, bisync would include directories in the list of deltas to evaluate by their modtime, relative to the prior sync. This was unnecessary, as rclone does not yet support setting modtime for directories. After this change, we skip directories when comparing modtimes. (In other words, we care only if a directory is created or deleted, not whether it is newer or older.)
Before this change, bisync had no mechanism for "retrying" a file again next time, in the event of an unexpected and possibly temporary error. After this change, bisync is now essentially able to mark a file as needing to be rechecked next time. Bisync does this by keeping one prior listing on hand at all times. In a low-confidence situation, bisync can revert a given file row back to its state at the end of the last known successful sync, ensuring that any subsequent changes will be re-noticed on the next run. This can potentially be helpful for a dynamically changing file system, where files may be changing quickly while bisync is working with them.
This introduces a few basic color codings to make the terminal output more readable (and more fun). Rclone's standard --color flag is supported. (AUTO|NEVER|ALWAYS) Only a few lines have colors right now -- more will probably be added in future versions.
Before this change, bisync needed to build a full listing for Path1, then a full listing for Path2, then compare them -- and each of those tasks needed to finish before the next one could start. In addition to being slow and inefficient, it also caused real problems if a file changed between the time bisync checked it on Path1 and the time it checked the corresponding file on Path2. This change solves these problems by listing both paths concurrently, using the same March infrastructure that check and sync use to traverse two directories in lock-step, optimized by Go's robust concurrency support. Listings should now be much faster, and any given path is now checked nearly-instantaneously on both sides, minimizing room for error. Further discussion: https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=4.%20Listings%20should%20alternate%20between%20paths%20to%20minimize%20errors
Before this change, changing the case of a file on a case insensitive remote would fatally panic when `--dry-run` was set, due to `moveOrCopyFile` attempting to access the non-existent `tmpObj` it (would normally have) created. After this change, the panic is avoided by skipping this step during a `--dry-run` (with the usual "skipped as --dry-run is set" log message.)
…4854 Before this change, a sync to a case insensitive dest (such as macOS / Windows) would not result in a matching filename if the source and dest had casing differences but were otherwise equal. For example, syncing `hello.txt` to `HELLO.txt` would result in the dest filename remaining `HELLO.txt`. Furthermore, `--local-case-sensitive` did not solve this, as it actually caused `HELLO.txt` to get deleted! After this change, `HELLO.txt` is renamed to `hello.txt` to match the source, only if the `--fix-case` flag is specified. (The old behavior remains the default.)
…ixes rclone#7270 Before this change, Bisync sometimes normalized NFD to NFC and sometimes did not, causing errors in some scenarios (particularly for users of macOS). It was similarly inconsistent in its handling of case-insensitivity. There were three main places where Bisync should have normalized, but didn't: 1. When building the list of files that need to be transferred during --resync 2. When building the list of deltas during a non-resync 3. When comparing Path1 to Path2 during --check-sync After this change, 1 and 3 are resolved, and bisync supports --no-unicode-normalization and --ignore-case-sync in the same way as sync. 2 will be addressed in a future update.
Before this change, --resync was handled in three steps, and needed to do a lot of unnecessary work to implement its own --ignore-existing logic, which also caused problems with unicode normalization, in addition to being pretty slow. After this change, it is refactored to produce the same result much more efficiently, by reducing the three steps to two and letting ci.IgnoreExisting do the work instead of reinventing the wheel. The behavior and sync order remain unchanged for now -- just faster (but see the ongoing lively discussions about potential future changes in rclone#5681!)
Refactored the case / unicode normalization logic to be much more efficient, and fix the last outstanding issue from rclone#7270. Before this change, we were doing lots of for loops and re-normalizing strings we had already normalized earlier. Now, we leave the normalizing entirely to March and avoid re-transforming later, which seems to make a large difference in terms of performance.
Before this change, bisync had no ability to retry in the event of sync errors. After this change, bisync will retry if --resilient is passed, but only in one direction at a time. We can safely retry in one direction because the source is still intact, even if the dest was left in a messy state. If the first direction still fails after our final retry, we abort and do NOT continue in the other direction, to prevent the messy dest from polluting the source. If the first direction succeeds, we do then allow retries in the other direction. The number of retries is controllable by --retries (default 3) bisync: high-level retries if --resilient Before this change, bisync had no ability to retry in the event of sync errors. After this change, bisync will retry if --resilient is passed, but only in one direction at a time. We can safely retry in one direction because the source is still intact, even if the dest was left in a messy state. If the first direction still fails after our final retry, we abort and do NOT continue in the other direction, to prevent the messy dest from polluting the source. If the first direction succeeds, we do then allow retries in the other direction. The number of retries is controllable by --retries (default 3)
rclone#5696 Before this change, bisync intentionally ignored Google Docs (albeit in a buggy way that caused problems during --resync.) After this change, Google Docs (including Google Sheets, Slides, etc.) are now supported in bisync, subject to the same options, defaults, and limitations as in `rclone sync`. When bisyncing drive with non-drive backends, the drive -> non-drive direction is controlled by `--drive-export-formats` (default `"docx,xlsx,pptx,svg"`) and the non-drive -> drive direction is controlled by `--drive-import-formats` (default none.) For example, with the default export/import formats, a Google Sheet on the drive side will be synced to an `.xlsx` file on the non-drive side. In the reverse direction, `.xlsx` files with filenames that match an existing Google Sheet will be synced to that Google Sheet, while `.xlsx` files that do NOT match an existing Google Sheet will be copied to drive as normal `.xlsx` files (without conversion to Sheets, although the Google Drive web browser UI may still give you the option to open it as one.) If `--drive-import-formats` is set (it's not, by default), then all of the specified formats will be converted to Google Docs, if there is no existing Google Doc with a matching name. Caution: such conversion can be quite lossy, and in most cases it's probably not what you want! To bisync Google Docs as URL shortcut links (in a manner similar to "Drive for Desktop"), use: `--drive-export-formats url` (or alternatives.) Note that these link files cannot be edited on the non-drive side -- you will get errors if you try to sync an edited link file back to drive. They CAN be deleted (it will result in deleting the corresponding Google Doc.) If you create a `.url` file on the non-drive side that does not match an existing Google Doc, bisyncing it will just result in copying the literal `.url` file over to drive (no Google Doc will be created.) So, as a general rule of thumb, think of them as read-only placeholders on the non-drive side, and make all your changes on the drive side. Likewise, even with other export-formats, it is best to only move/rename Google Docs on the drive side. This is because otherwise, bisync will interpret this as a file deleted and another created, and accordingly, it will delete the Google Doc and create a new file at the new path. (Whether or not that new file is a Google Doc depends on `--drive-import-formats`.) Lastly, take note that all Google Docs on the drive side have a size of `-1` and no checksum. Therefore, they cannot be reliably synced with the `--checksum` or `--size-only` flags. (To be exact: they will still get created/deleted, and bisync's delta engine will notice changes and queue them for syncing, but the underlying sync function will consider them identical and skip them.) To work around this, use the default (modtime and size) instead of `--checksum` or `--size-only`. To ignore Google Docs entirely, use `--drive-skip-gdocs`. Nearly all of the Google Docs logic is outsourced to the Drive backend, so future changes should also be supported by bisync.
Before this change, bisync supported `--backup-dir` only when `Path1` and `Path2` were different paths on the same remote. With this change, bisync introduces new `--backup-dir1` and `--backup-dir2` flags to support separate backup-dirs for `Path1` and `Path2`. `--backup-dir1` and `--backup-dir2` can use different remotes from each other, but `--backup-dir1` must use the same remote as `Path1`, and `--backup-dir2` must use the same remote as `Path2`. Each backup directory must not overlap its respective bisync Path without being excluded by a filter rule. The standard `--backup-dir` will also work, if both paths use the same remote (but note that deleted files from both paths would be mixed together in the same dir). If either `--backup-dir1` and `--backup-dir2` are set, they will override `--backup-dir`.
Similar to rclone@acf1e2d, go1.21.4 appears to have broken sync.MoveDir on Windows because filepath.VolumeName() returns `\\?` instead of `\\?\C:` in cleanRootPath. It looks like the Go team is aware of the issue and planning a fix, so this may only be needed temporarily.
Bisync checks file equality before renaming sync conflicts by comparing checksums. Before this change, backends without checksum support (notably Crypt) would fall back to --size-only for these checks, which is not a very safe method (differing files can sometimes have the same size, especially if they're small.) After this change, Crypt remotes fallback to using Cryptcheck so that checksums can be compared. As a last resort when neither Check nor Cryptcheck are available, files are compared using --download so that we can be certain the files are identical regardless of checksum support.
Before this change, a file would sometimes be silently deleted instead of
renamed on macOS, due to its unique handling of unicode normalization. Rclone
already had a SameObject check in place for case insensitivity before deleting
the source (for example if "hello.txt" was renamed to "HELLO.txt"), but had no
such check for unicode normalization. After this change, the delete is skipped
on macOS if the src and dst filenames normalize to the same NFC string.
Example of the previous behavior:
~ % rclone touch /Users/nielash/rename_test/ö
~ % rclone lsl /Users/nielash/rename_test/ö
0 2023-11-21 17:28:06.170486000 ö
~ % rclone moveto /Users/nielash/rename_test/ö /Users/nielash/rename_test/ö -vv
2023/11/21 17:28:51 DEBUG : rclone: Version "v1.64.0" starting with parameters ["rclone" "moveto" "/Users/nielash/rename_test/ö" "/Users/nielash/rename_test/ö" "-vv"]
2023/11/21 17:28:51 DEBUG : Creating backend with remote "/Users/nielash/rename_test/ö"
2023/11/21 17:28:51 DEBUG : Using config file from "/Users/nielash/.config/rclone/rclone.conf"
2023/11/21 17:28:51 DEBUG : fs cache: adding new entry for parent of "/Users/nielash/rename_test/ö", "/Users/nielash/rename_test"
2023/11/21 17:28:51 DEBUG : Creating backend with remote "/Users/nielash/rename_test/"
2023/11/21 17:28:51 DEBUG : fs cache: renaming cache item "/Users/nielash/rename_test/" to be canonical "/Users/nielash/rename_test"
2023/11/21 17:28:51 DEBUG : ö: Size and modification time the same (differ by 0s, within tolerance 1ns)
2023/11/21 17:28:51 DEBUG : ö: Unchanged skipping
2023/11/21 17:28:51 INFO : ö: Deleted
2023/11/21 17:28:51 INFO :
Transferred: 0 B / 0 B, -, 0 B/s, ETA -
Checks: 1 / 1, 100%
Deleted: 1 (files), 0 (dirs)
Elapsed time: 0.0s
2023/11/21 17:28:51 DEBUG : 5 go routines active
~ % rclone lsl /Users/nielash/rename_test/
~ %
…sts - see rclone#5679 Before this change, integration tests often could not be run on backends with differing features from the local system that goldenized them. In particular, differences in modtime precision, checksum support, and encoding would cause false positives. After this change, the tests more accurately account for the features of the backend being tested, which allows us to see true positives more clearly, and more meaningfully assess whether a backend is supported.
as these changes did not make it in time for 1.65
Before this change, bisync used the "canonical" Fs name in the filename for its
listing files, including any {hexstring} suffix. An unintended consequence of
this was that if a user added a backend-specific flag from the command line
(thus "overriding" the config), bisync would fail to find the listing files it
created during the prior run without this flag, due to the path now having a
{hexstring} suffix that wasn't there before (or vice versa, if the flag was
present when the session was established, and later removed.) This would
sometimes cause bisync to fail with a critical error (if no listing existed
with the alternate name), or worse -- it would sometimes cause bisync to use an
old, incorrect listing (if old listings with the alternate name DID still
exist, from before the user changed their flags.)
After this change, the issue is fixed by always normalizing the SessionName to
the non-canonical version (no {hexstring} suffix), regardless of the flags. To
avoid a breaking change, we first check if a suffixed listing exists. If so, we
rename it (and overwrite the non-suffixed version, if any.) If not, we carry on
with the non-suffixed version. (We should only find a suffixed version if
created prior to this commit.)
The result for the user is that the same pair of paths will always use the same
.lst filenames, with or without backend-specific flags.
nielash
force-pushed
the
bisync-canonical
branch
from
0843f7b to
7b78acf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of this change?
Before this change, bisync used the "canonical" Fs name in the filename for its listing files, including any {hexstring} suffix. An unintended consequence of this was that if a user added a backend-specific flag from the command line (thus "overriding" the config), bisync would fail to find the listing files it created during the prior run without this flag, due to the path now having a {hexstring} suffix that wasn't there before (or vice versa, if the flag was present when the session was established, and later removed.) This would sometimes cause bisync to fail with a critical error (if no listing existed with the alternate name), or worse -- it would sometimes cause bisync to use an old, incorrect listing (if old listings with the alternate name DID still exist, from before the user changed their flags.)
After this change, the issue is fixed by always normalizing the SessionName to the non-canonical version (no {hexstring} suffix), regardless of the flags. To avoid a breaking change, we first check if a suffixed listing exists. If so, we rename it (and overwrite the non-suffixed version, if any.) If not, we carry on with the non-suffixed version. (We should only find a suffixed version if created prior to this commit.)
The result for the user is that the same pair of paths will always use the same .lst filenames, with or without backend-specific flags.
Was the change discussed in an issue or in the forum before?
Checklist