Git development
 help / color / mirror / Atom feed
* [RFC/PATCH] git-rev-parse vs IFS
@ 2005-07-17  6:59 Junio C Hamano
  2005-07-17  8:06 ` [PATCH] rev-parse --sq bugfixes Junio C Hamano
  2005-07-17  8:45 ` [PATCH] Audit rev-parse users Junio C Hamano
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-07-17  6:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

"git whatchanged" does not work with parameters that have an IFS
character; this includes a filename with SP in it.  A silly
example:

    $ git-whatchanged --max-count=1
    diff-tree 98800286dffc76736549d7279622157ca2be9a3b (from 9171e0...
    Author: Junio C Hamano <junkio@cox.net>
    Date:   Sat Jul 16 23:13:03 2005 -0700

        add foo

    :000000 100644 0000000000000000000000000000000000000000 8b13789...
    $ git-whatchanged --max-count=1 'foo bar'
    $ ;# no output!

This is because the tail parameter to git-diff-tree used in
git-whatchanged is $(git-rev-parse --no-revs "$@"), and the
output from git-rev-parse is split at $IFS, so it ends up
getting "foo" and "bar" separately).

If it were just embedded SP and TAB in the filnames, I would say
we can set IFS to LF in the script that use git-rev-parse,
declare that we do not support filenames with embedded LF, and
be done with it.  The thing is, for pickaxe, it is very natural
to give a multi-line parameter, so setting IFS to LF is not a
general workaround.

Doing this portably is possible but a bit of pain.  I can add
git-rev-parse ability to sq_quote its outputs, and change the
callers to build a command line to "eval" using it.  Here is an
RFC patch that does it.  My barf-o-tolerance for shell quoting
and evaling is usually much higher than others, so while I feel
this is not too ugly, it may offend your aesthetics.

------------
Help scripts that use git-rev-parse to grok args with SP/TAB/LF

The git-rev-parse command uses LF to separate each arguments it
parses, so its users at least need to set IFS to LF to be able
to handle filenames with embedded SPs and TABs.  Some commands,
however, can take and do expect arguments with embedded LF
(notably, -Spickaxe of diff family), so even this workaround
does not work for them.

When --sq flag to git-rev-parse is given, instead of showing one
argument per line, it outputs arguments quoted for consumption
with "eval" by the caller, to remedy this situation.

As an example, this patch converts git-whatchanged to use this
new feature.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 git-whatchanged |    9 ++++++---
 rev-parse.c     |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 5 deletions(-)

79cdb9505b0ee9a47969c3cbc8c8bf2a35b23ca8
diff --git a/git-whatchanged b/git-whatchanged
--- a/git-whatchanged
+++ b/git-whatchanged
@@ -1,4 +1,7 @@
 #!/bin/sh
-git-rev-list $(git-rev-parse --default HEAD --revs-only "$@") |
-	git-diff-tree --stdin --pretty -r $(git-rev-parse --no-revs "$@") |
-	LESS="$LESS -S" ${PAGER:-less}
+rev_list_args=$(git-rev-parse --sq --default HEAD --revs-only "$@") &&
+diff_tree_args=$(git-rev-parse --sq --no-revs "$@") &&
+
+eval "git-rev-list $rev_list_args" |
+eval "git-diff-tree --stdin --pretty -r $diff_tree_args" |
+LESS="$LESS -S" ${PAGER:-less}
diff --git a/rev-parse.c b/rev-parse.c
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -15,6 +15,7 @@ static int do_rev_argument = 1;
 static int output_revs = 0;
 static int flags_only = 0;
 static int no_flags = 0;
+static int output_sq = 0;
 
 #define NORMAL 0
 #define REVERSED 1
@@ -54,14 +55,41 @@ static void show_rev(int type, const uns
 	if (no_revs)
 		return;
 	output_revs++;
+
+	/* Hexadecimal string plus possibly a carret;
+	 * this does not have to be quoted even under output_sq.
+	 */
 	printf("%s%s\n", type == show_type ? "" : "^", sha1_to_hex(sha1));
 }
 
+static void show(const char *arg)
+{
+	if (output_sq) {
+		int sq = '\'';
+		const char *next;
+
+		putchar(sq);
+		while (*arg) {
+			next = strchr(arg, sq);
+			if (!next) {
+				fputs(arg, stdout);
+				break;
+			}
+			fputs("'\\''", stdout);
+			arg = next + 1;
+		}
+		putchar(sq);
+		putchar('\n');
+	}
+	else
+		puts(arg);
+}
+
 static void show_rev_arg(char *rev)
 {
 	if (no_revs)
 		return;
-	puts(rev);
+	show(rev);
 }
 
 static void show_norev(char *norev)
@@ -70,7 +98,7 @@ static void show_norev(char *norev)
 		return;
 	if (revs_only)
 		return;
-	puts(norev);
+	show(norev);
 }
 
 static void show_arg(char *arg)
@@ -328,6 +356,10 @@ int main(int argc, char **argv)
 				single_rev = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--sq")) {
+				output_sq = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--not")) {
 				show_type ^= REVERSED;
 				continue;

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] Audit rev-parse users.
@ 2005-07-23  2:09 Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-07-23  2:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

This patch changes rev-parse users that pass a single argument
that is supposed to be a rev parameter to use "--verify".

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 git-checkout-script |    2 +-
 git-cherry          |    8 ++++----
 git-commit-script   |    2 +-
 git-rebase-script   |    8 ++++----
 git-resolve-script  |    4 ++--
 5 files changed, 12 insertions(+), 12 deletions(-)

