threads / patch / 17056

patchRe: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

Subject: Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

## tl;dr

10 messages between Jan 8, 2009 and Jan 22, 2009. Diffs are folded; open one to read it.

replies: 9people: 2as markdown or json

Kirill Smelkov· Jan 8, 2009, 18:22 UTC · lore

[PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

This implements `tg patch -i` and `tg patch -w` to see current patch as generated against not-yet-committed index and worktree.

NOTE: unfortunately `git cat-file blob <file>` does not provide an option
to cat file from worktree (only from an object or from index), so I had to
unroll my own `cat file topic:file` with special support for '(i)' and
'(w)' topics.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
---
 README                     |    5 +++--
 contrib/tg-completion.bash |    6 ++++++
 tg-patch.sh                |   31 +++++++++++++++++++++++++------
 tg.sh                      |   21 +++++++++++++++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)
Show changes to 4 files +55 −8

README, contrib/tg-completion.bash, tg-patch.sh, tg.sh

diff --git a/README b/README
index 1d38365..5796112 100644
--- a/README
+++ b/README
@@ -284,8 +284,9 @@ tg patch
 	tg patch will be able to automatically send the patches by mail
 	or save them to files. (TODO)
 
-	TODO: tg patch -i to base at index instead of branch,
-		-w for working tree
+	Options:
+	  -i		base patch generation on index instead of branch
+	  -w		base patch generation on working tree instead of branch
 
 tg mail
 ~~~~~~~
diff --git a/contrib/tg-completion.bash b/contrib/tg-completion.bash
index 9641d04..de8a7b5 100755
--- a/contrib/tg-completion.bash
+++ b/contrib/tg-completion.bash
@@ -359,6 +359,12 @@ _tg_patch ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 
 	case "$cur" in
+	-*)
+		__tgcomp "
+			-i
+			-w
+		"
+		;;
 	*)
 		__tgcomp "$(__tg_topics)"
 	esac
diff --git a/tg-patch.sh b/tg-patch.sh
index dc699d2..db1ad09 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -5,14 +5,25 @@
 
 name=
 
+topic=
+diff_opts=
+diff_committed_only=yes	# will be unset for index/worktree
+
 
 ## Parse options
 
 while [ -n "$1" ]; do
 	arg="$1"; shift
 	case "$arg" in
+	-i)
+		topic='(i)'
+		diff_opts="$diff_opts --cached";;
+		diff_committed_only=;
+	-w)
+		topic='(w)'
+		diff_committed_only=;;
 	-*)
-		echo "Usage: tg [...] patch [NAME]" >&2
+		echo "Usage: tg [...] patch [-i | -w] [NAME]" >&2
 		exit 1;;
 	*)
 		[ -z "$name" ] || die "name already specified ($name)"
@@ -20,31 +31,39 @@ while [ -n "$1" ]; do
 	esac
 done
 
+
+[ -n "$name"  -a  -z "$diff_committed_only" ]  &&
+	die "-i/-w are mutually exclusive with NAME"
+
 [ -n "$name" ] || name="$(git symbolic-ref HEAD | sed 's#^refs/\(heads\|top-bases\)/##')"
 base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)" ||
 	die "not a TopGit-controlled branch"
 
+# if not index/worktree, topic is current branch
+[ -z "$topic" ] && topic="$name"
+
+
 
 setup_pager
 
-git cat-file blob "$name:.topmsg"
+cat_file "$topic:.topmsg"
 echo
-[ -n "$(git grep '^[-]--' "$name" -- ".topmsg")" ] || echo '---'
+[ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
 
 # Evil obnoxious hack to work around the lack of git diff --exclude
 git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
-git diff-tree --name-only "$base_rev" "$name" |
+git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
 	fgrep -vx ".topdeps" |
 	fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
 if [ -s "$git_is_stupid" ]; then
-	cat "$git_is_stupid" | xargs git diff --patch-with-stat "$base_rev" "$name" --
+	cat "$git_is_stupid" | xargs git diff --patch-with-stat $diff_opts "$base_rev" ${diff_committed_only:+"$name"} --
 else
 	echo "No changes."
 fi
 rm "$git_is_stupid"
 
 echo '-- '
-echo "tg: ($base_rev..) $name (depends on: $(git cat-file blob "$name:.topdeps" | paste -s -d' '))"
+echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
 branch_contains "$name" "$base_rev" ||
 	echo "tg: The patch is out-of-date wrt. the base! Run \`$tg update\`."
 
diff --git a/tg.sh b/tg.sh
index b64fc3a..ab0db52 100644
--- a/tg.sh
+++ b/tg.sh
@@ -17,6 +17,27 @@ die()
 	exit 1
 }
 
