fix: fix single object cases for sync #740
Merged
Conversation
This file contains 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
tarikozyurtt
requested review from
ilkinulas and
seruman
and removed request for
a team
tarikozyurtt
changed the title
tarikozyurtt
changed the title
seruman
reviewed
Jul 11, 2024
|
Fixed styles and naming. |
seruman
approved these changes
Jul 11, 2024
ilkinulas
approved these changes
Jul 12, 2024
tarikozyurtt
added a commit
to tarikozyurtt/s5cmd
that referenced
this pull request
Jul 12, 2024
The problem is mainly caused by the `compareObjects` function inside `command/sync.go`, where `s5cmd` compares source and destination paths and extracts files that are present only in the source or destination path (while also counting nested folders or rather name with its prefixes) along with common objects. If they both are non-objects, like wildcard expression, prefix, or bucket, getting relative paths of files with `src.URL.Relative()` results in compatible and comparable paths. In this case, no problem is present, at least within the scope of this issue. However, when an object is selected as the source, it is not assigned a relative path using `func (u *url.URL) SetRelative(base *url.URL)`, so the `src.URL.Relative()` function returns its absolute path. Let's say the source file has an absolute path of `folder/foo.txt`. The algorithm compares `folder/foo.txt` with `s3://bucket/remoteFolder/` and looks for the item `s3://bucket/remoteFolder/folder/foo.txt`. If it does not match, except for the edge case where there is a duplicate item inside the searched path, the files never match. While copying files, `s5cmd` does not use relative paths, so `foo.txt` is written to the intended path in the remote. However, this happens during every sync operation, as they do not match. Problem solved by taking path of source object as its name. This made algorithm to simply look for matches in destination, a file named `foo.txt` as intended. This PR adds new test cases to the sync command. Previously, tests failed to capture sync command cases where the source is an object in a prefix, not an object directly in a bucket, or not multiple objects like a wildcard or prefix expression. If an object is in the `s3://bucket/` path, its relative path is the same as its absolute path, so they match during comparison. This prevented copying the file every time. The new test cases cover all scenarios. Resolves: peak#676.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The problem is mainly caused by the
compareObjectsfunction insidecommand/sync.go, wheres5cmdcompares source and destination paths and extracts files that are present only in the source or destination path (while also counting nested folders or rather name with its prefixes) along with common objects. If they both are non-objects, like wildcard expression, prefix, or bucket, getting relative paths of files withsrc.URL.Relative()results in compatible and comparable paths. In this case, no problem is present, at least within the scope of this issue.However, when an object is selected as the source, it is not assigned a relative path using
func (u *url.URL) SetRelative(base *url.URL), so thesrc.URL.Relative()function returns its absolute path.Let's say the source file has an absolute path of
folder/foo.txt. The algorithm comparesfolder/foo.txtwiths3://bucket/remoteFolder/and looks for the items3://bucket/remoteFolder/folder/foo.txt. If it does not match, except for the edge case where there is a duplicate item inside the searched path, the files never match.While copying files,
s5cmddoes not use relative paths, sofoo.txtis written to the intended path in the remote. However, this happens during every sync operation, as they do not match.Problem solved by taking path of source object as its name. This made algorithm to simply look for matches in destination, a file named
foo.txtas intended.This PR adds new test cases to the sync command. Previously, tests failed to capture sync command cases where the source is an object in a prefix, not an object directly in a bucket, or not multiple objects like a wildcard or prefix expression.
If an object is in the
s3://bucket/path, its relative path is the same as its absolute path, so they match during comparison. This prevented copying the file every time. The new test cases cover all scenarios.Resolves: #676.