[PATCH 0/5 v2] contrib/diffall: improvements

[PATCH 0/5 v2] contrib/diffall: improvements

[Tim Henigan]

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

[PATCH 1/5 v2] contrib/diffall: comment actual reason for 'cdup'

[Tim Henigan]

The comment from an earlier commit did not reflect the actual reason this
operation is needed.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

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

[PATCH 2/5 v2] contrib/diffall: create tmp dirs without mktemp

[Tim Henigan]

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 <tim.henigan@gmail.com>
---

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

[PATCH 3/5 v2] contrib/diffall: eliminate use of tar

[Tim Henigan]

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 <tim.henigan@gmail.com>
---

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

[PATCH 4/5 v2] contrib/diffall: eliminate duplicate while loops

[Tim Henigan]

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 <tim.henigan@gmail.com>
---

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

[PATCH 5/5 v2] contrib/diffall: fix cleanup trap on Windows

[Tim Henigan]

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 <tim.henigan@gmail.com>
---

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