+# cat_file "topic:file"
+# Like `git cat-file blob $1`, but topics (i) and (w) means index and worktree
+cat_file()
+{
+	arg="$1"
+	case "$arg" in
+	'(w):'*)
+		arg=$(echo "$arg" | tail --bytes=+5)
+		cat "$arg"
+		return
+		;;
+	'(i):'*)
+		# ':file' means cat from index
+		arg=$(echo "$arg" | tail --bytes=+5)
+		git cat-file blob ":$arg"
+		;;
+	*)
+		git cat-file blob "$arg"
+	esac
+}
+
 # setup_hook NAME
 setup_hook()
 {
-- 
tg: (a3a5be1..) t/tg-patch-worktree (depends on: t/tg-patch-setup-pager)
martin f krafft· Jan 8, 2009, 19:53 UTC · re: Kirill Smelkov · lore
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.09.0722 +1300]:
> This implements `tg patch -i` and `tg patch -w` to see current
> patch as generated against not-yet-committed index and worktree.

I think at this early stage, it would make sense to use long options and not reserve short options yet. Unless Petr disagrees, I'd kindly ask you to use long options instead. Once TopGit has been around for a while, we can provide short options for the most important long options.

This is possibly too conservative, but I've been bitten by lack of new letters before because I've used them all up for options that later turned out not to be needed.

I have not yet had the time to actually look at the patch.
-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"when zarathustra was alone... he said to his heart: 'could it be
 possible! this old saint in the forest hath not yet heard of it, that
 god is dead!'"
                                                 - friedrich nietzsche
 
spamtraps: madduck.bogus@madduck.net
Kirill Smelkov· Jan 8, 2009, 20:16 UTC · re: martin f krafft · lore
On Fri, Jan 09, 2009 at 08:53:56AM +1300, martin f krafft wrote:
Show 13 quoted lines
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.09.0722 +1300]:
> > This implements `tg patch -i` and `tg patch -w` to see current
> > patch as generated against not-yet-committed index and worktree.
> 
> I think at this early stage, it would make sense to use long options
> and not reserve short options yet. Unless Petr disagrees, I'd kindly
> ask you to use long options instead. Once TopGit has been around for
> a while, we can provide short options for the most important long
> options.
> 
> This is possibly too conservative, but I've been bitten by lack of
> new letters before because I've used them all up for options that
> later turned out not to be needed.

I agree, but when I found myself needing something like `tg patch --index`, I've spot this in README:

    --- a/README
    +++ b/README
    @@ -284,8 +284,9 @@ tg patch
            tg patch will be able to automatically send the patches by mail
            or save them to files. (TODO)
    
    -       TODO: tg patch -i to base at index instead of branch,
    -               -w for working tree
So I concluded -i/-w was planned from the beginning.

I myself would call these options --index and --work or something like that, but I'll be ok with any option.

Thanks, Kirill

Kirill Smelkov· Jan 8, 2009, 21:11 UTC · re: Kirill Smelkov · lore
I'm sorry, but I've found a mistake in my code for case:
Show changes to 4 files +57 −10

tg-patch.sh, README, contrib/tg-completion.bash, tg.sh

diff --git a/tg-patch.sh b/tg-patch.sh
index db1ad09..d701c54 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -17,8 +17,8 @@ while [ -n "$1" ]; do
        case "$arg" in
        -i)
                topic='(i)'
-               diff_opts="$diff_opts --cached";;
-               diff_committed_only=;
+               diff_opts="$diff_opts --cached";
+               diff_committed_only=;;
        -w)
                topic='(w)'
                diff_committed_only=;;


So here is corrected patch:


From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
To: Petr Baudis <pasky@suse.cz>
Cc: martin f krafft <madduck@madduck.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

This implements `tg patch -i` and `tg patch -w` to see current patch as
generated against not-yet-committed index and worktree.


NOTE: unfortunately `git cat-file blob <file>` does not provide an option
to cat file from worktree (only from an object or from index), so I had to
unroll my own `cat file topic:file` with special support for '(i)' and
'(w)' topics.

Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

---
 README                     |    5 +++--
 contrib/tg-completion.bash |    6 ++++++
 tg-patch.sh                |   31 +++++++++++++++++++++++++------
 tg.sh                      |   21 +++++++++++++++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/README b/README
index 1d38365..5796112 100644
--- a/README
+++ b/README
@@ -284,8 +284,9 @@ tg patch
 	tg patch will be able to automatically send the patches by mail
 	or save them to files. (TODO)
 