f3818c97f985fe20d24fe3d12cca5ce1cfbf8d74
diff --git a/git-checkout-script b/git-checkout-script
--- a/git-checkout-script
+++ b/git-checkout-script
@@ -22,7 +22,7 @@ while [ "$#" != "0" ]; do
 		force=1
 		;;
 	*)
-		rev=$(git-rev-parse --verify --revs-only "$arg^0") || exit
+		rev=$(git-rev-parse --verify "$arg^0") || exit
 		if [ -z "$rev" ]; then
 			echo "unknown flag $arg"
 			exit 1
diff --git a/git-cherry b/git-cherry
--- a/git-cherry
+++ b/git-cherry
@@ -28,11 +28,11 @@ The output is intended to be used as:
 '
 
 case "$#" in
-1) linus=`git-rev-parse "$1"` &&
-   junio=`git-rev-parse HEAD` || exit
+1) linus=`git-rev-parse --verify "$1"` &&
+   junio=`git-rev-parse --verify HEAD` || exit
    ;;
-2) linus=`git-rev-parse "$1"` &&
-   junio=`git-rev-parse "$2"` || exit
+2) linus=`git-rev-parse --verify "$1"` &&
+   junio=`git-rev-parse --verify "$2"` || exit
    ;;
 *) echo >&2 "$usage"; exit 1 ;;
 esac
diff --git a/git-commit-script b/git-commit-script
--- a/git-commit-script
+++ b/git-commit-script
@@ -16,7 +16,7 @@ do
     -m) shift
         case "$#" in
 	0) usage ;;
-	*) use_commit=`git-rev-parse "$1"` ||
+	*) use_commit=`git-rev-parse --verify "$1"` ||
 	   exit ;;
 	esac
 	;;
diff --git a/git-rebase-script b/git-rebase-script
--- a/git-rebase-script
+++ b/git-rebase-script
@@ -11,11 +11,11 @@ upstream tree.'
 : ${GIT_DIR=.git}
 
 case "$#" in
-1) linus=`git-rev-parse "$1"` &&
-   junio=`git-rev-parse HEAD` || exit
+1) linus=`git-rev-parse --verify "$1"` &&
+   junio=`git-rev-parse --verify HEAD` || exit
    ;;
-2) linus=`git-rev-parse "$1"` &&
-   junio=`git-rev-parse "$2"` || exit
+2) linus=`git-rev-parse --verify "$1"` &&
+   junio=`git-rev-parse --verify "$2"` || exit
    ;;
 *) echo >&2 "$usage"; exit 1 ;;
 esac
diff --git a/git-resolve-script b/git-resolve-script
--- a/git-resolve-script
+++ b/git-resolve-script
@@ -6,8 +6,8 @@
 #
 . git-sh-setup-script || die "Not a git archive"
 
-head=$(git-rev-parse --revs-only "$1")
-merge=$(git-rev-parse --revs-only "$2")
+head=$(git-rev-parse --verify "$1")
+merge=$(git-rev-parse --verify "$2")
 merge_msg="$3"
 
 dropheads() {

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] Audit rev-parse users.
@ 2005-08-15  9:55 Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-08-15  9:55 UTC (permalink / raw)
  To: git

Make sure that we say --verify when we want to get a single SHA1
name.  Also when we say --verify, --revs-only is redundant.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 git-rebase-script |    2 +-
 git-reset-script  |    4 ++--
 git-tag-script    |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

fb1dea6f0298f4b47ae6a3dc08a1bc2398ef8053
diff --git a/git-rebase-script b/git-rebase-script
--- a/git-rebase-script
+++ b/git-rebase-script
@@ -28,7 +28,7 @@ case "$#" in
 esac
 
 git-read-tree -m -u $ours $upstream &&
-echo "$upstream" >"$GIT_DIR/HEAD" || exit
+git-rev-parse --verify "$upstream^0" >"$GIT_DIR/HEAD" || exit
 
 tmp=.rebase-tmp$$
 fail=$tmp-fail
diff --git a/git-reset-script b/git-reset-script
--- a/git-reset-script
+++ b/git-reset-script
@@ -1,7 +1,7 @@
 #!/bin/sh
 . git-sh-setup-script || die "Not a git archive"
-rev=$(git-rev-parse --revs-only --verify --default HEAD "$@") || exit
-rev=$(git-rev-parse --revs-only --verify $rev^0) || exit
+rev=$(git-rev-parse --verify --default HEAD "$@") || exit
+rev=$(git-rev-parse --verify $rev^0) || exit
 git-read-tree --reset "$rev" && {
 	if orig=$(git-rev-parse --verify HEAD 2>/dev/null)
 	then
diff --git a/git-tag-script b/git-tag-script
--- a/git-tag-script
+++ b/git-tag-script
@@ -47,7 +47,7 @@ if [ -e "$GIT_DIR/refs/tags/$name" -a -z
 fi
 shift
 
-object=$(git-rev-parse --verify --revs-only --default HEAD "$@") || exit 1
+object=$(git-rev-parse --verify --default HEAD "$@") || exit 1
 type=$(git-cat-file -t $object) || exit 1
 tagger=$(git-var GIT_COMMITTER_IDENT) || exit 1
 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-08-15  9:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-17  6:59 [RFC/PATCH] git-rev-parse vs IFS Junio C Hamano
2005-07-17  8:06 ` [PATCH] rev-parse --sq bugfixes Junio C Hamano
2005-07-18 17:27   ` [PATCH] Help scripts that use git-rev-parse to grok args with SP/TAB/LF Junio C Hamano
2005-07-17  8:45 ` [PATCH] Audit rev-parse users Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2005-07-23  2:09 Junio C Hamano
2005-08-15  9:55 Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox