threads / patch / 3290

patch, 3 partsCall extended-semantics commands through variables.

Subject: [PATCH 1/3] Call extended-semantics commands through variables.

## tl;dr

7 messages between Feb 10, 2006 and Feb 14, 2006. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Jason Riedy· Feb 10, 2006, 23:35 UTC · lore

In some places, git shell scripts rely on semantics for xargs, find, and cpio that do not exist in all versions of those commands. Both xargs and find rely on -0 for handling multi-word names, and for some reason the cpio calls do not work with pkgsrc's default cpio.

Replacing all such calls with calls through variables allows more portability. Also, the variables can point to debugging scripts that log arguments, outputs, etc.

The variables are just XARGS, FIND, and CPIO. No GIT_ was appended so a user can set those once for all scripts that may use them (e.g. configure). A follow-on patch will modify the Makefile to allow installation-specific defaults.

Tested on Solaris 8 with those arguments pointing at GNU tools as well as some Linux versions. (The AIX machine I use is down for maintenance right now.)

Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
---
 git-add.sh           |    3 ++-
 git-clone.sh         |   10 ++++++----
 git-count-objects.sh |    8 +++++---
 git-grep.sh          |    3 ++-
 git-ls-remote.sh     |    4 +++-
 git-merge.sh         |    5 +++--
 git-prune.sh         |    5 +++--
 git-push.sh          |    5 +++--
 git-repack.sh        |    3 ++-
 9 files changed, 29 insertions(+), 17 deletions(-)
846024657d04675a762fd9edaba3c0612f616a41
Show changes to 9 files +29 −17

git-add.sh, git-clone.sh, git-count-objects.sh, git-grep.sh, git-ls-remote.sh, git-merge.sh, git-prune.sh, git-push.sh, git-repack.sh

diff --git a/git-add.sh b/git-add.sh
index f719b4b..bd79b4f 100755
--- a/git-add.sh
+++ b/git-add.sh
@@ -3,6 +3,7 @@
 USAGE='[-n] [-v] <file>...'
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
+: ${XARGS:=xargs}
 
 show_only=
 verbose=
@@ -35,7 +36,7 @@ else
 fi |
 case "$show_only" in
 true)
-	xargs -0 echo ;;
+	${XARGS} -0 echo ;;
 *)
 	git-update-index --add $verbose -z --stdin ;;
 esac
diff --git a/git-clone.sh b/git-clone.sh
index 47f3ec9..2c9b45e 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -7,6 +7,8 @@
 
 # See git-sh-setup why.
 unset CDPATH
+: ${FIND:=find}
+: ${CPIO:=cpio}
 
 usage() {
 	echo >&2 "Usage: $0 [--bare] [-l [-s]] [-q] [-u <upload-pack>] [-o <name>] [-n] <repo> [<dir>]"
@@ -141,19 +143,19 @@ yes,yes)
 	no)
 	    # See if we can hardlink and drop "l" if not.
 	    sample_file=$(cd "$repo" && \
-			  find objects -type f -print | sed -e 1q)
+			  ${FIND} objects -type f -print | sed -e 1q)
 
 	    # objects directory should not be empty since we are cloning!
 	    test -f "$repo/$sample_file" || exit
 
 	    l=
-	    if ln "$repo/$sample_file" "$GIT_DIR/objects/sample" 2>/dev/null
+	    if ln "$repo/$sample_file" "$DIR/objects/sample" 2>/dev/null
 	    then
 		    l=l
 	    fi &&
 	    rm -f "$GIT_DIR/objects/sample" &&
 	    cd "$repo" &&
-	    find objects -depth -print | cpio -puamd$l "$GIT_DIR/" || exit 1
+	    ${FIND} objects -depth -print | ${CPIO} -puamd$l "$GIT_DIR/" || exit 1
 	    ;;
 	yes)
 	    mkdir -p "$GIT_DIR/objects/info"
@@ -234,7 +236,7 @@ then
 		"URL: $repo
 Pull: $head_points_at:$origin" &&
 		git-update-ref "refs/heads/$origin" $(git-rev-parse HEAD) &&