-	TODO: tg patch -i to base at index instead of branch,
-		-w for working tree
+	Options:
+	  -i		base patch generation on index instead of branch
+	  -w		base patch generation on working tree instead of branch
 
 tg mail
 ~~~~~~~
diff --git a/contrib/tg-completion.bash b/contrib/tg-completion.bash
index 9641d04..de8a7b5 100755
--- a/contrib/tg-completion.bash
+++ b/contrib/tg-completion.bash
@@ -359,6 +359,12 @@ _tg_patch ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 
 	case "$cur" in
+	-*)
+		__tgcomp "
+			-i
+			-w
+		"
+		;;
 	*)
 		__tgcomp "$(__tg_topics)"
 	esac
diff --git a/tg-patch.sh b/tg-patch.sh
index dc699d2..d701c54 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -5,14 +5,25 @@
 
 name=
 
+topic=
+diff_opts=
+diff_committed_only=yes	# will be unset for index/worktree
+
 
 ## Parse options
 
 while [ -n "$1" ]; do
 	arg="$1"; shift
 	case "$arg" in
+	-i)
+		topic='(i)'
+		diff_opts="$diff_opts --cached";
+		diff_committed_only=;;
+	-w)
+		topic='(w)'
+		diff_committed_only=;;
 	-*)
-		echo "Usage: tg [...] patch [NAME]" >&2
+		echo "Usage: tg [...] patch [-i | -w] [NAME]" >&2
 		exit 1;;
 	*)
 		[ -z "$name" ] || die "name already specified ($name)"
@@ -20,31 +31,39 @@ while [ -n "$1" ]; do
 	esac
 done
 
+
+[ -n "$name"  -a  -z "$diff_committed_only" ]  &&
+	die "-i/-w are mutually exclusive with NAME"
+
 [ -n "$name" ] || name="$(git symbolic-ref HEAD | sed 's#^refs/\(heads\|top-bases\)/##')"
 base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)" ||
 	die "not a TopGit-controlled branch"
 
+# if not index/worktree, topic is current branch
+[ -z "$topic" ] && topic="$name"
+
+
 
 setup_pager
 
-git cat-file blob "$name:.topmsg"
+cat_file "$topic:.topmsg"
 echo
-[ -n "$(git grep '^[-]--' "$name" -- ".topmsg")" ] || echo '---'
+[ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
 
 # Evil obnoxious hack to work around the lack of git diff --exclude
 git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
-git diff-tree --name-only "$base_rev" "$name" |
+git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
 	fgrep -vx ".topdeps" |
 	fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
 if [ -s "$git_is_stupid" ]; then
-	cat "$git_is_stupid" | xargs git diff --patch-with-stat "$base_rev" "$name" --
+	cat "$git_is_stupid" | xargs git diff --patch-with-stat $diff_opts "$base_rev" ${diff_committed_only:+"$name"} --
 else
 	echo "No changes."
 fi
 rm "$git_is_stupid"
 
 echo '-- '
-echo "tg: ($base_rev..) $name (depends on: $(git cat-file blob "$name:.topdeps" | paste -s -d' '))"
+echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
 branch_contains "$name" "$base_rev" ||
 	echo "tg: The patch is out-of-date wrt. the base! Run \`$tg update\`."
 
diff --git a/tg.sh b/tg.sh
index b64fc3a..1762f03 100644
--- a/tg.sh
+++ b/tg.sh
@@ -17,6 +17,27 @@ die()
 	exit 1
 }
 
+# cat_file "topic:file"
+# Like `git cat-file blob $1`, but topics '(i)' and '(w)' means index and worktree
+cat_file()
+{
+	arg="$1"
+	case "$arg" in
+	'(w):'*)
+		arg=$(echo "$arg" | tail --bytes=+5)
+		cat "$arg"
+		return
+		;;
+	'(i):'*)
+		# ':file' means cat from index
+		arg=$(echo "$arg" | tail --bytes=+5)
+		git cat-file blob ":$arg"
+		;;
+	*)
+		git cat-file blob "$arg"
+	esac
+}
+
 # setup_hook NAME
 setup_hook()
 {
-- 
tg: (a3a5be1..) t/tg-patch-worktree (depends on: t/tg-patch-setup-pager)
Kirill Smelkov· Jan 18, 2009, 15:06 UTC · re: Kirill Smelkov · lore
On Fri, Jan 09, 2009 at 12:11:49AM +0300, Kirill Smelkov wrote:
Show 38 quoted lines
> I'm sorry, but I've found a mistake in my code for case:
> 
> diff --git a/tg-patch.sh b/tg-patch.sh
> index db1ad09..d701c54 100644
> --- a/tg-patch.sh
> +++ b/tg-patch.sh
> @@ -17,8 +17,8 @@ while [ -n "$1" ]; do
>         case "$arg" in
>         -i)
>                 topic='(i)'
> -               diff_opts="$diff_opts --cached";;
> -               diff_committed_only=;
> +               diff_opts="$diff_opts --cached";
> +               diff_committed_only=;;
>         -w)
>                 topic='(w)'
>                 diff_committed_only=;;
> 
> 
> So here is corrected patch:
> 
> 
> From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> To: Petr Baudis <pasky@suse.cz>
> Cc: martin f krafft <madduck@madduck.net>
> Cc: Git Mailing List <git@vger.kernel.org>
> Subject: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index
> 
> This implements `tg patch -i` and `tg patch -w` to see current patch as
> generated against not-yet-committed index and worktree.
> 
> 
> NOTE: unfortunately `git cat-file blob <file>` does not provide an option
> to cat file from worktree (only from an object or from index), so I had to
> unroll my own `cat file topic:file` with special support for '(i)' and
> '(w)' topics.
> 
> Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
[...]
Petr, Martin,
What's the state of this patch?

