Allow git cp to create destination folder automatically

[weiw005]

fixed #1089

[hyperupcall]

It looks like there is a race and an extra subshell here. What about something like this?:

DESTINATION_FILENAME=${DESTINATION_FILENAME%/}
if ! mkdir -p "${DESTINATION_FILENAME%/*}"; then
  echo 1>&2 "Failed to create destination directory: $DESTINATION_DIR"
  exit 80
fi

[weiw005]

It looks like there is a race and an extra subshell here. What about something like this?:

DESTINATION_FILENAME=${DESTINATION_FILENAME%/}
if ! mkdir -p "${DESTINATION_FILENAME%/*}"; then
  echo 1>&2 "Failed to create destination directory: $DESTINATION_DIR"
  exit 80
fi

Thanks! Removed the subprocess command. PTAL.

(BTW my knowledge with shell script is very limited, so open to further suggestions)

[spacewander]

We can use mkdir -p directly as the cmd won't return error if the dir exists?

[weiw005]

Make sense. Removed this check.

[spacewander]

What if the DESTINATION_FILENAME doesn't contain the directory, like ./foo?

[weiw005]

Good question! Although the code still works for ./foo (because DESTINATION_DIR will simply be ./ and creating it is a harmless no-op), another case that I tested did lead to incorrect behavior which is when the DESTINATION_FILENAME is just a filename. The previous revision would create a folder of that name, and make a copy of the source file into that folder (rather than copy to a file of that name).

I've now switched the condition to be checking whether DESTINATION_FILENAME contains any slash characters - if not the folder creation is skipped.

[spacewander]

@hyperupcall
Please take a look when you have free time.

[hyperupcall]

The new revision looks good to me - fixed that big.

My only comment is that it seems git-cp would previously silently work if a directory name were passed, it ended with /, and the directory already existed. If this new failure is acceptable then these changes LGTM!

[weiw005]

@spacewander @hyperupcall Thanks for the review! Would either of you be able to click the merge button for me? I don't have write access to this repo.

[hyperupcall]

@weiw005 @spacewander will be able to do it

[spacewander]

Merged. Thanks!
Also thanks @hyperupcall for the review :)