-
Notifications
You must be signed in to change notification settings - Fork 249
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
storage/url: fixes the bug where some part of destination path is removed by cp #456
Merged
igungor
merged 15 commits into
peak:master
from
kucukaslan:fix-360-preserve-destination-path
Jul 22, 2022
Merged
storage/url: fixes the bug where some part of destination path is removed by cp #456
igungor
merged 15 commits into
peak:master
from
kucukaslan:fix-360-preserve-destination-path
Jul 22, 2022
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
…oved by `cp` The cause of problem is a little bit weird. (url).SetRelative method was given a source (abs:) path as it was provided in command line arguments including the wildcard characters(* and ?). Hence it determined relative path incorrectly, where "expected" relative path became "../expected" which in turn caused the some part of destination to be lost (since path/filepath.Join converts e.g. /a/b/../c to /a/c) We also need to identify whether the * and ? characters present in base strings are part of key or they are used for wildcard operation. So we need to know if the "raw" flag is given. To. solve this I changed (URL).SetRelative method to take argument of type URL rather than string. Fixes #360
kucukaslan
requested review from
aykutfarsak and
seruman
and removed request for
a team
July 1, 2022 13:50
aykutfarsak
reviewed
Jul 4, 2022
seruman
requested changes
Jul 5, 2022
Co-authored-by: Selman Kayrancioglu <seruman@users.noreply.github.com>
use raw string in path strings of windows test. add bucket names for the s3 key examples.
seruman
reviewed
Jul 6, 2022
seruman
approved these changes
Jul 6, 2022
aykutfarsak
reviewed
Jul 7, 2022
Co-authored-by: Aykut Farsak <aykutfarsak@gmail.com>
aykutfarsak
approved these changes
Jul 7, 2022
igungor
reviewed
Jul 18, 2022
The testcases will be generated depending on the OS using the filepath.Seperator, instead of preparing two different testcases one of whom is to be selected on runtime environment.
igungor
approved these changes
Jul 22, 2022
igungor
changed the title
storage/url: fixes the bug where some part of destination path is rem…
storage/url: fixes the bug where some part of destination path is removed by cp
Jul 22, 2022
igungor
pushed a commit
that referenced
this pull request
Aug 22, 2022
Assumes that user has two copies of s5cmd one is default commandline app, second one is in the current directory. A bucket name MUST be provided with -b flag. User can optionally: - add a key prefix with -k flag. - specify hyperfine warmup counts with -w flag - specify hyperfine runs counts with -r flag. Example calls `./benchmark.sh -b mcanktmpbuck` `./benchmark.sh -w 1 -r 4 -b mcanktmpbuck -k example_key_prefix ` * allow user to specify what version/PR/commit of s5cmd will be used in the tests. user can specify either version(tag), commit hash, or the PR number of the s5cmd that are to be used in the tests. Two versions of the s5cmd will be used namely old and new. They can be specified by -o and -n flags, respectively. Though version specified by o does not have to be older than that of specified by -n flag. Default values of versions are v1.4.0 for old (-o), and v2.0.0 for new (-n). Example execution that compares performance of #456 with the v1.4.0: ` ./benchmark.sh -b mys5cmbuck -n 456 -o v1.4.0 -k 1829` Also note that user must provide a proper bucket which she has write/read/delete access. Co-Authored-By: boraberke <67373739+boraberke@users.noreply.github.com> Co-authored-by: MUHAMMED CAN KÜÇÜKASLAN <muhammedcankucukaslan@gmail.com>
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.
…oved by
cp
The cause of problem is a little bit weird.
(url).SetRelative method was given the source (abs.) path as it was provided in command line arguments including the wildcard characters(* and ?). Hence it determined relative path incorrectly, where "expected" relative path became "../expected" which in turn caused the some part of destination to be lost (since path/filepath.Join converts e.g. /a/b/../c to /a/c)
We also need to identify whether the * and ? characters present in base strings are part of key or they are used for wildcard operation. So we need to know if the "raw" flag is given. To. solve this I changed (URL).SetRelative method to take argument of type URL rather than string.
Fixes #360
ps. This PR is aimed for the v2.1.0.