Quote rsync paths to avoid problems with spaces in dir names #286
Conversation
| @@ -245,7 +245,7 @@ def write(content, dest) | |||
| def rsync(source_path, target_path, extra_opts = '--delete') | |||
| cmd = %Q{rsync -rl #{rsync_debug} #{rsync_permissions} --rsh="ssh #{ssh_args}" #{extra_opts}} | |||
| cmd << rsync_excludes.map { |ignore| " --exclude '#{ignore}'" }.join | |||
| cmd << %Q{ #{adjust_rsync_path_on_client(source_path)} :#{adjust_rsync_path_on_node(target_path)}} | |||
| cmd << %Q{ '#{adjust_rsync_path_on_client(source_path)}' ':#{adjust_rsync_path_on_node(target_path)}'} | |||
There was a problem hiding this comment.
So this probably works, I wonder if we should maybe build a command array to pass to system and let Kernel.system do the quoting (where we can anyway, --rsh could be tricky).
Is there a reason we can't do that?
There was a problem hiding this comment.
That would hopefully also clean up the method. It's honestly quite messy now.
|
Any chance this will get added in soon? :) 👍 |
|
I'll try to get the refactoring done tonight. But I don't think we have any schedule for the next release. |
|
@matschaffer something like this? Unfortunately it doesn't look any cleaner to me, but should for sure handle quotes and spaces better. |
|
Exactly. Wasn't really hoping for any beauty just safety. |
|
Rebased onto master. I tested manually and seemed to work as expected. Didn't run integration tests though. |
Quote rsync paths to avoid problems with spaces in dir names
|
I'll give em a kick. |
Fixes #281