-		(cd "$GIT_DIR" && find "refs/heads" -type f -print) |
+		(cd "$GIT_DIR" && ${FIND} "refs/heads" -type f -print) |
 		while read ref
 		do
 			head=`expr "$ref" : 'refs/heads/\(.*\)'` &&
diff --git a/git-count-objects.sh b/git-count-objects.sh
index 40c58ef..5a4550c 100755
--- a/git-count-objects.sh
+++ b/git-count-objects.sh
@@ -4,6 +4,8 @@
 #
 
 GIT_DIR=`git-rev-parse --git-dir` || exit $?
+: ${FIND:=find}
+: ${XARGS:=xargs}
 
 dc </dev/null 2>/dev/null || {
 	# This is not a real DC at all -- it just knows how
@@ -20,12 +22,12 @@ dc </dev/null 2>/dev/null || {
 	}
 }
 
-echo $(find "$GIT_DIR/objects"/?? -type f -print 2>/dev/null | wc -l) objects, \
+echo $(${GIT_FIND} "$GIT_DIR/objects"/?? -type f -print 2>/dev/null | wc -l) objects, \
 $({
     echo 0
     # "no-such" is to help Darwin folks by not using xargs -r.
-    find "$GIT_DIR/objects"/?? -type f -print 2>/dev/null |
-    xargs du -k "$GIT_DIR/objects/no-such" 2>/dev/null |
+    ${FIND} "$GIT_DIR/objects"/?? -type f -print 2>/dev/null |
+    ${XARGS} du -k "$GIT_DIR/objects/no-such" 2>/dev/null |
     sed -e 's/[ 	].*/ +/'
     echo p
 } | dc) kilobytes
diff --git a/git-grep.sh b/git-grep.sh
index ad4f2fe..a58ab37 100755
--- a/git-grep.sh
+++ b/git-grep.sh
@@ -6,6 +6,7 @@
 USAGE='[<option>...] [-e] <pattern> [<path>...]'
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
+: ${XARGS:=xargs}
 
 got_pattern () {
 	if [ -z "$no_more_patterns" ]
@@ -59,4 +60,4 @@ done
 	usage
 }
 git-ls-files -z "${git_flags[@]}" -- "$@" |
-	xargs -0 grep "${flags[@]}" -e "$pattern" --
+	${XARGS} -0 grep "${flags[@]}" -e "$pattern" --
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index 2c9a588..340bde0 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 #
 
+: ${FIND:=find}
+
 usage () {
     echo >&2 "usage: $0 [--heads] [--tags] [-u|--upload-pack <upload-pack>]"
     echo >&2 "          <repository> <refs>..."
@@ -63,7 +65,7 @@ rsync://* )
 		echo "failed	slurping"
 		exit
 	}
-	(cd $tmpdir && find refs -type f) |
+	(cd $tmpdir && ${FIND} refs -type f) |
 	while read path
 	do
 		cat "$tmpdir/$path" | tr -d '\012'
diff --git a/git-merge.sh b/git-merge.sh
index dc17baf..c9b03c8 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -6,6 +6,7 @@
 
 USAGE='[-n] [--no-commit] [-s <strategy>]... <merge-message> <head> <remote>+'
 . git-sh-setup
+: ${CPIO:=cpio}
 
 LF='
 '
@@ -22,14 +23,14 @@ dropsave() {
 savestate() {
 	# Stash away any local modifications.
 	git-diff-index -z --name-only $head |
-	cpio -0 -o >"$GIT_DIR/MERGE_SAVE"
+	${CPIO} -0 -o >"$GIT_DIR/MERGE_SAVE"
 }
 
 restorestate() {
         if test -f "$GIT_DIR/MERGE_SAVE"
 	then
 		git reset --hard $head
-		cpio -iuv <"$GIT_DIR/MERGE_SAVE"
+		${CPIO} -iuv <"$GIT_DIR/MERGE_SAVE"
 		git-update-index --refresh >/dev/null
 	fi
 }
diff --git a/git-prune.sh b/git-prune.sh
index c5a5d29..781393f 100755
--- a/git-prune.sh
+++ b/git-prune.sh
@@ -2,6 +2,7 @@
 
 USAGE='[-n] [--] [<head>...]'
 . git-sh-setup
+: ${XARGS:=xargs}
 
 dryrun=
 echo=
@@ -27,7 +28,7 @@ sed -ne '/unreachable /{
     s|\(..\)|\1/|p
 }' | {
 	cd "$GIT_OBJECT_DIRECTORY" || exit
