-
Notifications
You must be signed in to change notification settings - Fork 71
Description
Considering that I can pass this option simply like this when using rsync directly...
# rsync -r <source> <dest> --filter='dir-merge /.rsync-filter'
, it seems like a surprising behavior that when you try to pass these exact same rsync args to timemachine, it results in an error:
# timemachine <source> <dest> -- --filter='dir-merge /.rsync-filter'
unexpected end of filter rule: dir-merge
It appears that timemachine is passing these args on to rsync without any quotes around it at all, the same as this:
# rsync -r <source> <dest> --filter=dir-merge /.rsync-filter
unexpected end of filter rule: dir-merge
I don't know off-hand what the right solution is, but IMHO this is a bug.
Attempts to solve...
I tried changing this line:
- $* \
+ \"$*\" \and in fact it does fix it for the simple example above. However, as soon as you add multiple pass-through args, it fails, because it seems to be sending them through as a single arg:
# timemachine <source> <dest> -- --progress --filter='dir-merge /.rsync-filter'
rsync: --progress --filter=dir-merge /.rsync-filter: unknown option
I suspect this behavior has something to do with the attempt to construct the command to run in a string and then eval it — which is very difficult to do properly and securely and handle all edge cases (see http://mywiki.wooledge.org/BashFAQ/048). Presumably word splitting is happening when we don't want it to be?
Why are we using eval anyway? I see that we were not using eval here in 4740817, for example. And of curiousity, I checked out that commit, and tried my example and it works!
I believe this could be solved fairly easily using an array of args rather than a string (see http://mywiki.wooledge.org/BashFAQ/050). However, it looks like you are using sh rather than bash, so I don't think we can use arrays, right? 😢
As the FAQ above says:
POSIX sh has no arrays, so the closest you can come is to build up a list of elements in the positional parameters.
I'll push up a MR with that approach and see if it works better... See #85.
See also
While you're at it, see if this is related to #82 and whether the same general solution could apply to both...