cytopia / cytopia/linux-timemachine

Allow rsync options to be passed without requiring double quoting (--filter="'dir-merge /.rsync-filter'")

Open
#84 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Shell
Stars
841
Forks
75
PR merge metrics
No merged PRs in 30d

Description

Considering that I can pass this option simply like this when using rsync directly...

```
# rsync -r --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 -- --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 --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:
```diff
- $* \
+ \"$*\" \
```

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 -- --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](http://mywiki.wooledge.org/WordSplitting) 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 4740817b61325852b0cd677da29f5a92fd48cd6e, 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? :cry:

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...

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.