-	xargs $echo rm -f
+	${XARGS} $echo rm -f
 	rmdir 2>/dev/null [0-9a-f][0-9a-f]
 }
 
@@ -37,7 +38,7 @@ if redundant=$(git-pack-redundant --all 
 then
 	if test "" = "$dryrun"
 	then
-		echo "$redundant" | xargs rm -f
+		echo "$redundant" | ${XARGS} rm -f
 	else
 		echo rm -f "$redundant"
 	fi
diff --git a/git-push.sh b/git-push.sh
index 706db99..1a6c96d 100755
--- a/git-push.sh
+++ b/git-push.sh
@@ -2,6 +2,7 @@
 
 USAGE='[--all] [--tags] [--force] <repository> [<refspec>...]'
 . git-sh-setup
+: ${FIND:=find}
 
 # Parse out parameters and then stop at remote, so that we can
 # translate it using .git/branches information
@@ -46,9 +47,9 @@ case "$has_all" in
 '')
 	case "$do_tags,$#" in
 	yes,1)
-		set x $(cd "$GIT_DIR/refs" && find tags -type f -print) ;;
+		set x $(cd "$GIT_DIR/refs" && ${FIND} tags -type f -print) ;;
 	yes,*)
-		set x $(cd "$GIT_DIR/refs" && find tags -type f -print) \
+		set x $(cd "$GIT_DIR/refs" && ${FIND} tags -type f -print) \
 		    $(get_remote_refs_for_push "$@") ;;
 	,*)
 		set x $(get_remote_refs_for_push "$@") ;;
diff --git a/git-repack.sh b/git-repack.sh
index 1fafb6e..56a33a7 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -5,6 +5,7 @@
 
 USAGE='[-a] [-d] [-l] [-n]'
 . git-sh-setup
+: ${FIND:=find}
 	
 no_update_info= all_into_one= remove_redundant= local=
 while case "$#" in 0) break ;; esac
