# [PATCH 0/5 v2] contrib/diffall: improvements 6 messages from 2012-03-14 to 2012-03-14. Participants: Tim Henigan. Thread: https://gitlist.dev/t/29948 ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 0/5 v2] contrib/diffall: improvements Message-ID: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-1-git-send-email-tim.henigan%40gmail.com ``` This series implements a number of changes to make the script more robust. Changes in v2: - Fixed a bug in patch 3/5 that copied modified working copy files to the wrong tmp directory location. This affected any modified working copy file not located at the repo root. - Added patch 5/5 to insure that the tmp dir is actually deleted when the script is run on Windows. - Shortened the commit message summary line for each patch. Tim Henigan (5): contrib/diffall: comment actual reason for 'cdup' contrib/diffall: create tmp dirs without mktemp contrib/diffall: eliminate use of tar contrib/diffall: eliminate duplicate while loops contrib/diffall: fix cleanup trap on Windows contrib/diffall/git-diffall | 52 ++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 28 deletions(-) -- 1.7.10.rc0 ``` ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 1/5 v2] contrib/diffall: comment actual reason for 'cdup' Message-ID: <1331743086-32304-2-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-2-git-send-email-tim.henigan%40gmail.com In-Reply-To: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> ``` The comment from an earlier commit did not reflect the actual reason this operation is needed. Signed-off-by: Tim Henigan --- v2 did not affect this patch. contrib/diffall/git-diffall | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall index 9bbd27f..d706a6d 100755 --- a/contrib/diffall/git-diffall +++ b/contrib/diffall/git-diffall @@ -36,7 +36,9 @@ fi start_dir=$(pwd) -# needed to access tar utility +# All the file paths returned by the diff command are relative to the root +# of the working copy. So if the script is called from a subdirectory, it +# must switch to the root of working copy before trying to use those paths. cdup=$(git rev-parse --show-cdup) && cd "$cdup" || { echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree" -- 1.7.10.rc0 ``` ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 2/5 v2] contrib/diffall: create tmp dirs without mktemp Message-ID: <1331743086-32304-3-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-3-git-send-email-tim.henigan%40gmail.com In-Reply-To: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> ``` mktemp is not available on all platforms. Instead of littering the code with a work-around, this commit replaces mktemp with a one-line Perl script. Signed-off-by: Tim Henigan --- v2 did not affect this patch. contrib/diffall/git-diffall | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall index d706a6d..443f646 100755 --- a/contrib/diffall/git-diffall +++ b/contrib/diffall/git-diffall @@ -45,13 +45,10 @@ cd "$cdup" || { exit 1 } -# mktemp is not available on all platforms (missing from msysgit) -# Use a hard-coded tmp dir if it is not available -tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || { - tmp=/tmp/git-diffall-tmp.$$ - mkdir "$tmp" || exit 1 -} - +# set up temp dir +tmp=$(perl -e 'use File::Temp qw(tempdir); + $t=tempdir("/tmp/git-diffall.XXXXX") or exit(1); + print $t') || exit 1 trap 'rm -rf "$tmp" 2>/dev/null' EXIT left= -- 1.7.10.rc0 ``` ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 3/5 v2] contrib/diffall: eliminate use of tar Message-ID: <1331743086-32304-4-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-4-git-send-email-tim.henigan%40gmail.com In-Reply-To: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> ``` The 'tar' utility is not available on all platforms (some only support 'gnutar'). An earlier commit created a work-around for this problem, but a better solution is to eliminate the use of 'tar' completely. Signed-off-by: Tim Henigan --- Changes in v2: - Added missing '$name' component to the 'cp' target. This bug resulted in modified working copy files being copied to the wrong location in the tmp directory. It only affected files in sub- directories of the repo root. contrib/diffall/git-diffall | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall index 443f646..f981ac1 100755 --- a/contrib/diffall/git-diffall +++ b/contrib/diffall/git-diffall @@ -202,10 +202,14 @@ then fi done < "$tmp/filelist" else - # Mac users have gnutar rather than tar - (tar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && tar -x)) || { - gnutar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && gnutar -x) - } + while read name + do + if test -e "$name" + then + mkdir -p "$tmp/$right_dir/$(dirname "$name")" + cp "$name" "$tmp/$right_dir/$name" + fi + done < "$tmp/filelist" fi # Populate the tmp/left_dir directory with the files to be compared -- 1.7.10.rc0 ``` ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 4/5 v2] contrib/diffall: eliminate duplicate while loops Message-ID: <1331743086-32304-5-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-5-git-send-email-tim.henigan%40gmail.com In-Reply-To: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> ``` There were 3 instances of a 'while read; do' that used identical logic to populate '/tmp/right_dir'. This commit groups them into a single loop. Signed-off-by: Tim Henigan --- v2 did not affect this patch. contrib/diffall/git-diffall | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall index f981ac1..91a31c8 100755 --- a/contrib/diffall/git-diffall +++ b/contrib/diffall/git-diffall @@ -179,38 +179,32 @@ fi mkdir -p "$tmp/$left_dir" "$tmp/$right_dir" # Populate the tmp/right_dir directory with the files to be compared -if test -n "$right" -then - while read name - do +while read name +do + if test -n "$right" + then ls_list=$(git ls-tree $right "$name") if test -n "$ls_list" then mkdir -p "$tmp/$right_dir/$(dirname "$name")" git show "$right":"$name" >"$tmp/$right_dir/$name" || true fi - done < "$tmp/filelist" -elif test -n "$compare_staged" -then - while read name - do + elif test -n "$compare_staged" + then ls_list=$(git ls-files -- "$name") if test -n "$ls_list" then mkdir -p "$tmp/$right_dir/$(dirname "$name")" git show :"$name" >"$tmp/$right_dir/$name" fi - done < "$tmp/filelist" -else - while read name - do + else if test -e "$name" then mkdir -p "$tmp/$right_dir/$(dirname "$name")" cp "$name" "$tmp/$right_dir/$name" fi - done < "$tmp/filelist" -fi + fi +done < "$tmp/filelist" # Populate the tmp/left_dir directory with the files to be compared while read name -- 1.7.10.rc0 ``` ## Tim Henigan, 2012-03-14 16:38 Subject: [PATCH 5/5 v2] contrib/diffall: fix cleanup trap on Windows Message-ID: <1331743086-32304-6-git-send-email-tim.henigan@gmail.com> URL: https://gitlist.dev/e/1331743086-32304-6-git-send-email-tim.henigan%40gmail.com In-Reply-To: <1331743086-32304-1-git-send-email-tim.henigan@gmail.com> ``` Prior to this commit, the cleanup trap that removes the tmp dir created by the script would fail on Windows. The error was silently ignored by the script. On Windows, a directory cannot be removed while it is the working directory of the process (thanks to Johannes Sixt on the Git list for this info [1]). This commit eliminates the 'cd' into the tmp directory that caused the error. [1]: http://article.gmane.org/gmane.comp.version-control.git/193086 Signed-off-by: Tim Henigan --- This patch was added in v2 of the series. contrib/diffall/git-diffall | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall index 91a31c8..84f2b65 100755 --- a/contrib/diffall/git-diffall +++ b/contrib/diffall/git-diffall @@ -49,7 +49,7 @@ cd "$cdup" || { tmp=$(perl -e 'use File::Temp qw(tempdir); $t=tempdir("/tmp/git-diffall.XXXXX") or exit(1); print $t') || exit 1 -trap 'rm -rf "$tmp" 2>/dev/null' EXIT +trap 'rm -rf "$tmp"' EXIT left= right= @@ -233,9 +233,8 @@ do fi done < "$tmp/filelist" -cd "$tmp" -LOCAL="$left_dir" -REMOTE="$right_dir" +LOCAL="$tmp/$left_dir" +REMOTE="$tmp/$right_dir" if test -n "$diff_tool" then -- 1.7.10.rc0 ```