A shell function to copy files and directories, preserving directory structure

Question

For a script, I came up with a shell function that might be generally useful. Before I add it to the code base, I’d like to hear someone’s opinion on this.

I don’t write shell scripts often and had to look up a lot of things.

The idea of this function is to copy stuff from a source folder to a destination folder with the same folder structure above the source folder, but – in contrast to e.g. rsync, only select files or folders.

# Copy files with parent directories to a destination
# $1: Source root directory
# $2: Source relative path(s) to be copied; may be single files or directories; may contain wildcards; may contain '..', but not to undo Source root.
# $3: Destination root directory
dircp() {
    if [ $# -lt 3 ]; then
        echo 'Error: Too few arguments.'
        echo "Hint: Use"
        echo "    dircp . '<relpath1>' '<relpath2>' ... <destination>"
        echo "to copy from the current directory."
        return 1
    fi
    if [ ! -d $1 ]; then
        echo "Error: Source root directory $1 does not exist (or is not a directory)."
        return 1
    fi
    if [ ! -d ${!#} ]; then
        echo "Error: Destination root directory $3 does not exist (or is not a directory)."
        return 1
    fi
    impl() {
        srcroot=$1
        dstroot=${!#}
        for srcpath in ${@:2:$#-2}; do
            if ! ls -d "$srcpath" > /dev/null 2>&1; then
                echo "Path $srcpath does not exist!"
                return 1
            fi
            dstpath="$dstroot/${srcpath#$srcroot/}"
            if ! (mkdir -p $(dirname "$dstpath") && cp -r "$srcpath" "$dstroot/${srcpath#$srcroot/}"); then
                break
            fi
        done
    }
    for srcpath in ${@:2:$#-2}; do
        impl $1 $1/$srcpath ${!#} # expands unexpanded wildcards in $2
    done
}

For example:

dircp src 'a/b/c/*.txt' dst

copies the text files in src/a/b/c to dst/a/b/c and notably creates a, b, and/or c if they’re not present. Only dst has to be present. Multiple relative paths can be passed.

---

Probably one thing that can be improved is if ! ls -d "$srcpath" > /dev/null 2>&1; then to test if the file exists. I tried if [ ! -a "$srcpath" ]; then, but then I need quotes around the second argument of dircp, when they should be largely optional.

Answer 1

UI / design

I respectfully reject the premise that this is an improvement on rsync.

For an "easy" copy, almost any tool will be adequate, including cp -r and tar cf - . | (cd /some/where && tar xvf -) -- that second one works well with ssh pipelines.

But sometimes we need to worry about network mounts and "big" files. In which case the twin spectres of "disk full" and CTRL/C interrupt are ever present. In such cases, I tend to strongly value the fact that rsync never exposes a partial copy to the destination filesystem. It (roughly) will copy ReadMe as .123tmpReadMe, and do an atomic rename after all the bits have arrived.

That said, I do confess that rsync's distinction between directory foo and foo/ takes some getting used to, so I would encourage writing a bash frontend script which accepts whatever you feel would be an improved syntax, and then issues an rsync command to get the work done, revealing the command line args being used.

relative paths

# $2: Source relative path(s) to be copied; may be single files or directories; may contain wildcards; may contain '..', but not to undo Source root.

Thank you for writing such documentation comments, I appreciate it.

I'm afraid I failed to understand the meaning of "not to undo Source root". I initially parsed it as a security related item, don't go above Source root? But that doesn't seem relevant for a command line tool invoked by a trusted user.

Examples wouldn't hurt, such as confirming that a ** recursive wildcard is supported.

The whole plural aspect of jamming "paths" and "files" into the $2 parameter seems unfortunate. Usual unix approach would be to put them at the end, so we can have one or more without worrying about quoting details.

chdir

Maybe it's my limited imagination? But I can't imagine why I would want that mandatory $1 argument.

Often I want bash TAB completion of pathnames, so I cd to the source directory, and now the command becomes dircp .

Or maybe I don't need completion, so I type (cd /some/where && cp foo /baz). For makefiles that works out to make -C /some/where target; consider adopting an optional -C dir argument for dircp.

shebang

I assume you issued $ chmod a+x dircp. That should always go hand-in-hand with an initial shebang. For example:

#! /usr/bin/env bash

Also acceptable: #! /bin/bash

ls antipattern

            if ! ls -d "$srcpath" ...

This crops up often enough that FAQ pages are devoted to it.

I imagine that [ -e "$srcpath" ] would suffice. Consult the test man page for details. (Kudos for getting the quoting correct here!)

quoting

In my opinion, bash is not an easy language to author solid code in. There's a lot of details, and punctuation. Starting with quoting nonsense.

Please understand that echo "a b" c is very different from echo a b c. Alas, it is all too easy for a SPACE embedded within a filename to be lost in that way. So we kind of have to be paranoid about quoting everything, and shellcheck can help. Anything submitted to CodeReview should have already gone through a relevant linter and passed it cleanly. Here's the results I obtained.

In dircp.sh line 14:
    if [ ! -d $1 ]; then
              ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ ! -d "$1" ]; then


In dircp.sh line 25:
        for srcpath in ${@:2:$#-2}; do
                       ^---------^ SC2068 (error): Double quote array expansions to avoid re-splitting elements.


In dircp.sh line 30:
            dstpath="$dstroot/${srcpath#$srcroot/}"
                                        ^------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

Did you mean: 
            dstpath="$dstroot/${srcpath#"$srcroot"/}"


In dircp.sh line 31:
            if ! (mkdir -p $(dirname "$dstpath") && cp -r "$srcpath" "$dstroot/${srcpath#$srcroot/}"); then
                           ^-------------------^ SC2046 (warning): Quote this to prevent word splitting.
                                                                                         ^------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

Did you mean: 
            if ! (mkdir -p $(dirname "$dstpath") && cp -r "$srcpath" "$dstroot/${srcpath#"$srcroot"/}"); then


In dircp.sh line 36:
    for srcpath in ${@:2:$#-2}; do
                   ^---------^ SC2068 (error): Double quote array expansions to avoid re-splitting elements.


In dircp.sh line 37:
        impl $1 $1/$srcpath ${!#} # expands unexpanded wildcards in $2
             ^-- SC2086 (info): Double quote to prevent globbing and word splitting.
                ^-- SC2086 (info): Double quote to prevent globbing and word splitting.
                   ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
        impl "$1" "$1"/"$srcpath" ${!#} # expands unexpanded wildcards in $2

For more information:
  https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

Recommend you tidy those up, and then come back for a second pass.

Answer 2

I agree with @J_H that you essential try to reimplement a solved problem. In particular, you should take a look into the --include and --exclude options of tar.

That being said, there are quite a few general things which can be said about your script, whereas I'll skip what was already mentioned by @J_H.

    if [ $# -lt 3 ]; then

If you can assume that this is indeed run by bash, I'd always prefer a conditional command over a test command, because it has less quoting issues and easier syntax. E.g. while shellcheck will point out quoting issues for [ ! -d $1 ] there is no such issue for [[ ! -d $1 ]]. And for the above you could write [[ $# < 3 ]] which is arguably more readable.

On the other hand, if you want to support other shells, you may have to keep the test commands and carefully quote everything.

        echo 'Error: Too few arguments.'
        echo "Hint: Use"
        echo "    dircp . '<relpath1>' '<relpath2>' ... <destination>"
        echo "to copy from the current directory."
        return 1

This is clearly error output and should be redirected to stderr by redirecting each echo via >&2. You can also group the echos together, so you don't have to repeat the redirect in each line:

        {
            echo 'Error: Too few arguments.'
            echo "Hint: Use"
            echo "    dircp . '<relpath1>' '<relpath2>' ... <destination>"
            echo "to copy from the current directory."
        } >&2

The same thing applies of course for the other error messages. In general, not only error messages should be redirected to stderr, but any kind of diagnostic messages which are not part of the main output of the program.

    if [ ! -d ${!#} ]; then
        echo "Error: Destination root directory $3 does not exist (or is not a directory)."

The error message doesn't match the test. Also, I don't think I ever saw referencing the last parameter via ${!#}, I think using ${@: -1} is more idiomatic.

    impl() {

There is no such thing as a 'local' function in bash. You're essentially defining this function anew on each dircp call and leaking it to the current shell namespace. Instead, it's more idiomatic if you define it outside of dircp and prefix its name with __ to indicate that it's not meant to be called directly. (It should get a better name than impl though.)

Alternatively, you could wrap the function definition and its call in a subshell via ( ... ). But while that works and doesn't leak anything, I'm afraid it isn't very idiomatic.

The best solution would probably be to get rid of enclosing dircp function and just make an executable script called dircp with a proper shebang line, as @J_H already indicated. Having a function is useful if you need to modify the current shell environment, but I don't see that use case here.

        srcroot=$1
        dstroot=${!#}

These should be declared as local otherwise they leak outside. I'd even use local -r here, since you don't need to reassign them.

        for srcpath in ${@:2:$#-2}; do

Also loop variables should be declared local, otherwise they are leaking.

        impl $1 $1/$srcpath ${!#} # expands unexpanded wildcards in $2

Seems you missed an opportunity here to reorder the arguments to make your life a little easier. Since impl is only an internal helper function, you can change the signature at will and pass the last argument as second. I.e.

        impl "$1" "${@: -1}" $1/$srcpath  # expands unexpanded wildcards in $2

Then within impl you can do a shift 2 before the for loop, which means you don't have to repeat the ugly ${@:2:$#-2} since all loop items are in $@ now (which are the default items for for).