I don't understand why this gets ignored. Maybe I do something wrong? ...

Thanks, Kirill

martin f krafft· Jan 20, 2009, 00:21 UTC · re: Kirill Smelkov · lore
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.19.0206 +1100]:
> I don't understand why this gets ignored. Maybe I do something wrong?

I am not ignoring it, I am just currently flooded and travelling. I don't see any obvious problems with your patch, but I do not have the time to test it right now. I will do my best to do another release when I return from Tasmania, hopefully on the plane trip home. Maybe Petr can get around to it before I do.

Cheers, and sorry,
-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
if you find a spelling mistake in the above, you get to keep it.
 
spamtraps: madduck.bogus@madduck.net
Kirill Smelkov· Jan 20, 2009, 17:56 UTC · re: martin f krafft · lore
On Tue, Jan 20, 2009 at 11:21:03AM +1100, martin f krafft wrote:
Show 10 quoted lines
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.19.0206 +1100]:
> > I don't understand why this gets ignored. Maybe I do something wrong?
> 
> I am not ignoring it, I am just currently flooded and travelling.
> I don't see any obvious problems with your patch, but I do not have
> the time to test it right now. I will do my best to do another
> release when I return from Tasmania, hopefully on the plane trip
> home. Maybe Petr can get around to it before I do.
> 
> Cheers, and sorry,
No problem and take your time.
It was just me having no idea about what's going on.

Happy travelling, Kirill

martin f krafft· Jan 21, 2009, 03:20 UTC · re: Kirill Smelkov · lore
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.21.0456 +1100]:
> No problem and take your time.
> 
> It was just me having no idea about what's going on.

I am a little confused about the patches, and git-am does not seem to be able to apply two patches from the same email. I use Maildir, so I am passing the single message with multiple patches to git-am, and it applies all changes into one single commit.

Anyway, do you have a repo which I can cherry-pick from?
-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"out of the crooked timber of humanity,
 no straight thing was ever made."
                                                       -- imanuel kant
 
spamtraps: madduck.bogus@madduck.net
Kirill Smelkov· Jan 21, 2009, 20:26 UTC · re: martin f krafft · lore
On Wed, Jan 21, 2009 at 02:20:52PM +1100, martin f krafft wrote:
Show 9 quoted lines
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.21.0456 +1100]:
> > No problem and take your time.
> > 
> > It was just me having no idea about what's going on.
> 
> I am a little confused about the patches, and git-am does not seem
> to be able to apply two patches from the same email. I use Maildir,
> so I am passing the single message with multiple patches to git-am,
> and it applies all changes into one single commit.
I see.

It seems I need to study how to prepare patches with patchy comments, or get my hands dirty with `git am` :)

> Anyway, do you have a repo which I can cherry-pick from?
Sure
    git://repo.or.cz/topgit/kirr.git  for-upstream

Thanks, Kirill

martin f krafft· Jan 22, 2009, 00:28 UTC · re: Kirill Smelkov · lore
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.22.0726 +1100]:
> It seems I need to study how to prepare patches with patchy
> comments, or get my hands dirty with `git am` :)

I believe git-send-email --editor can be used to create a lead-in mail to describe what follows, and send patches in separate messages in reply to that lead-in.

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"welcome to american airlines, sir. here's your avocado - remember to
 keep it turned on and with you at all times. please turn your luggage
 over to the armadillos for rootling."
                      -- http://azure.humbug.org.au/~aj/armadillos.txt
 
spamtraps: madduck.bogus@madduck.net

← back to recent threads