@@ -36,7 +37,7 @@ case ",$all_into_one," in
 
 	# Redundancy check in all-into-one case is trivial.
 	existing=`cd "$PACKDIR" && \
-	    find . -type f \( -name '*.pack' -o -name '*.idx' \) -print`
+	    ${FIND} . -type f \( -name '*.pack' -o -name '*.idx' \) -print`
 	;;
 esac
 if [ "$local" ]; then

-- 
1.1.6.g0d39d
Junio C Hamano· Feb 11, 2006, 06:25 UTC · re: Jason Riedy · lore

Re: [PATCH 1/3] Call extended-semantics commands through variables.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
Show 5 quoted lines
> The variables are just XARGS, FIND, and CPIO.  No GIT_
> was appended so a user can set those once for all scripts
> that may use them (e.g. configure).  A follow-on patch
> will modify the Makefile to allow installation-specific
> defaults.

The use of FIND or CPIO in git clone does not need -0 (and the code does not use -0, nor your patch adds -0), so you should not have to override them this way. Except refnames, the file names are not something under arbitrary user control and usual LF termination would work fine. Even refnames cannot contain LF in them. Same thing for FIND in ls-remote and git-push.

FIND in count-objects only lists .git/objects/?? so filenames there are already well-behaved and you do not need -0 there for FIND nor XARGS. Same thing for XARGS in git-prune.sh and FIND in repack (BTW, you got count-objects one wrong; there is a leftover GIT_FIND there).

Although most of what your patch does seems to be unnecessary, reviewing this patch gave me an opportunity to see if we fail to use -0 where we should, and I am grateful for it.

I did not see any place we did not use -0 when we should, except one. The places we _do_ use -0 currently should be converted with something like your patch to use -0 capable version of the tool.

The exception is where finding refnames from rsync'ed copy of a remote (the remote may have screwed-up refnames just to be hostile), but nobody should be using rsync transfer anyway, so...

Jason Riedy· Feb 11, 2006, 23:10 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] Call extended-semantics commands through variables.

And Junio C Hamano writes:
 - The use of FIND or CPIO in git clone does not need -0 (and the
 - code does not use -0, nor your patch adds -0), so you should not
 - have to override them this way.

I'm not sure what's up with cpio, but git causes pkgsrc's default cpio to segfault on my Solaris machines. It's easier to point CPIO at a different cpio than debug a utility I've never really used. ;) I thought it was git-clone breaking in the tests, but it could have been git-merge. I'll check again when I get a chance.

And I worry about using different programs in different scripts, so I just changed all of them.

 - (BTW, you got count-objects one wrong; there is a leftover 
 - GIT_FIND there).

Friday afternoon patching, sorry. That also means either that count-objects has no test cases or that branch of it is not exercised by tests.

 - The places we _do_ use -0 currently should be converted
 - with something like your patch to use -0 capable version of the
 - tool.

Again, I'm not very comfortable using different finds or xargs in different places. But if you want, I'll re-do the patch with just those locations changed, and I'll double- check which cpio invocation is breaking.

 - [...] but nobody should be using rsync transfer anyway,
 - so...

Is there a better way of grabbing all the tags now? I haven't kept track, as I haven't had to do that in a while.

Jason
Junio C Hamano· Feb 11, 2006, 23:32 UTC · re: Jason Riedy · lore

Re: [PATCH 1/3] Call extended-semantics commands through variables.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
> And I worry about using different programs in different 
> scripts, so I just changed all of them.
That's a good point.  I stand corrected.
Jason Riedy· Feb 14, 2006, 05:12 UTC · re: Junio C Hamano · lore

git 1.2 works on Solaris, AIX [was Re: [PATCH 1/3] Call extended-semantics commands through variables.]

The AIX machines I work on are back, and it looks like my patches are unnecessary, at least for my use. It'd be cute to allow builders to point at GNU tools, but not terribly useful. The File::Find patch to git-archimport.perl might be nice, but it functions as-is.

To have diff and merge on my path with this AIX platform, I have to pull *all* the GNU tools into my path. (NERSC uses modules.) I suspect that is a rather common setup, so it's not worth the serious surgery to redirect diff and merge. diff is used in C and shell, and merge is in shell, Perl, and Python sources.

And pkgsrc on Solaris appears happy using GNU's cpio (under archivers/gcpio) rather than its default, plain one. I hadn't realized I could replace it easily.

So with the GNU tools in the path and a properly built Python, the mainline code works on Solaris 8 and AIX.

For posterity: Any problems with git-merge-recursive.py on AIX
likely are a yucky Python/AIX problem.  The sha has 'sem_trywait: 
Permission denied\n' prepended to it a few times.  You need to 
rebuild Python with HAVE_BROKEN_POSIX_SEMAPHORES:
  https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1106262&group_id=5470

If anyone really wants to point at particular tools but not require them in the user's path, the simplest way would be to link the correct tools (or wrappers) into the GIT_EXEC_PATH and prepend that to the PATH *everywhere*. But it's not worth the effort until someone really needs it.

Jason
Junio C Hamano· Feb 12, 2006, 00:36 UTC · re: Jason Riedy · lore

Re: [PATCH 1/3] Call extended-semantics commands through variables.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
> Is there a better way of grabbing all the tags now?  I haven't
> kept track, as I haven't had to do that in a while.

Recent 'git-fetch' automatically follows tags that are attached to commits you slurp (following example set by Cogito), to reduce the need to grab all tags to begin with. That would not help tags that are attached to objects that are not part of branches you are tracking; you can use 'git-fetch --tags' for them.

← back to recent threads