[PATCH v2 00/44] Ruby support

[PATCH v2 00/44] Ruby support

[Felipe Contreras]

This is a different patch series that tries a different approach.

First, this series allows external scripts to also access libgit through Ruby
bindings:

  git ruby <<EOF
  for_each_ref() do |name, sha1, flags|
    puts "%s: %s" % [name, sha1_to_hex(sha1)]
  end
  EOF

This way third party script writers can write Ruby code that resembles Git's
internal C code, but in a much simpler way.

Since the importance of the contrib area is fading, and it's important to move
all Git commands to C, it's helpful to have tools that allows third parties to
rewrite scripts that are easier to rewrite to C.

In addition, this would help Git developers to create quick prototypes and test
new code.

Finally, and perhaps most importantly, Ruby scripts can be intermediaries
between perl/shell scripts, and the final desired C code.

I already argued this, but here is a clear demonstration.

I took git-request-pull.sh, converted it to Ruby and incrementally moodified it
until the resulting code was essentially what the code would look like in C,
and then, finally, the rewrite to C, which follows the exact same logic of the
Ruby script.

In the progress of doing this, it was clear the shell script was buggy, so it
was fixed, and the C code needed fixes as well.

If we are serious about converting *all scripts* to C, clearly a powerful tool
is needed, and Ruby bindings fit the bill.

Ruby scripts could be provided alongside the perl/shell alternatives, so people
that build with NO_RUBY=y don't loose any functionality. And eventually the
Ruby scripts (and originals) get removed once the conversion to C is completed.

Even if Ruby is not used as an intermediary tool, and no official Git scripts are
converted to C this way, the bindings would still be useful to third parties.

Who wouldn't benefit from adding this support?

Felipe Contreras (44):
  Add support for ruby commands
  ruby: add support for internal ruby programs
  request-pull: fix annotated tag check
  request-pull: fix for specific branch
  request-pull: use appropriate ref
  request-pull: fix exact match checking
  request-pull: trivial simplification
  request-pull: t: trivial whitespace style fixes
  request-pull: t: add missing cat
  ruby: rewrite 'request-pull'
  ruby: request-pull: rewrite perl script
  ruby: request-pull: trivial simplifications
  ruby: bind setup_git_directory()
  ruby: bind dwim_ref()
  ruby: request-pull: use native dwim_ref()
  ruby: bind git_config()
  ruby: request-pull: use native git_config()
  ruby: bind read_ref()/peel_ref()
  ruby: bind get_sha1()
  ruby: request-pull: simplify tag fetching
  ruby: request-pull: use get_sha1()
  ruby: add Commit class
  ruby: bind get_merge_bases()
  ruby: request-pull: use get_merge_bases()
  ruby: request-pull: trivial cleanups
  ruby: bind remote and transport stuff
  ruby: request-pull: use native remote and transport
  ruby: bind find_unique_abbrev()
  ruby: request-pull: use native commit info
  ruby: bind read_sha1_file()
  ruby: request-pull: use read_sha1_file()
  revision: add missing include
  shortlog: add missing declaration
  shortlog: split builtin from common code
  ruby: add RevInfo and DiffOptions
  ruby: bind shortlog()
  ruby: request-pull: use shortlog()
  ruby: bind diff_tree_sha1()
  ruby: bind log_tree_diff_flush()
  ruby: request-pull: use native diff_tree stuff
  ruby: request-pull: remove rescue block
  ruby: remove GIT_PAGER from environment
  ruby: add simpler option parser
  request-pull: rewrite to C

 .gitignore              |   1 +
 Makefile                |  31 ++-
 builtin.h               |   1 +
 builtin/request-pull.c  | 275 +++++++++++++++++++++++
 builtin/shortlog.c      | 184 +--------------
 git-rb-setup.rb         | 109 +++++++++
 git-request-pull.sh     | 162 --------------
 git.c                   |   1 +
 revision.h              |   1 +
 ruby.c                  | 584 ++++++++++++++++++++++++++++++++++++++++++++++++
 shortlog.c              | 181 +++++++++++++++
 shortlog.h              |   8 +
 t/t10000-ruby.sh        | 233 +++++++++++++++++++
 t/t5150-request-pull.sh |  81 +++++--
 14 files changed, 1492 insertions(+), 360 deletions(-)
 create mode 100644 builtin/request-pull.c
 create mode 100644 git-rb-setup.rb
 delete mode 100755 git-request-pull.sh
 create mode 100644 ruby.c
 create mode 100644 shortlog.c
 create mode 100755 t/t10000-ruby.sh

-- 
1.8.4-fc

[PATCH v2 01/44] Add support for ruby commands

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    With this support, third parties would be able to write scripts more easily and
    access libgit facilities.
    
    Also, it could help developers by allowing easier prototyping
    
      git ruby > actual <<EOF &&
      for_each_ref() do |name, sha1, flags|
        puts "%s: %s" % [name, sha1_to_hex(sha1)]
      end

 .gitignore       |  1 +
 Makefile         | 19 ++++++++++++++++++-
 git-rb-setup.rb  | 11 +++++++++++
 ruby.c           | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t10000-ruby.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 git-rb-setup.rb
 create mode 100644 ruby.c
 create mode 100755 t/t10000-ruby.sh

diff --git a/.gitignore b/.gitignore
index 6b1fd1b..51b04be 100644
--- a/.gitignore
+++ b/.gitignore
@@ -139,6 +139,7 @@
 /git-rev-parse
 /git-revert
 /git-rm
+/git-ruby
 /git-send-email
 /git-send-pack
 /git-sh-i18n
diff --git a/Makefile b/Makefile
index 3588ca1..2fdee15 100644
--- a/Makefile
+++ b/Makefile
@@ -491,6 +491,8 @@ SCRIPT_PERL += git-svn.perl
 SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
+SCRIPT_RUBY += git-rb-setup.rb
+
 NO_INSTALL += git-remote-testgit
 NO_INSTALL += git-remote-testpy
 
@@ -502,6 +504,7 @@ SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON))
 SCRIPT_SH_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_SH_GEN))
 SCRIPT_PERL_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PERL_GEN))
 SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN))
+SCRIPT_RUBY_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_RUBY))
 
 # Individual rules to allow e.g.
 # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script"
@@ -511,13 +514,15 @@ build-perl-script: $(SCRIPT_PERL_GEN)
 build-sh-script: $(SCRIPT_SH_GEN)
 build-python-script: $(SCRIPT_PYTHON_GEN)
 
-.PHONY: install-perl-script install-sh-script install-python-script
+.PHONY: install-perl-script install-sh-script install-python-script install-ruby-script
 install-sh-script: $(SCRIPT_SH_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 install-perl-script: $(SCRIPT_PERL_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 install-python-script: $(SCRIPT_PYTHON_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+install-ruby-script: $(SCRIPT_RUBY_INS)
+	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
 .PHONY: clean-perl-script clean-sh-script clean-python-script
 clean-sh-script:
@@ -530,6 +535,7 @@ clean-python-script:
 SCRIPTS = $(SCRIPT_SH_INS) \
 	  $(SCRIPT_PERL_INS) \
 	  $(SCRIPT_PYTHON_INS) \
+	  $(SCRIPT_RUBY_INS) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS
@@ -1502,6 +1508,12 @@ ifneq (,$(XDL_FAST_HASH))
 	BASIC_CFLAGS += -DXDL_FAST_HASH
 endif
 
+ifndef NO_RUBY
+	RUBY_LIBS = $(shell pkg-config --libs ruby-2.0)
+	RUBY_CFLAGS = $(shell pkg-config --cflags ruby-2.0)
+	PROGRAM_OBJS += ruby.o
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
@@ -2059,6 +2071,11 @@ git-http-push$X: revision.o http.o http-push.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+git-ruby$X: BASIC_CFLAGS += $(RUBY_CFLAGS)
+git-ruby$X: ruby.o GIT-LDFLAGS $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) $(RUBY_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+		$(LIBS) $(RUBY_LIBS)
+
 git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
 	$(VCSSVN_LIB)
diff --git a/git-rb-setup.rb b/git-rb-setup.rb
new file mode 100644
index 0000000..969278a
--- /dev/null
+++ b/git-rb-setup.rb
@@ -0,0 +1,11 @@
+#!/usr/bin/env ruby
+
+def die(*args)
+  fmt = args.shift
+  $stderr.printf("fatal: %s\n" % fmt, *args)
+  exit 128
+end
+
+def sha1_to_hex(sha1)
+  sha1.unpack('H*').first
+end
diff --git a/ruby.c b/ruby.c
new file mode 100644
index 0000000..ee6a0e7
--- /dev/null
+++ b/ruby.c
@@ -0,0 +1,56 @@
+#include "cache.h"
+#include "exec_cmd.h"
+#include "refs.h"
+
+#undef NORETURN
+#undef PATH_SEP
+
+#include <ruby.h>
+
+static inline VALUE sha1_to_str(const unsigned char *sha1)
+{
+	return rb_str_new((const char *)sha1, 20);
+}
+
+static int for_each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
+{
+	VALUE r;
+	r = rb_yield_values(3, rb_str_new2(refname), sha1_to_str(sha1), INT2FIX(flags));
+	return r == Qfalse;
+}
+
+static VALUE git_rb_for_each_ref(void)
+{
+	int r;
+	r = for_each_ref(for_each_ref_fn, NULL);
+	return INT2FIX(r);
+}
+
+static void git_ruby_init(void)
+{
+	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
+}
+
+static int run_ruby_command(const char *cmd, int argc, const char **argv)
+{
+	static char buf[PATH_MAX + 1];
+	void *node;
+	struct stat st;
+
+	ruby_init();
+	git_ruby_init();
+
+	node = ruby_options(argc, (char **)argv);
+
+	ruby_script(cmd);
+	snprintf(buf, PATH_MAX, "%s/%s", git_exec_path(), "git-rb-setup.rb");
+	if (!stat(buf, &st))
+		rb_load(rb_str_new2(buf), 0);
+
+	return ruby_run_node(node);
+}
+
+int main(int argc, const char **argv)
+{
+	return run_ruby_command(argv[1], argc, argv);
+}
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
new file mode 100755
index 0000000..eb03706
--- /dev/null
+++ b/t/t10000-ruby.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Felipe Contreras
+#
+
+test_description='test ruby support'
+
+. ./test-lib.sh
+
+test_expect_success 'basic support' '
+	git ruby > actual <<-EOF &&
+	puts "hello world"
+	EOF
+	echo "hello world" > expected &&
+	test_cmp expected actual
+'
+
+test_expect_success 'argument passing' '
+	cat > script <<-"EOF" &&
+	p($0)
+	p(ARGV)
+	EOF
+	git ruby script foo bar > actual &&
+	cat > expected <<-EOF &&
+	"script"
+	["foo", "bar"]
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'test for_each_ref()' '
+	test_commit foo &&
+	git ruby > actual <<-EOF &&
+	for_each_ref() do |name, sha1, flags|
+		puts "%s: %s" % [name, sha1_to_hex(sha1)]
+	end
+	EOF
+	git for-each-ref --format="%(refname): %(objectname)" > expected &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.8.4-fc

[PATCH v2 02/44] ruby: add support for internal ruby programs

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile |  9 +++++++++
 ruby.c   | 19 ++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 2fdee15..27e61c0 100644
--- a/Makefile
+++ b/Makefile
@@ -429,6 +429,7 @@ BASIC_LDFLAGS =
 # Guard against environment variables
 BUILTIN_OBJS =
 BUILT_INS =
+RUBY_PROGRAMS =
 COMPAT_CFLAGS =
 COMPAT_OBJS =
 XDIFF_OBJS =
@@ -493,6 +494,8 @@ SCRIPT_PYTHON += git-p4.py
 
 SCRIPT_RUBY += git-rb-setup.rb
 
+PROGRAMS += $(RUBY_PROGRAMS)
+
 NO_INSTALL += git-remote-testgit
 NO_INSTALL += git-remote-testpy
 
@@ -1754,6 +1757,12 @@ $(BUILT_INS): git$X
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@
 
+$(RUBY_PROGRAMS): git-ruby$X
+	$(QUIET_BUILT_IN)$(RM) $@ && \
+	ln $< $@ 2>/dev/null || \
+	ln -s $< $@ 2>/dev/null || \
+	cp $< $@
+
 common-cmds.h: ./generate-cmdlist.sh command-list.txt
 
 common-cmds.h: $(wildcard Documentation/git-*.txt)
diff --git a/ruby.c b/ruby.c
index ee6a0e7..339e376 100644
--- a/ruby.c
+++ b/ruby.c
@@ -52,5 +52,22 @@ static int run_ruby_command(const char *cmd, int argc, const char **argv)
 
 int main(int argc, const char **argv)
 {
-	return run_ruby_command(argv[1], argc, argv);
+	if (!strcmp(argv[0], "git-ruby")) {
+		return run_ruby_command(argv[1], argc, argv);
+	} else {
+		const char *cmd = argv[0];
+		static char buf[PATH_MAX + 1];
+		const char *args[argc + 1];
+		int i;
+
+		snprintf(buf, PATH_MAX, "%s/%s.rb",
+				git_exec_path(), basename((char *)cmd));
+
+		args[0] = "git";
+		args[1] = buf;
+		for (i = 0; i < argc - 1; i++)
+			args[i + 2] = (char *)argv[i + 1];
+
+		return run_ruby_command(cmd, argc + 1, args);
+	}
 }
-- 
1.8.4-fc

[PATCH v2 03/44] request-pull: fix annotated tag check

[Felipe Contreras]

The was just not doing anything, reporting an appropriately tagged ref
worked only by luck.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.sh     |  2 +-
 t/t5150-request-pull.sh | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index ebf1269..6348dac 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -89,7 +89,7 @@ find_matching_ref='
 		my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
 		next unless ($sha1 eq $ARGV[1]);
 		$found = abbr($ref);
-		if ($deref && $ref eq "tags/$ARGV[2]") {
+		if ($deref && $ref eq "refs/tags/$ARGV[2]") {
 			$tagged = $found;
 			last;
 		}
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 1afa0d5..54f41bf 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -234,4 +234,19 @@ test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
 
 '
 
+test_expect_success 'pull request when pushed tag' '
+	rm -fr downstream.git &&
+	git init --bare downstream.git &&
+	(
+		cd local &&
+		git checkout initial &&
+		git merge --ff-only master &&
+		git tag zeebra &&
+		git push origin master:for-upstream full zeebra &&
+		git request-pull initial origin 2>../err
+	) &&
+	cat err &&
+	! grep "You locally have .* but it does not (yet)" err
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 04/44] request-pull: fix for specific branch

[Felipe Contreras]

Wether we use a symbolic ref, or we specify the branch directly
shouldn't really matter, we would want the branch description either
way.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.sh     | 13 +++++--------
 t/t5150-request-pull.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 6348dac..3c9a982 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -37,15 +37,12 @@ done
 
 base=$1 url=$2 head=${3-HEAD} status=0 branch_name=
 
-headref=$(git symbolic-ref -q "$head")
-if git show-ref -q --verify "$headref"
+headref=$(git rev-parse -q --verify --symbolic-full-name "$head")
+branch_name=${headref#refs/heads/}
+if test "z$branch_name" = "z$headref" ||
+	! git config "branch.$branch_name.description" >/dev/null
 then
-	branch_name=${headref#refs/heads/}
-	if test "z$branch_name" = "z$headref" ||
-		! git config "branch.$branch_name.description" >/dev/null
-	then
-		branch_name=
-	fi
+	branch_name=
 fi
 
 tag_name=$(git describe --exact "$head^0" 2>/dev/null)
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 54f41bf..a9ee96a 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -249,4 +249,36 @@ test_expect_success 'pull request when pushed tag' '
 	! grep "You locally have .* but it does not (yet)" err
 '
 
+test_expect_success 'pull request with branch description' '
+	test_when_finished "(cd local && git checkout - && git branch -D for-upstream)" &&
+	rm -fr downstream.git &&
+	git init --bare downstream.git &&
+	(
+		cd local &&
+		git checkout -b for-upstream master &&
+		git config branch.for-upstream.description "Branch for upstream$LF" &&
+		git push origin for-upstream &&
+		git request-pull initial origin >../request
+	) &&
+	cat request &&
+	grep "(from the branch description for for-upstream local branch)" request &&
+	grep "Branch for upstream" request
+'
+
+test_expect_success 'pull request with branch description from rev' '
+	test_when_finished "(cd local && git checkout - && git branch -D for-upstream)" &&
+	rm -fr downstream.git &&
+	git init --bare downstream.git &&
+	(
+		cd local &&
+		git checkout -b for-upstream master &&
+		git config branch.for-upstream.description "Branch for upstream$LF" &&
+		git push origin for-upstream &&
+		git request-pull initial origin for-upstream >../request
+	) &&
+	cat request &&
+	grep "(from the branch description for for-upstream local branch)" request &&
+	grep "Branch for upstream" request
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 05/44] request-pull: use appropriate ref

[Felipe Contreras]

"Are you sure you pushed 'HEAD'?" doesn't say anything helpful.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 3c9a982..3e510ca 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -151,9 +151,10 @@ git diff -M --stat --summary $patch $merge_base..$headrev || status=1
 
 if test -z "$ref"
 then
+	short_headref=$(git rev-parse -q --verify --symbolic-full-name --abbrev-ref "$head")
 	echo "warn: No branch of $url is at:" >&2
 	git show -s --format='warn:   %h: %s' $headrev >&2
-	echo "warn: Are you sure you pushed '$head' there?" >&2
+	echo "warn: Are you sure you pushed '$short_headref' there?" >&2
 	status=1
 fi
 exit $status
-- 
1.8.4-fc

[PATCH v2 06/44] request-pull: fix exact match checking

[Felipe Contreras]

It obviously never worked, refs don't start with '/'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.sh     | 10 ++++++++--
 t/t5150-request-pull.sh | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 3e510ca..c970244 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -38,6 +38,11 @@ done
 base=$1 url=$2 head=${3-HEAD} status=0 branch_name=
 
 headref=$(git rev-parse -q --verify --symbolic-full-name "$head")
+if test "$headref" = "HEAD"
+then
+	headref=
+fi
+
 branch_name=${headref#refs/heads/}
 if test "z$branch_name" = "z$headref" ||
 	! git config "branch.$branch_name.description" >/dev/null
@@ -90,8 +95,9 @@ find_matching_ref='
 			$tagged = $found;
 			last;
 		}
-		if ($ref =~ m|/\Q$ARGV[0]\E$|) {
+		if ($ref eq $ARGV[0]) {
 			$exact = $found;
+			last;
 		}
 	}
 	if ($tagged) {
@@ -103,7 +109,7 @@ find_matching_ref='
 	}
 '
 
-ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$head" "$headrev" "$tag_name")
+ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$headref" "$headrev" "$tag_name")
 
 url=$(git ls-remote --get-url "$url")
 
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index a9ee96a..73f0369 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -281,4 +281,25 @@ test_expect_success 'pull request with branch description from rev' '
 	grep "Branch for upstream" request
 '
 
+test_expect_success 'pull request with exact match' '
+	test_when_finished "(cd local && git checkout - && git branch -D for-upstream)" &&
+	rm -fr downstream.git &&
+	git init --bare downstream.git &&
+	(
+		cd local &&
+		git checkout -b for-upstream master &&
+		git push origin master:for-upstream master:zeebra &&
+		git request-pull initial origin for-upstream >../request
+	) &&
+	cat request &&
+	sed -nf read-request.sed <request >digest &&
+	cat digest &&
+	{
+		read task &&
+		read repository &&
+		read branch
+	} <digest &&
+	test "$branch" = for-upstream
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 07/44] request-pull: trivial simplification

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index c970244..d8b4c3c 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -86,25 +86,19 @@ find_matching_ref='
 		}
 	}
 
-	my ($tagged, $branch, $found);
+	my ($found);
 	while (<STDIN>) {
 		my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
 		next unless ($sha1 eq $ARGV[1]);
 		$found = abbr($ref);
 		if ($deref && $ref eq "refs/tags/$ARGV[2]") {
-			$tagged = $found;
 			last;
 		}
 		if ($ref eq $ARGV[0]) {
-			$exact = $found;
 			last;
 		}
 	}
-	if ($tagged) {
-		print "$tagged\n";
-	} elsif ($exact) {
-		print "$exact\n";
-	} elsif ($found) {
+	if ($found) {
 		print "$found\n";
 	}
 '
-- 
1.8.4-fc

[PATCH v2 08/44] request-pull: t: trivial whitespace style fixes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5150-request-pull.sh | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 73f0369..9375912 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -5,7 +5,6 @@ test_description='Test workflows involving pull request.'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-
 	git init --bare upstream.git &&
 	git init --bare downstream.git &&
 	git clone upstream.git upstream-private &&
@@ -55,11 +54,9 @@ test_expect_success 'setup' '
 But keep the old version, too, in case some people prefer it." &&
 		git checkout master
 	)
-
 '
 
 test_expect_success 'setup: two scripts for reading pull requests' '
-
 	downstream_url_for_sed=$(
 		printf "%s\n" "$downstream_url" |
 		sed -e '\''s/\\/\\\\/g'\'' -e '\''s/[[/.*^$]/\\&/g'\''
@@ -113,11 +110,9 @@ test_expect_success 'setup: two scripts for reading pull requests' '
 	n
 	b shortlog
 	EOT
-
 '
 
 test_expect_success 'pull request when forgot to push' '
-
 	rm -fr downstream.git &&
 	git init --bare downstream.git &&
 	(
@@ -129,11 +124,9 @@ test_expect_success 'pull request when forgot to push' '
 	) &&
 	grep "No branch of.*is at:\$" err &&
 	grep "Are you sure you pushed" err
-
 '
 
 test_expect_success 'pull request after push' '
-
 	rm -fr downstream.git &&
 	git init --bare downstream.git &&
 	(
@@ -157,11 +150,9 @@ test_expect_success 'pull request after push' '
 	) &&
 	test "$branch" = for-upstream &&
 	test_cmp local/mnemonic.txt upstream-private/mnemonic.txt
-
 '
 
 test_expect_success 'request names an appropriate branch' '
-
 	rm -fr downstream.git &&
 	git init --bare downstream.git &&
 	(
@@ -180,11 +171,9 @@ test_expect_success 'request names an appropriate branch' '
 		read branch
 	} <digest &&
 	test "$branch" = tags/full
-
 '
 
 test_expect_success 'pull request format' '
-
 	rm -fr downstream.git &&
 	git init --bare downstream.git &&
 	cat <<-\EOT >expect &&
@@ -217,11 +206,9 @@ test_expect_success 'pull request format' '
 	) &&
 	<request sed -nf fuzz.sed >request.fuzzy &&
 	test_i18ncmp expect request.fuzzy
-
 '
 
 test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
-
 	(
 		cd local &&
 		OPTIONS_KEEPDASHDASH=Yes &&
@@ -231,7 +218,6 @@ test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
 		git push origin master:for-upstream &&
 		git request-pull -- initial "$downstream_url" >../request
 	)
-
 '
 
 test_expect_success 'pull request when pushed tag' '
-- 
1.8.4-fc

[PATCH v2 09/44] request-pull: t: add missing cat

[Felipe Contreras]

Otherwise we wouldn't know why it failed.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5150-request-pull.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9375912..60a38be 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -122,6 +122,7 @@ test_expect_success 'pull request when forgot to push' '
 		test_must_fail git request-pull initial "$downstream_url" \
 			2>../err
 	) &&
+	cat err &&
 	grep "No branch of.*is at:\$" err &&
 	grep "Are you sure you pushed" err
 '
-- 
1.8.4-fc

[PATCH v2 10/44] ruby: rewrite 'request-pull'

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    The objective is to be as close to the original as possible, to minimize
    regressions, even if the style is not very Ruby-ish.

 Makefile            |   3 +-
 git-rb-setup.rb     |  17 ++++++
 git-request-pull.rb | 146 +++++++++++++++++++++++++++++++++++++++++++++++
 git-request-pull.sh | 160 ----------------------------------------------------
 4 files changed, 165 insertions(+), 161 deletions(-)
 create mode 100755 git-request-pull.rb
 delete mode 100755 git-request-pull.sh

diff --git a/Makefile b/Makefile
index 27e61c0..44b4cd5 100644
--- a/Makefile
+++ b/Makefile
@@ -466,7 +466,6 @@ SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-remote-testgit.sh
 SCRIPT_SH += git-repack.sh
-SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
@@ -493,7 +492,9 @@ SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
 SCRIPT_RUBY += git-rb-setup.rb
+SCRIPT_RUBY += git-request-pull.rb
 
+RUBY_PROGRAMS += git-request-pull$X
 PROGRAMS += $(RUBY_PROGRAMS)
 
 NO_INSTALL += git-remote-testgit
diff --git a/git-rb-setup.rb b/git-rb-setup.rb
index 969278a..92998a2 100644
--- a/git-rb-setup.rb
+++ b/git-rb-setup.rb
@@ -9,3 +9,20 @@ end
 def sha1_to_hex(sha1)
   sha1.unpack('H*').first
 end
+
+class CommandError < RuntimeError
+
+  def initialize(command)
+     @command = command
+  end
+
+  def to_s
+    Array(@command).join(' ').inspect
+  end
+
+end
+
+def run(cmd, *args)
+  system(*cmd, *args)
+  raise CommandError.new(cmd) unless $?.success?
+end
diff --git a/git-request-pull.rb b/git-request-pull.rb
new file mode 100755
index 0000000..60cf6ce
--- /dev/null
+++ b/git-request-pull.rb
@@ -0,0 +1,146 @@
+#!git ruby
+
+ENV['GIT_PAGER'] =
+
+patch = ''
+
+def usage
+  puts <<EOF
+usage: git request-pull [options] start url [end]
+
+    -p                    show patch text as well
+
+EOF
+  exit 1
+end
+
+until ARGV.empty?
+  case ARGV.first
+  when '-p'
+    patch = '-p'
+  when '--'
+    ARGV.shift
+    break
+  when /^-/
+    usage
+  else
+    break
+  end
+  ARGV.shift
+end
+
+base = ARGV[0]
+url = ARGV[1]
+head = ARGV[2] || 'HEAD'
+status = 0
+branch_name = nil
+
+headref = `git rev-parse -q --verify --symbolic-full-name "#{head}"`.chomp
+headref = "" if headref == "HEAD"
+
+branch_name = headref.gsub(%r{^refs/heads/}, '')
+if branch_name == headref ||
+  ! system(%[git config "branch.#{branch_name}.description" >/dev/null])
+  branch_name = nil
+end
+
+tag_name = `git describe --exact "#{head}^0" 2>/dev/null`.chomp
+
+usage unless base or url
+
+baserev = `git rev-parse --verify --quiet "#{base}"^0`.chomp
+die "Not a valid revision: #{base}" if baserev.empty?
+
+headrev = `git rev-parse --verify --quiet "#{head}"^0`.chomp
+die "Not a valid revision: #{head}" if headrev.empty?
+
+merge_base = `git merge-base #{baserev} #{headrev}`.chomp
+die "No commits in common between #{base} and #{head}" unless $?.success?
+
+# $head is the token given from the command line, and $tag_name, if
+# exists, is the tag we are going to show the commit information for.
+# If that tag exists at the remote and it points at the commit, use it.
+# Otherwise, if a branch with the same name as $head exists at the remote
+# and their values match, use that instead.
+#
+# Otherwise find a random ref that matches $headrev.
+find_matching_ref='
+	sub abbr {
+		my $ref = shift;
+		if ($ref =~ s|^refs/heads/|| || $ref =~ s|^refs/tags/|tags/|) {
+			return $ref;
+		} else {
+			return $ref;
+		}
+	}
+
+	my ($found);
+	while (<STDIN>) {
+		my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
+		next unless ($sha1 eq $ARGV[1]);
+		$found = abbr($ref);
+		if ($deref && $ref eq "refs/tags/$ARGV[2]") {
+			last;
+		}
+		if ($ref eq $ARGV[0]) {
+			last;
+		}
+	}
+	if ($found) {
+		print "$found\n";
+	}
+'
+
+ref = `git ls-remote "#{url}" | perl -e '#{find_matching_ref}' "#{headref}" "#{headrev}" "#{tag_name}"`.chomp
+url = `git ls-remote --get-url "#{url}"`.chomp
+
+begin
+  run(%[git show -s --format='The following changes since commit %H:
+
+  %s (%ci)
+
+are available in the git repository at:
+' #{merge_base}])
+  puts "  #{url}" + (ref.empty? ? "" : " #{ref}")
+  run(%[git show -s --format='
+for you to fetch changes up to %H:
+
+  %s (%ci)
+
+----------------------------------------------------------------' #{headrev}])
+
+  if branch_name
+    puts "(from the branch description for #{branch_name} local branch)"
+    puts
+    run(%[git config "branch.#{branch_name}.description"])
+  end
+
+  if not tag_name.empty?
+    if ref.empty? || ref != "tags/#{tag_name}"
+      $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
+      $stderr.puts "warn: appear to be at #{url}"
+      $stderr.puts "warn: Do you want to push it there, perhaps?"
+    end
+    run(%[git cat-file tag "#{tag_name}" | sed -n -e '1,/^$/d' -e '/^-----BEGIN PGP /q' -e p])
+    puts
+  end
+
+  if branch_name || ! tag_name.empty?
+    puts "----------------------------------------------------------------"
+  end
+
+  run(%[git shortlog ^#{baserev} #{headrev}])
+  run(%[git diff -M --stat --summary #{patch} #{merge_base}..#{headrev}])
+
+  if ref.empty?
+    short_headref = `git rev-parse -q --verify --symbolic-full-name --abbrev-ref "#{head}"`.chomp
+    $stderr.puts "warn: No branch of #{url} is at:"
+    run("git show -s --format='warn:   %h: %s' #{headrev} >&2")
+    $stderr.puts "warn: Are you sure you pushed '#{short_headref}' there?"
+    status = 1
+  end
+rescue CommandError
+  status = 1
+end
+
+exit status
diff --git a/git-request-pull.sh b/git-request-pull.sh
deleted file mode 100755
index d8b4c3c..0000000
--- a/git-request-pull.sh
+++ /dev/null
@@ -1,160 +0,0 @@
-#!/bin/sh
-# Copyright 2005, Ryan Anderson <ryan@michonline.com>
-#
-# This file is licensed under the GPL v2, or a later version
-# at the discretion of Linus Torvalds.
-
-USAGE='<start> <url> [<end>]'
-LONG_USAGE='Summarizes the changes between two commits to the standard output,
-and includes the given URL in the generated summary.'
-SUBDIRECTORY_OK='Yes'
-OPTIONS_KEEPDASHDASH=
-OPTIONS_SPEC='git request-pull [options] start url [end]
---
-p    show patch text as well
-'
-
-. git-sh-setup
-
-GIT_PAGER=
-export GIT_PAGER
-
-patch=
-while	case "$#" in 0) break ;; esac
-do
-	case "$1" in
-	-p)
-		patch=-p ;;
-	--)
-		shift; break ;;
-	-*)
-		usage ;;
-	*)
-		break ;;
-	esac
-	shift
-done
-
-base=$1 url=$2 head=${3-HEAD} status=0 branch_name=
-
-headref=$(git rev-parse -q --verify --symbolic-full-name "$head")
-if test "$headref" = "HEAD"
-then
-	headref=
-fi
-
-branch_name=${headref#refs/heads/}
-if test "z$branch_name" = "z$headref" ||
-	! git config "branch.$branch_name.description" >/dev/null
-then
-	branch_name=
-fi
-
-tag_name=$(git describe --exact "$head^0" 2>/dev/null)
-
-test -n "$base" && test -n "$url" || usage
-
-baserev=$(git rev-parse --verify --quiet "$base"^0)
-if test -z "$baserev"
-then
-    die "fatal: Not a valid revision: $base"
-fi
-
-headrev=$(git rev-parse --verify --quiet "$head"^0)
-if test -z "$headrev"
-then
-    die "fatal: Not a valid revision: $head"
-fi
-
-merge_base=$(git merge-base $baserev $headrev) ||
-die "fatal: No commits in common between $base and $head"
-
-# $head is the token given from the command line, and $tag_name, if
-# exists, is the tag we are going to show the commit information for.
-# If that tag exists at the remote and it points at the commit, use it.
-# Otherwise, if a branch with the same name as $head exists at the remote
-# and their values match, use that instead.
-#
-# Otherwise find a random ref that matches $headrev.
-find_matching_ref='
-	sub abbr {
-		my $ref = shift;
-		if ($ref =~ s|^refs/heads/|| || $ref =~ s|^refs/tags/|tags/|) {
-			return $ref;
-		} else {
-			return $ref;
-		}
-	}
-
-	my ($found);
-	while (<STDIN>) {
-		my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
-		next unless ($sha1 eq $ARGV[1]);
-		$found = abbr($ref);
-		if ($deref && $ref eq "refs/tags/$ARGV[2]") {
-			last;
-		}
-		if ($ref eq $ARGV[0]) {
-			last;
-		}
-	}
-	if ($found) {
-		print "$found\n";
-	}
-'
-
-ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$headref" "$headrev" "$tag_name")
-
-url=$(git ls-remote --get-url "$url")
-
-git show -s --format='The following changes since commit %H:
-
-  %s (%ci)
-
-are available in the git repository at:
-' $merge_base &&
-echo "  $url${ref+ $ref}" &&
-git show -s --format='
-for you to fetch changes up to %H:
-
-  %s (%ci)
-
-----------------------------------------------------------------' $headrev &&
-
-if test -n "$branch_name"
-then
-	echo "(from the branch description for $branch_name local branch)"
-	echo
-	git config "branch.$branch_name.description"
-fi &&
-
-if test -n "$tag_name"
-then
-	if test -z "$ref" || test "$ref" != "tags/$tag_name"
-	then
-		echo >&2 "warn: You locally have $tag_name but it does not (yet)"
-		echo >&2 "warn: appear to be at $url"
-		echo >&2 "warn: Do you want to push it there, perhaps?"
-	fi
-	git cat-file tag "$tag_name" |
-	sed -n -e '1,/^$/d' -e '/^-----BEGIN PGP /q' -e p
-	echo
-fi &&
-
-if test -n "$branch_name" || test -n "$tag_name"
-then
-	echo "----------------------------------------------------------------"
-fi &&
-
-git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || status=1
-
-if test -z "$ref"
-then
-	short_headref=$(git rev-parse -q --verify --symbolic-full-name --abbrev-ref "$head")
-	echo "warn: No branch of $url is at:" >&2
-	git show -s --format='warn:   %h: %s' $headrev >&2
-	echo "warn: Are you sure you pushed '$short_headref' there?" >&2
-	status=1
-fi
-exit $status
-- 
1.8.4-fc

[PATCH v2 11/44] ruby: request-pull: rewrite perl script

[Felipe Contreras]

Ruby can do it just fine, no need for perl.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    I'm mostly ignoring the comment, my hope is that it would be redundant, as the
    code would be self-documenting.

 git-request-pull.rb | 65 +++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 35 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 60cf6ce..eee2e28 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -14,6 +14,35 @@ EOF
   exit 1
 end
 
+def abbr(ref)
+  if (ref =~ %r{^refs/heads/(.*)} || ref =~ %r{^refs/(tags/.*)})
+    return $1
+  end
+  return ref
+end
+
+# $head is the token given from the command line, and $tag_name, if
+# exists, is the tag we are going to show the commit information for.
+# If that tag exists at the remote and it points at the commit, use it.
+# Otherwise, if a branch with the same name as $head exists at the remote
+# and their values match, use that instead.
+#
+# Otherwise find a random ref that matches $headrev.
+
+def get_ref(url, headref, headrev, tag_name)
+  found = nil
+  IO.popen(%[git ls-remote "#{url}"]) do |out|
+    out.each do |l|
+      sha1, ref, deref = l.scan(/^(\S+)\s+(\S+?)(\^\{\})?$/).first
+      next unless sha1 == headrev
+      found = abbr(ref)
+      break if (deref && ref == "refs/tags/#{tag_name}")
+      break if ref == headref
+    end
+  end
+  return found
+end
+
 until ARGV.empty?
   case ARGV.first
   when '-p'
@@ -57,41 +86,7 @@ die "Not a valid revision: #{head}" if headrev.empty?
 merge_base = `git merge-base #{baserev} #{headrev}`.chomp
 die "No commits in common between #{base} and #{head}" unless $?.success?
 
-# $head is the token given from the command line, and $tag_name, if
-# exists, is the tag we are going to show the commit information for.
-# If that tag exists at the remote and it points at the commit, use it.
-# Otherwise, if a branch with the same name as $head exists at the remote
-# and their values match, use that instead.
-#
-# Otherwise find a random ref that matches $headrev.
-find_matching_ref='
-	sub abbr {
-		my $ref = shift;
-		if ($ref =~ s|^refs/heads/|| || $ref =~ s|^refs/tags/|tags/|) {
-			return $ref;
-		} else {
-			return $ref;
-		}
-	}
-
-	my ($found);
-	while (<STDIN>) {
-		my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
-		next unless ($sha1 eq $ARGV[1]);
-		$found = abbr($ref);
-		if ($deref && $ref eq "refs/tags/$ARGV[2]") {
-			last;
-		}
-		if ($ref eq $ARGV[0]) {
-			last;
-		}
-	}
-	if ($found) {
-		print "$found\n";
-	}
-'
-
-ref = `git ls-remote "#{url}" | perl -e '#{find_matching_ref}' "#{headref}" "#{headrev}" "#{tag_name}"`.chomp
+ref = get_ref(url, headref, headrev, tag_name) || ''
 url = `git ls-remote --get-url "#{url}"`.chomp
 
 begin
-- 
1.8.4-fc

[PATCH v2 12/44] ruby: request-pull: trivial simplifications

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Let's start deviating from the original script.
    
    There are no functional changes though.

 git-request-pull.rb | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index eee2e28..9d3e9ab 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -62,21 +62,20 @@ base = ARGV[0]
 url = ARGV[1]
 head = ARGV[2] || 'HEAD'
 status = 0
-branch_name = nil
+branch_name = branch_desc = nil
+
+usage unless base or url
 
 headref = `git rev-parse -q --verify --symbolic-full-name "#{head}"`.chomp
-headref = "" if headref == "HEAD"
 
-branch_name = headref.gsub(%r{^refs/heads/}, '')
-if branch_name == headref ||
-  ! system(%[git config "branch.#{branch_name}.description" >/dev/null])
-  branch_name = nil
+if headref.start_with?('refs/heads')
+  branch_name = headref[11..-1]
+  branch_desc = `git config "branch.#{branch_name}.description"`.chomp
+  branch_name = nil if branch_desc.empty?
 end
 
 tag_name = `git describe --exact "#{head}^0" 2>/dev/null`.chomp
 
-usage unless base or url
-
 baserev = `git rev-parse --verify --quiet "#{base}"^0`.chomp
 die "Not a valid revision: #{base}" if baserev.empty?
 
@@ -86,7 +85,7 @@ die "Not a valid revision: #{head}" if headrev.empty?
 merge_base = `git merge-base #{baserev} #{headrev}`.chomp
 die "No commits in common between #{base} and #{head}" unless $?.success?
 
-ref = get_ref(url, headref, headrev, tag_name) || ''
+ref = get_ref(url, headref != "HEAD" ? headref : nil, headrev, tag_name)
 url = `git ls-remote --get-url "#{url}"`.chomp
 
 begin
@@ -96,7 +95,7 @@ begin
 
 are available in the git repository at:
 ' #{merge_base}])
-  puts "  #{url}" + (ref.empty? ? "" : " #{ref}")
+  puts "  #{url}" + (ref ? " #{ref}" : "")
   run(%[git show -s --format='
 for you to fetch changes up to %H:
 
@@ -107,11 +106,11 @@ for you to fetch changes up to %H:
   if branch_name
     puts "(from the branch description for #{branch_name} local branch)"
     puts
-    run(%[git config "branch.#{branch_name}.description"])
+    puts branch_desc
   end
 
   if not tag_name.empty?
-    if ref.empty? || ref != "tags/#{tag_name}"
+    if ref != "tags/#{tag_name}"
       $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
       $stderr.puts "warn: appear to be at #{url}"
       $stderr.puts "warn: Do you want to push it there, perhaps?"
@@ -124,14 +123,13 @@ for you to fetch changes up to %H:
     puts "----------------------------------------------------------------"
   end
 
-  run(%[git shortlog ^#{baserev} #{headrev}])
-  run(%[git diff -M --stat --summary #{patch} #{merge_base}..#{headrev}])
+  run(%[git shortlog ^#{base} #{head}])
+  run(%[git diff -M --stat --summary #{patch} ^#{merge_base} #{head}])
 
-  if ref.empty?
-    short_headref = `git rev-parse -q --verify --symbolic-full-name --abbrev-ref "#{head}"`.chomp
+  if ! ref
     $stderr.puts "warn: No branch of #{url} is at:"
-    run("git show -s --format='warn:   %h: %s' #{headrev} >&2")
-    $stderr.puts "warn: Are you sure you pushed '#{short_headref}' there?"
+    run("git show -s --format='warn:   %h: %s' #{head} >&2")
+    $stderr.puts "warn: Are you sure you pushed '#{abbr(headref)}' there?"
     status = 1
   end
 rescue CommandError
-- 
1.8.4-fc

[PATCH v2 13/44] ruby: bind setup_git_directory()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c           | 16 ++++++++++++++++
 t/t10000-ruby.sh | 13 +++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/ruby.c b/ruby.c
index 339e376..c2ef72e 100644
--- a/ruby.c
+++ b/ruby.c
@@ -12,6 +12,21 @@ static inline VALUE sha1_to_str(const unsigned char *sha1)
 	return rb_str_new((const char *)sha1, 20);
 }
 
+static inline VALUE cstr_to_str(const char *str)
+{
+	if (str == NULL)
+		return Qnil;
+	return rb_str_new2(str);
+}
+
+static VALUE git_rb_setup_git_directory(VALUE self)
+{
+	int nongit_ok;
+	const char *prefix;
+	prefix = setup_git_directory_gently(&nongit_ok);
+	return rb_ary_new3(2, cstr_to_str(prefix), INT2FIX(nongit_ok));
+}
+
 static int for_each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
 	VALUE r;
@@ -28,6 +43,7 @@ static VALUE git_rb_for_each_ref(void)
 
 static void git_ruby_init(void)
 {
+	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 }
 
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index eb03706..2098e29 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -39,4 +39,17 @@ test_expect_success 'test for_each_ref()' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test setup_git_directory()' '
+	mkdir t &&
+	(
+	cd t &&
+	git ruby > ../actual <<-EOF
+	prefix, nongit_ok = setup_git_directory()
+	puts prefix
+	EOF
+	) &&
+	echo "t/" > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 14/44] ruby: bind dwim_ref()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c           | 11 +++++++++++
 t/t10000-ruby.sh | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/ruby.c b/ruby.c
index c2ef72e..0ab56de 100644
--- a/ruby.c
+++ b/ruby.c
@@ -41,10 +41,21 @@ static VALUE git_rb_for_each_ref(void)
 	return INT2FIX(r);
 }
 
+static VALUE git_rb_dwim_ref(VALUE self, VALUE name)
+{
+	unsigned char buf[20];
+	char *ref;
+	int r;
+
+	r = dwim_ref(RSTRING_PTR(name), RSTRING_LEN(name), buf, &ref);
+	return rb_ary_new3(3, sha1_to_str(buf), INT2NUM(r), cstr_to_str(ref));
+}
+
 static void git_ruby_init(void)
 {
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
+	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index 2098e29..d719b30 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -52,4 +52,15 @@ test_expect_success 'test setup_git_directory()' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test dwim_ref()' '
+	git ruby > actual <<-EOF &&
+	sha1, num, ref = dwim_ref("HEAD")
+	puts sha1_to_hex(sha1), num, ref
+	EOF
+	git rev-parse -q --verify HEAD > expected &&
+	echo 1 >> expected &&
+	git rev-parse -q --verify --symbolic-full-name HEAD >> expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 15/44] ruby: request-pull: use native dwim_ref()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Our first real usage of the bindings doesn't look too different from what it
    would look like in C.
    
    In Ruby all variables are pointers to objects, and there's no concept of
    pointer to variables, so what in C looks like "ret1 = func(var, &ret2)", in
    Ruby looks like "ret1, ret2 = func(var)".

 git-request-pull.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 9d3e9ab..416f880 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -66,7 +66,7 @@ branch_name = branch_desc = nil
 
 usage unless base or url
 
-headref = `git rev-parse -q --verify --symbolic-full-name "#{head}"`.chomp
+_, _, headref = dwim_ref(head)
 
 if headref.start_with?('refs/heads')
   branch_name = headref[11..-1]
-- 
1.8.4-fc

[PATCH v2 16/44] ruby: bind git_config()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Our first usage of Ruby blocks.
    
    Notice how the code inside the block is in the same scope as the code outside.
    
      cb_data = "config"
      git_config() do |key, value|
        puts "%s: %s=%s" % [cb_data, key, value]
      end
    
    This invalidates the need for callback data, because the block has access to
    those variables already.

 ruby.c           | 15 +++++++++++++++
 t/t10000-ruby.sh | 10 ++++++++++
 2 files changed, 25 insertions(+)

diff --git a/ruby.c b/ruby.c
index 0ab56de..36de943 100644
--- a/ruby.c
+++ b/ruby.c
@@ -51,11 +51,26 @@ static VALUE git_rb_dwim_ref(VALUE self, VALUE name)
 	return rb_ary_new3(3, sha1_to_str(buf), INT2NUM(r), cstr_to_str(ref));
 }
 
+static int git_config_fn(const char *var, const char *value, void *cb_data)
+{
+	VALUE r;
+	r = rb_yield_values(2, rb_str_new2(var), rb_str_new2(value));
+	return r == Qfalse;
+}
+
+static VALUE git_rb_git_config(VALUE self)
+{
+	int r;
+	r = git_config(git_config_fn, NULL);
+	return INT2FIX(r);
+}
+
 static void git_ruby_init(void)
 {
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
+	rb_define_global_function("git_config", git_rb_git_config, 0);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index d719b30..cdbf5d8 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -63,4 +63,14 @@ test_expect_success 'test dwim_ref()' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test git_config()' '
+	git ruby > actual <<-EOF &&
+	git_config() do |key, value|
+	  puts "%s=%s" % [key, value]
+	end
+	EOF
+	git config --list > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 17/44] ruby: request-pull: use native git_config()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 416f880..0f85024 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -14,6 +14,13 @@ EOF
   exit 1
 end
 
+def read_branch_desc(name)
+  git_config() do |key, value|
+    return value if key == "branch.#{name}.description"
+  end
+  return nil
+end
+
 def abbr(ref)
   if (ref =~ %r{^refs/heads/(.*)} || ref =~ %r{^refs/(tags/.*)})
     return $1
@@ -70,8 +77,8 @@ _, _, headref = dwim_ref(head)
 
 if headref.start_with?('refs/heads')
   branch_name = headref[11..-1]
-  branch_desc = `git config "branch.#{branch_name}.description"`.chomp
-  branch_name = nil if branch_desc.empty?
+  branch_desc = read_branch_desc(branch_name)
+  branch_name = nil if not branch_desc
 end
 
 tag_name = `git describe --exact "#{head}^0" 2>/dev/null`.chomp
-- 
1.8.4-fc

[PATCH v2 18/44] ruby: bind read_ref()/peel_ref()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/ruby.c b/ruby.c
index 36de943..92c636c 100644
--- a/ruby.c
+++ b/ruby.c
@@ -65,12 +65,30 @@ static VALUE git_rb_git_config(VALUE self)
 	return INT2FIX(r);
 }
 
+static VALUE git_rb_read_ref(VALUE self, VALUE refname)
+{
+	unsigned char sha1[20];
+	if (read_ref(RSTRING_PTR(refname), sha1))
+		return Qnil;
+	return sha1_to_str(sha1);
+}
+
+static VALUE git_rb_peel_ref(VALUE self, VALUE refname)
+{
+	unsigned char sha1[20];
+	if (peel_ref(RSTRING_PTR(refname), sha1))
+		return Qnil;
+	return sha1_to_str(sha1);
+}
+
 static void git_ruby_init(void)
 {
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
 	rb_define_global_function("git_config", git_rb_git_config, 0);
+	rb_define_global_function("read_ref", git_rb_read_ref, 1);
+	rb_define_global_function("peel_ref", git_rb_peel_ref, 1);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
-- 
1.8.4-fc

[PATCH v2 19/44] ruby: bind get_sha1()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c           | 11 +++++++++++
 t/t10000-ruby.sh |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/ruby.c b/ruby.c
index 92c636c..45111e6 100644
--- a/ruby.c
+++ b/ruby.c
@@ -81,6 +81,16 @@ static VALUE git_rb_peel_ref(VALUE self, VALUE refname)
 	return sha1_to_str(sha1);
 }
 
+static VALUE git_rb_get_sha1(VALUE self, VALUE name)
+{
+	unsigned char buf[20];
+	int r;
+	r = get_sha1(RSTRING_PTR(name), buf);
+	if (r)
+		return Qnil;
+	return sha1_to_str(buf);
+}
+
 static void git_ruby_init(void)
 {
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
@@ -89,6 +99,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("git_config", git_rb_git_config, 0);
 	rb_define_global_function("read_ref", git_rb_read_ref, 1);
 	rb_define_global_function("peel_ref", git_rb_peel_ref, 1);
+	rb_define_global_function("get_sha1", git_rb_get_sha1, 1);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index cdbf5d8..849a615 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -73,4 +73,12 @@ test_expect_success 'test git_config()' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test get_sha1()' '
+	git ruby > actual <<-EOF &&
+	puts sha1_to_hex(get_sha1("HEAD"))
+	EOF
+	git rev-parse -q --verify HEAD > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 20/44] ruby: request-pull: simplify tag fetching

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    For the moment we use "str[10..-1]" to denote the equivalent of "str + 10" in
    C, even though Ruby has nicer constructs than that.

 git-request-pull.rb | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 0f85024..25dc04d 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -21,6 +21,15 @@ def read_branch_desc(name)
   return nil
 end
 
+def describe(rev)
+  for_each_ref() do |name, sha1, flags|
+    next unless name.start_with?('refs/tags/')
+    next unless peel_ref(name) == get_sha1(rev)
+    return name[10..-1]
+  end
+  return nil
+end
+
 def abbr(ref)
   if (ref =~ %r{^refs/heads/(.*)} || ref =~ %r{^refs/(tags/.*)})
     return $1
@@ -81,7 +90,7 @@ if headref.start_with?('refs/heads')
   branch_name = nil if not branch_desc
 end
 
-tag_name = `git describe --exact "#{head}^0" 2>/dev/null`.chomp
+tag_name = describe(head)
 
 baserev = `git rev-parse --verify --quiet "#{base}"^0`.chomp
 die "Not a valid revision: #{base}" if baserev.empty?
@@ -116,7 +125,7 @@ for you to fetch changes up to %H:
     puts branch_desc
   end
 
-  if not tag_name.empty?
+  if tag_name
     if ref != "tags/#{tag_name}"
       $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
       $stderr.puts "warn: appear to be at #{url}"
@@ -126,7 +135,7 @@ for you to fetch changes up to %H:
     puts
   end
 
-  if branch_name || ! tag_name.empty?
+  if branch_name || tag_name
     puts "----------------------------------------------------------------"
   end
 
-- 
1.8.4-fc

[PATCH v2 21/44] ruby: request-pull: use get_sha1()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    To minimize the changes in the rest of the script, the sha1 is converted by to
    a hex.

 git-request-pull.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 25dc04d..9267147 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -92,11 +92,14 @@ end
 
 tag_name = describe(head)
 
-baserev = `git rev-parse --verify --quiet "#{base}"^0`.chomp
-die "Not a valid revision: #{base}" if baserev.empty?
+baserev = get_sha1("#{base}^0")
+die "Not a valid revision: #{base}" unless baserev
 
-headrev = `git rev-parse --verify --quiet "#{head}"^0`.chomp
-die "Not a valid revision: #{head}" if headrev.empty?
+headrev = get_sha1("#{head}^0")
+die "Not a valid revision: #{head}" unless headrev
+
+baserev = sha1_to_hex(baserev)
+headrev = sha1_to_hex(headrev)
 
 merge_base = `git merge-base #{baserev} #{headrev}`.chomp
 die "No commits in common between #{base} and #{head}" unless $?.success?
-- 
1.8.4-fc

[PATCH v2 22/44] ruby: add Commit class

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    The Commit class inherits from the Object class, so commit.sha1() just works,
    even though it's an Object method.
    
    For now we don't define initializers or allocators, so Git::Commit.new() cannot
    really be used, but it's not needed at this point. These would be demonstrated
    later in the series.

 ruby.c           | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t10000-ruby.sh | 21 ++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/ruby.c b/ruby.c
index 45111e6..53afa32 100644
--- a/ruby.c
+++ b/ruby.c
@@ -1,12 +1,17 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "refs.h"
+#include "object.h"
+#include "commit.h"
 
 #undef NORETURN
 #undef PATH_SEP
 
 #include <ruby.h>
 
+static VALUE git_rb_object;
+static VALUE git_rb_commit;
+
 static inline VALUE sha1_to_str(const unsigned char *sha1)
 {
 	return rb_str_new((const char *)sha1, 20);
@@ -19,6 +24,18 @@ static inline VALUE cstr_to_str(const char *str)
 	return rb_str_new2(str);
 }
 
+static inline char *str_to_cstr(VALUE str)
+{
+	if (str == Qnil)
+		return NULL;
+	return RSTRING_PTR(str);
+}
+
+static inline unsigned char *str_to_sha1(VALUE str)
+{
+	return (unsigned char *)str_to_cstr(str);
+}
+
 static VALUE git_rb_setup_git_directory(VALUE self)
 {
 	int nongit_ok;
@@ -91,8 +108,67 @@ static VALUE git_rb_get_sha1(VALUE self, VALUE name)
 	return sha1_to_str(buf);
 }
 
+static VALUE git_rb_object_get(VALUE class, VALUE id)
+{
+	struct object *object;
+	object = parse_object(str_to_sha1(id));
+	return Data_Wrap_Struct(git_rb_object, NULL, NULL, object);
+}
+
+static VALUE git_rb_commit_get(VALUE class, VALUE id)
+{
+	struct object *object;
+	object = parse_object(str_to_sha1(id));
+	if (!object || object->type != OBJ_COMMIT)
+		return Qnil;
+	return Data_Wrap_Struct(git_rb_commit, NULL, NULL, object);
+}
+
+static VALUE git_rb_object_sha1(VALUE self)
+{
+	struct object *object;
+	Data_Get_Struct(self, struct object, object);
+	return sha1_to_str(object->sha1);
+}
+
+static VALUE git_rb_object_type(VALUE self)
+{
+	struct object *object;
+	Data_Get_Struct(self, struct object, object);
+	return INT2FIX(object->type);
+}
+
+static VALUE git_rb_object_to_s(VALUE self)
+{
+	struct object *object;
+	Data_Get_Struct(self, struct object, object);
+	return rb_str_new2(sha1_to_hex(object->sha1));
+}
+
+static VALUE git_rb_commit_buffer(VALUE self)
+{
+	struct commit *commit;
+	Data_Get_Struct(self, struct commit, commit);
+	return cstr_to_str(commit->buffer);
+}
+
 static void git_ruby_init(void)
 {
+	VALUE mod;
+
+	mod = rb_define_module("Git");
+
+	rb_define_global_const("OBJ_BAD", INT2FIX(OBJ_BAD));
+	rb_define_global_const("OBJ_NONE", INT2FIX(OBJ_NONE));
+	rb_define_global_const("OBJ_COMMIT", INT2FIX(OBJ_COMMIT));
+	rb_define_global_const("OBJ_TREE", INT2FIX(OBJ_TREE));
+	rb_define_global_const("OBJ_BLOB", INT2FIX(OBJ_BLOB));
+	rb_define_global_const("OBJ_TAG", INT2FIX(OBJ_TAG));
+	rb_define_global_const("OBJ_OFS_DELTA", INT2FIX(OBJ_OFS_DELTA));
+	rb_define_global_const("OBJ_REF_DELTA", INT2FIX(OBJ_REF_DELTA));
+	rb_define_global_const("OBJ_ANY", INT2FIX(OBJ_ANY));
+	rb_define_global_const("OBJ_MAX", INT2FIX(OBJ_MAX));
+
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
@@ -100,6 +176,16 @@ static void git_ruby_init(void)
 	rb_define_global_function("read_ref", git_rb_read_ref, 1);
 	rb_define_global_function("peel_ref", git_rb_peel_ref, 1);
 	rb_define_global_function("get_sha1", git_rb_get_sha1, 1);
+
+	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
+	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
+	rb_define_method(git_rb_object, "sha1", git_rb_object_sha1, 0);
+	rb_define_method(git_rb_object, "type", git_rb_object_type, 0);
+	rb_define_method(git_rb_object, "to_s", git_rb_object_to_s, 0);
+
+	git_rb_commit = rb_define_class_under(mod, "Commit", git_rb_object);
+	rb_define_singleton_method(git_rb_commit, "get", git_rb_commit_get, 1);
+	rb_define_method(git_rb_commit, "buffer", git_rb_commit_buffer, 0);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index 849a615..e5a397c 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -81,4 +81,25 @@ test_expect_success 'test get_sha1()' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test Object' '
+	git ruby > actual <<-EOF &&
+	object = Git::Object.get(get_sha1("HEAD"))
+	puts object, object.type == OBJ_COMMIT, sha1_to_hex(object.sha1)
+	EOF
+	git rev-parse -q --verify HEAD > expected &&
+	echo "true" >> expected &&
+	git rev-parse -q --verify HEAD >> expected &&
+	test_cmp expected actual
+'
+
+test_expect_success 'test Commit' '
+	git ruby > actual <<-EOF &&
+	commit = Git::Commit.get(get_sha1("HEAD"))
+	puts commit, commit.buffer
+	EOF
+	git rev-parse -q --verify HEAD > expected &&
+	git cat-file commit HEAD >> expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 23/44] ruby: bind get_merge_bases()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    We could call it get_merge_bases_many(), but the fact that it's awkward to
    pass arrays in C is a limitation we don't have in Ruby. We can pass as many
    commits as we want easily.

 ruby.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/ruby.c b/ruby.c
index 53afa32..cc8fe0e 100644
--- a/ruby.c
+++ b/ruby.c
@@ -11,6 +11,7 @@
 
 static VALUE git_rb_object;
 static VALUE git_rb_commit;
+static VALUE git_rb_commit_list;
 
 static inline VALUE sha1_to_str(const unsigned char *sha1)
 {
@@ -152,6 +153,36 @@ static VALUE git_rb_commit_buffer(VALUE self)
 	return cstr_to_str(commit->buffer);
 }
 
+static VALUE git_rb_commit_list_each(VALUE self)
+{
+	struct commit_list *e, *list;
+	Data_Get_Struct(self, struct commit_list, list);
+
+	for (e = list; e; e = e->next) {
+		VALUE c;
+		c = Data_Wrap_Struct(git_rb_commit, NULL, NULL, e->item);
+		rb_yield(c);
+	}
+
+	return self;
+}
+
+static VALUE git_rb_get_merge_bases(VALUE self, VALUE commits, VALUE cleanup)
+{
+	struct commit *g_commits[RARRAY_LEN(commits)];
+	struct commit_list *result;
+	int i;
+
+	for (i = 0; i < RARRAY_LEN(commits); i++) {
+		VALUE commit = RARRAY_PTR(commits)[i];
+		Data_Get_Struct(commit, struct commit, g_commits[i]);
+	}
+	result = get_merge_bases_many(g_commits[0], RARRAY_LEN(commits) - 1, g_commits + 1, NUM2INT(cleanup));
+	if (!result)
+		return Qnil;
+	return Data_Wrap_Struct(git_rb_commit_list, NULL, NULL, result);
+}
+
 static void git_ruby_init(void)
 {
 	VALUE mod;
@@ -176,6 +207,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("read_ref", git_rb_read_ref, 1);
 	rb_define_global_function("peel_ref", git_rb_peel_ref, 1);
 	rb_define_global_function("get_sha1", git_rb_get_sha1, 1);
+	rb_define_global_function("get_merge_bases", git_rb_get_merge_bases, 2);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
@@ -186,6 +218,10 @@ static void git_ruby_init(void)
 	git_rb_commit = rb_define_class_under(mod, "Commit", git_rb_object);
 	rb_define_singleton_method(git_rb_commit, "get", git_rb_commit_get, 1);
 	rb_define_method(git_rb_commit, "buffer", git_rb_commit_buffer, 0);
+
+	git_rb_commit_list = rb_define_class_under(mod, "CommitList", rb_cData);
+	rb_include_module(git_rb_commit_list, rb_mEnumerable);
+	rb_define_method(git_rb_commit_list, "each", git_rb_commit_list_each, 0);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
-- 
1.8.4-fc

[PATCH v2 24/44] ruby: request-pull: use get_merge_bases()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Now headrev and baserev become Git::Commit objects, fortunately
    'puts("#{commit}")' or 'printf("%s\n", commit) convert the commit to a String
    (commit.to_s()), which is the hex sha1, so the rest of the script doesn't need
    to be changed.

 git-request-pull.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 9267147..be4947d 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -98,13 +98,15 @@ die "Not a valid revision: #{base}" unless baserev
 headrev = get_sha1("#{head}^0")
 die "Not a valid revision: #{head}" unless headrev
 
-baserev = sha1_to_hex(baserev)
-headrev = sha1_to_hex(headrev)
+baserev = Git::Commit.get(baserev)
+headrev = Git::Commit.get(headrev)
 
-merge_base = `git merge-base #{baserev} #{headrev}`.chomp
-die "No commits in common between #{base} and #{head}" unless $?.success?
+merge_bases = get_merge_bases([baserev, headrev], 0);
+die "No commits in common between #{base} and #{head}" unless merge_bases
 
-ref = get_ref(url, headref != "HEAD" ? headref : nil, headrev, tag_name)
+merge_base = sha1_to_hex(merge_bases.first.sha1)
+
+ref = get_ref(url, headref != "HEAD" ? headref : nil, headrev.to_s, tag_name)
 url = `git ls-remote --get-url "#{url}"`.chomp
 
 begin
-- 
1.8.4-fc

[PATCH v2 25/44] ruby: request-pull: trivial cleanups

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    The fist override of a native class, which is one of the strong points of Ruby.
    
    We could use skip_prefix(str, prefix) like in C, but str.skip_prefix(prefix) is
    more natrual in Ruby.

 git-rb-setup.rb     |  6 ++++++
 git-request-pull.rb | 36 ++++++++++++++++++------------------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/git-rb-setup.rb b/git-rb-setup.rb
index 92998a2..6f283da 100644
--- a/git-rb-setup.rb
+++ b/git-rb-setup.rb
@@ -26,3 +26,9 @@ def run(cmd, *args)
   system(*cmd, *args)
   raise CommandError.new(cmd) unless $?.success?
 end
+
+class String
+  def skip_prefix(prefix)
+    return self[prefix.length..-1]
+  end
+end
diff --git a/git-request-pull.rb b/git-request-pull.rb
index be4947d..79a26dc 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -25,7 +25,7 @@ def describe(rev)
   for_each_ref() do |name, sha1, flags|
     next unless name.start_with?('refs/tags/')
     next unless peel_ref(name) == get_sha1(rev)
-    return name[10..-1]
+    return name.skip_prefix('refs/tags/')
   end
   return nil
 end
@@ -43,17 +43,17 @@ end
 # Otherwise, if a branch with the same name as $head exists at the remote
 # and their values match, use that instead.
 #
-# Otherwise find a random ref that matches $headrev.
+# Otherwise find a random ref that matches $head_id.
 
-def get_ref(url, headref, headrev, tag_name)
+def get_ref(url, head_ref, head_id, tag_name)
   found = nil
   IO.popen(%[git ls-remote "#{url}"]) do |out|
     out.each do |l|
       sha1, ref, deref = l.scan(/^(\S+)\s+(\S+?)(\^\{\})?$/).first
-      next unless sha1 == headrev
+      next unless sha1 == head_id
       found = abbr(ref)
       break if (deref && ref == "refs/tags/#{tag_name}")
-      break if ref == headref
+      break if ref == head_ref
     end
   end
   return found
@@ -82,31 +82,31 @@ branch_name = branch_desc = nil
 
 usage unless base or url
 
-_, _, headref = dwim_ref(head)
+_, _, head_ref = dwim_ref(head)
 
-if headref.start_with?('refs/heads')
-  branch_name = headref[11..-1]
+if head_ref.start_with?('refs/heads')
+  branch_name = head_ref[11..-1]
   branch_desc = read_branch_desc(branch_name)
   branch_name = nil if not branch_desc
 end
 
 tag_name = describe(head)
 
-baserev = get_sha1("#{base}^0")
-die "Not a valid revision: #{base}" unless baserev
+base_id = get_sha1("#{base}^0")
+die "Not a valid revision: #{base}" unless base_id
 
-headrev = get_sha1("#{head}^0")
-die "Not a valid revision: #{head}" unless headrev
+head_id = get_sha1("#{head}^0")
+die "Not a valid revision: #{head}" unless head_id
 
-baserev = Git::Commit.get(baserev)
-headrev = Git::Commit.get(headrev)
+base_commit = Git::Commit.get(base_id)
+head_commit = Git::Commit.get(head_id)
 
-merge_bases = get_merge_bases([baserev, headrev], 0);
+merge_bases = get_merge_bases([base_commit, head_commit], 0);
 die "No commits in common between #{base} and #{head}" unless merge_bases
 
 merge_base = sha1_to_hex(merge_bases.first.sha1)
 
-ref = get_ref(url, headref != "HEAD" ? headref : nil, headrev.to_s, tag_name)
+ref = get_ref(url, head_ref != "HEAD" ? head_ref : nil, head_commit.to_s, tag_name)
 url = `git ls-remote --get-url "#{url}"`.chomp
 
 begin
@@ -122,7 +122,7 @@ for you to fetch changes up to %H:
 
   %s (%ci)
 
-----------------------------------------------------------------' #{headrev}])
+----------------------------------------------------------------' #{head_commit}])
 
   if branch_name
     puts "(from the branch description for #{branch_name} local branch)"
@@ -150,7 +150,7 @@ for you to fetch changes up to %H:
   if ! ref
     $stderr.puts "warn: No branch of #{url} is at:"
     run("git show -s --format='warn:   %h: %s' #{head} >&2")
-    $stderr.puts "warn: Are you sure you pushed '#{abbr(headref)}' there?"
+    $stderr.puts "warn: Are you sure you pushed '#{abbr(head_ref)}' there?"
     status = 1
   end
 rescue CommandError
-- 
1.8.4-fc

[PATCH v2 26/44] ruby: bind remote and transport stuff

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Only the few fields that will be needed later on.

 ruby.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/ruby.c b/ruby.c
index cc8fe0e..ee548dc 100644
--- a/ruby.c
+++ b/ruby.c
@@ -3,6 +3,8 @@
 #include "refs.h"
 #include "object.h"
 #include "commit.h"
+#include "remote.h"
+#include "transport.h"
 
 #undef NORETURN
 #undef PATH_SEP
@@ -12,6 +14,9 @@
 static VALUE git_rb_object;
 static VALUE git_rb_commit;
 static VALUE git_rb_commit_list;
+static VALUE git_rb_remote;
+static VALUE git_rb_transport;
+static VALUE git_rb_ref;
 
 static inline VALUE sha1_to_str(const unsigned char *sha1)
 {
@@ -183,6 +188,76 @@ static VALUE git_rb_get_merge_bases(VALUE self, VALUE commits, VALUE cleanup)
 	return Data_Wrap_Struct(git_rb_commit_list, NULL, NULL, result);
 }
 
+static VALUE git_rb_remote_get(VALUE self, VALUE name)
+{
+	struct remote *remote;
+	remote = remote_get(RSTRING_PTR(name));
+	if (!remote)
+		return Qnil;
+	return Data_Wrap_Struct(git_rb_remote, NULL, NULL, remote);
+}
+
+static VALUE git_rb_remote_url(VALUE self)
+{
+	struct remote *remote;
+	VALUE url;
+	int i;
+
+	Data_Get_Struct(self, struct remote, remote);
+	url = rb_ary_new2(remote->url_nr);
+	for (i = 0; i < remote->url_nr; i++)
+		rb_ary_store(url, i, rb_str_new2(remote->url[i]));
+	return url;
+}
+
+static VALUE git_rb_transport_get(VALUE self, VALUE remote, VALUE url)
+{
+	struct transport *transport;
+	struct remote *g_remote;
+	Data_Get_Struct(remote, struct remote, g_remote);
+	transport = transport_get(g_remote, str_to_cstr(url));
+	if (!transport)
+		return Qnil;
+	return Data_Wrap_Struct(git_rb_transport, NULL, transport_disconnect, transport);
+}
+
+static VALUE git_rb_transport_get_remote_refs(VALUE self)
+{
+	struct transport *transport;
+	const struct ref *ref;
+	Data_Get_Struct(self, struct transport, transport);
+	ref = transport_get_remote_refs(transport);
+	return Data_Wrap_Struct(git_rb_ref, NULL, NULL, (void *)ref);
+}
+
+static VALUE git_rb_ref_each(VALUE self)
+{
+	struct ref *e, *ref;
+	Data_Get_Struct(self, struct ref, ref);
+
+	for (e = ref; e; e = e->next) {
+		VALUE c;
+		c = Data_Wrap_Struct(git_rb_ref, NULL, NULL, e);
+		rb_yield(c);
+	}
+
+	return self;
+}
+
+static VALUE git_rb_ref_name(VALUE self)
+{
+	struct ref *ref;
+	Data_Get_Struct(self, struct ref, ref);
+	return rb_str_new2(ref->name);
+}
+
+static VALUE git_rb_ref_old_sha1(VALUE self)
+{
+	struct ref *ref;
+	Data_Get_Struct(self, struct ref, ref);
+	return sha1_to_str(ref->old_sha1);
+}
+
 static void git_ruby_init(void)
 {
 	VALUE mod;
@@ -208,6 +283,8 @@ static void git_ruby_init(void)
 	rb_define_global_function("peel_ref", git_rb_peel_ref, 1);
 	rb_define_global_function("get_sha1", git_rb_get_sha1, 1);
 	rb_define_global_function("get_merge_bases", git_rb_get_merge_bases, 2);
+	rb_define_global_function("remote_get", git_rb_remote_get, 1);
+	rb_define_global_function("transport_get", git_rb_transport_get, 2);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
@@ -222,6 +299,18 @@ static void git_ruby_init(void)
 	git_rb_commit_list = rb_define_class_under(mod, "CommitList", rb_cData);
 	rb_include_module(git_rb_commit_list, rb_mEnumerable);
 	rb_define_method(git_rb_commit_list, "each", git_rb_commit_list_each, 0);
+
+	git_rb_remote = rb_define_class_under(mod, "Remote", rb_cData);
+	rb_define_method(git_rb_remote, "url", git_rb_remote_url, 0);
+
+	git_rb_transport = rb_define_class_under(mod, "Transport", rb_cData);
+	rb_define_method(git_rb_transport, "get_remote_refs", git_rb_transport_get_remote_refs, 0);
+
+	git_rb_ref = rb_define_class_under(mod, "Ref", rb_cData);
+	rb_include_module(git_rb_ref, rb_mEnumerable);
+	rb_define_method(git_rb_ref, "each", git_rb_ref_each, 0);
+	rb_define_method(git_rb_ref, "name", git_rb_ref_name, 0);
+	rb_define_method(git_rb_ref, "old_sha1", git_rb_ref_old_sha1, 0);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
-- 
1.8.4-fc

[PATCH v2 27/44] ruby: request-pull: use native remote and transport

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 79a26dc..869f160 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -45,16 +45,15 @@ end
 #
 # Otherwise find a random ref that matches $head_id.
 
-def get_ref(url, head_ref, head_id, tag_name)
+def get_ref(transport, head_ref, head_id, tag_name)
   found = nil
-  IO.popen(%[git ls-remote "#{url}"]) do |out|
-    out.each do |l|
-      sha1, ref, deref = l.scan(/^(\S+)\s+(\S+?)(\^\{\})?$/).first
-      next unless sha1 == head_id
-      found = abbr(ref)
-      break if (deref && ref == "refs/tags/#{tag_name}")
-      break if ref == head_ref
-    end
+  transport.get_remote_refs().each do |e|
+    sha1 = e.old_sha1
+    next unless sha1 == head_id
+    ref, deref = e.name.scan(/^(\S+?)(\^\{\})?$/).first
+    found = abbr(ref)
+    break if (deref && ref == "refs/tags/#{tag_name}")
+    break if ref == head_ref
   end
   return found
 end
@@ -106,8 +105,11 @@ die "No commits in common between #{base} and #{head}" unless merge_bases
 
 merge_base = sha1_to_hex(merge_bases.first.sha1)
 
-ref = get_ref(url, head_ref != "HEAD" ? head_ref : nil, head_commit.to_s, tag_name)
-url = `git ls-remote --get-url "#{url}"`.chomp
+remote = remote_get(url)
+transport = transport_get(remote, nil)
+
+ref = get_ref(transport, head_ref != "HEAD" ? head_ref : nil, head_id, tag_name)
+url = remote.url.first
 
 begin
   run(%[git show -s --format='The following changes since commit %H:
-- 
1.8.4-fc

[PATCH v2 28/44] ruby: bind find_unique_abbrev()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ruby.c b/ruby.c
index ee548dc..7496c18 100644
--- a/ruby.c
+++ b/ruby.c
@@ -258,6 +258,13 @@ static VALUE git_rb_ref_old_sha1(VALUE self)
 	return sha1_to_str(ref->old_sha1);
 }
 
+static VALUE git_rb_find_unique_abbrev(VALUE self, VALUE sha1, VALUE len)
+{
+	const char *abbrev;
+	abbrev = find_unique_abbrev(str_to_sha1(sha1), NUM2INT(len));
+	return rb_str_new2(abbrev);
+}
+
 static void git_ruby_init(void)
 {
 	VALUE mod;
@@ -275,6 +282,8 @@ static void git_ruby_init(void)
 	rb_define_global_const("OBJ_ANY", INT2FIX(OBJ_ANY));
 	rb_define_global_const("OBJ_MAX", INT2FIX(OBJ_MAX));
 
+	rb_define_global_const("DEFAULT_ABBREV", INT2FIX(DEFAULT_ABBREV));
+
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
@@ -285,6 +294,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("get_merge_bases", git_rb_get_merge_bases, 2);
 	rb_define_global_function("remote_get", git_rb_remote_get, 1);
 	rb_define_global_function("transport_get", git_rb_transport_get, 2);
+	rb_define_global_function("find_unique_abbrev", git_rb_find_unique_abbrev, 2);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
-- 
1.8.4-fc

[PATCH v2 29/44] ruby: request-pull: use native commit info

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    We could use a bit more of the internal parsing in C, or we could export more,
    but there's no need to, parse_buffer() is relatively simple in Ruby.

 git-request-pull.rb | 47 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 869f160..5f40901 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -1,5 +1,7 @@
 #!git ruby
 
+require 'date'
+
 ENV['GIT_PAGER'] =
 
 patch = ''
@@ -58,6 +60,25 @@ def get_ref(transport, head_ref, head_id, tag_name)
   return found
 end
 
+def parse_buffer(buffer)
+  summary = []
+  date = msg = nil
+  header, body = buffer.split("\n\n", 2)
+  header.each_line do |line|
+    case line
+    when /^committer ([^<>]+) <(\S+)> (.+)$/
+      date = DateTime.strptime($3, '%s %z')
+    end
+  end
+  body.each_line do |l|
+    break if (l.strip.empty?)
+    summary << l.chomp
+  end
+  summary = summary.join(' ')
+  date = date.strftime('%F %T %z')
+  return [summary, date]
+end
+
 until ARGV.empty?
   case ARGV.first
   when '-p'
@@ -103,7 +124,8 @@ head_commit = Git::Commit.get(head_id)
 merge_bases = get_merge_bases([base_commit, head_commit], 0);
 die "No commits in common between #{base} and #{head}" unless merge_bases
 
-merge_base = sha1_to_hex(merge_bases.first.sha1)
+merge_base_id = merge_bases.first.sha1
+merge_base_commit = Git::Commit.get(merge_base_id)
 
 remote = remote_get(url)
 transport = transport_get(remote, nil)
@@ -111,20 +133,25 @@ transport = transport_get(remote, nil)
 ref = get_ref(transport, head_ref != "HEAD" ? head_ref : nil, head_id, tag_name)
 url = remote.url.first
 
+merge_base_summary, merge_base_date = parse_buffer(merge_base_commit.buffer)
+head_summary, head_date = parse_buffer(head_commit.buffer)
+
 begin
-  run(%[git show -s --format='The following changes since commit %H:
+  puts "The following changes since commit %s:
 
-  %s (%ci)
+  %s (%s)
 
 are available in the git repository at:
-' #{merge_base}])
+
+" % [merge_base_commit, merge_base_summary, merge_base_date]
   puts "  #{url}" + (ref ? " #{ref}" : "")
-  run(%[git show -s --format='
-for you to fetch changes up to %H:
+  puts "
+for you to fetch changes up to %s:
 
-  %s (%ci)
+  %s (%s)
 
-----------------------------------------------------------------' #{head_commit}])
+----------------------------------------------------------------
+" % [head_commit, head_summary, head_date]
 
   if branch_name
     puts "(from the branch description for #{branch_name} local branch)"
@@ -147,11 +174,11 @@ for you to fetch changes up to %H:
   end
 
   run(%[git shortlog ^#{base} #{head}])
-  run(%[git diff -M --stat --summary #{patch} ^#{merge_base} #{head}])
+  run(%[git diff -M --stat --summary #{patch} ^#{merge_base_commit} #{head}])
 
   if ! ref
     $stderr.puts "warn: No branch of #{url} is at:"
-    run("git show -s --format='warn:   %h: %s' #{head} >&2")
+    $stderr.puts "warn:   %s: %s'" % [find_unique_abbrev(head_id, DEFAULT_ABBREV), head_summary]
     $stderr.puts "warn: Are you sure you pushed '#{abbr(head_ref)}' there?"
     status = 1
   end
-- 
1.8.4-fc

[PATCH v2 30/44] ruby: bind read_sha1_file()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/ruby.c b/ruby.c
index 7496c18..360515f 100644
--- a/ruby.c
+++ b/ruby.c
@@ -265,6 +265,18 @@ static VALUE git_rb_find_unique_abbrev(VALUE self, VALUE sha1, VALUE len)
 	return rb_str_new2(abbrev);
 }
 
+static VALUE git_rb_read_sha1_file(VALUE self, VALUE sha1, VALUE type)
+{
+	enum object_type g_type;
+	void *buffer;
+	unsigned long size;
+
+	buffer = read_sha1_file(str_to_sha1(sha1), &g_type, &size);
+	if (!buffer)
+		return Qnil;
+	return rb_ary_new3(2, rb_str_new(buffer, size), INT2FIX(g_type));
+}
+
 static void git_ruby_init(void)
 {
 	VALUE mod;
@@ -295,6 +307,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("remote_get", git_rb_remote_get, 1);
 	rb_define_global_function("transport_get", git_rb_transport_get, 2);
 	rb_define_global_function("find_unique_abbrev", git_rb_find_unique_abbrev, 2);
+	rb_define_global_function("read_sha1_file", git_rb_read_sha1_file, 1);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
-- 
1.8.4-fc

[PATCH v2 31/44] ruby: request-pull: use read_sha1_file()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 5f40901..233981f 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -165,7 +165,8 @@ for you to fetch changes up to %s:
       $stderr.puts "warn: appear to be at #{url}"
       $stderr.puts "warn: Do you want to push it there, perhaps?"
     end
-    run(%[git cat-file tag "#{tag_name}" | sed -n -e '1,/^$/d' -e '/^-----BEGIN PGP /q' -e p])
+    buffer, _ = read_sha1_file(get_sha1(tag_name))
+    puts buffer.scan(/(?:\n\n)(.+)(?:-----BEGIN PGP )?/m).first
     puts
   end
 
-- 
1.8.4-fc

[PATCH v2 32/44] revision: add missing include

[Felipe Contreras]

Otherwise we might not have 'struct diff_options'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 revision.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/revision.h b/revision.h
index 95859ba..4aa5c5a 100644
--- a/revision.h
+++ b/revision.h
@@ -5,6 +5,7 @@
 #include "grep.h"
 #include "notes.h"
 #include "commit.h"
+#include "diff.h"
 
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
-- 
1.8.4-fc

[PATCH v2 33/44] shortlog: add missing declaration

[Felipe Contreras]

Otherwise we would have to include commit.h.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 shortlog.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/shortlog.h b/shortlog.h
index de4f86f..54bc07c 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -19,6 +19,8 @@ struct shortlog {
 	struct string_list mailmap;
 };
 
+struct commit;
+
 void shortlog_init(struct shortlog *log);
 
 void shortlog_add_commit(struct shortlog *log, struct commit *commit);
-- 
1.8.4-fc

[PATCH v2 34/44] shortlog: split builtin from common code

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    Somebody forgot to do this.

 Makefile           |   1 +
 builtin/shortlog.c | 184 +----------------------------------------------------
 shortlog.c         | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 shortlog.h         |   6 ++
 4 files changed, 189 insertions(+), 183 deletions(-)
 create mode 100644 shortlog.c

diff --git a/Makefile b/Makefile
index 44b4cd5..709c087 100644
--- a/Makefile
+++ b/Makefile
@@ -880,6 +880,7 @@ LIB_OBJS += sha1-lookup.o
 LIB_OBJS += sha1_file.o
 LIB_OBJS += sha1_name.o
 LIB_OBJS += shallow.o
+LIB_OBJS += shortlog.o
 LIB_OBJS += sideband.o
 LIB_OBJS += sigchain.o
 LIB_OBJS += strbuf.o
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1434f8f..f84c6f8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -1,11 +1,8 @@
 #include "builtin.h"
 #include "cache.h"
 #include "commit.h"
-#include "diff.h"
 #include "string-list.h"
 #include "revision.h"
-#include "utf8.h"
-#include "mailmap.h"
 #include "shortlog.h"
 #include "parse-options.h"
 
@@ -14,81 +11,6 @@ static char const * const shortlog_usage[] = {
 	NULL
 };
 
-static int compare_by_number(const void *a1, const void *a2)
-{
-	const struct string_list_item *i1 = a1, *i2 = a2;
-	const struct string_list *l1 = i1->util, *l2 = i2->util;
-
-	if (l1->nr < l2->nr)
-		return 1;
-	else if (l1->nr == l2->nr)
-		return 0;
-	else
-		return -1;
-}
-
-static void insert_one_record(struct shortlog *log,
-			      const char *author,
-			      const char *oneline)
-{
-	const char *dot3 = log->common_repo_prefix;
-	char *buffer, *p;
-	struct string_list_item *item;
-	const char *mailbuf, *namebuf;
-	size_t namelen, maillen;
-	const char *eol;
-	struct strbuf subject = STRBUF_INIT;
-	struct strbuf namemailbuf = STRBUF_INIT;
-	struct ident_split ident;
-
-	if (split_ident_line(&ident, author, strlen(author)))
-		return;
-
-	namebuf = ident.name_begin;
-	mailbuf = ident.mail_begin;
-	namelen = ident.name_end - ident.name_begin;
-	maillen = ident.mail_end - ident.mail_begin;
-
-	map_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
-	strbuf_add(&namemailbuf, namebuf, namelen);
-
-	if (log->email)
-		strbuf_addf(&namemailbuf, " <%.*s>", (int)maillen, mailbuf);
-
-	item = string_list_insert(&log->list, namemailbuf.buf);
-	if (item->util == NULL)
-		item->util = xcalloc(1, sizeof(struct string_list));
-
-	/* Skip any leading whitespace, including any blank lines. */
-	while (*oneline && isspace(*oneline))
-		oneline++;
-	eol = strchr(oneline, '\n');
-	if (!eol)
-		eol = oneline + strlen(oneline);
-	if (!prefixcmp(oneline, "[PATCH")) {
-		char *eob = strchr(oneline, ']');
-		if (eob && (!eol || eob < eol))
-			oneline = eob + 1;
-	}
-	while (*oneline && isspace(*oneline) && *oneline != '\n')
-		oneline++;
-	format_subject(&subject, oneline, " ");
-	buffer = strbuf_detach(&subject, NULL);
-
-	if (dot3) {
-		int dot3len = strlen(dot3);
-		if (dot3len > 5) {
-			while ((p = strstr(buffer, dot3)) != NULL) {
-				int taillen = strlen(p) - dot3len;
-				memcpy(p, "/.../", 5);
-				memmove(p + 5, p + dot3len, taillen + 1);
-			}
-		}
-	}
-
-	string_list_append(item->util, buffer);
-}
-
 static void read_from_stdin(struct shortlog *log)
 {
 	char author[1024], oneline[1024];
@@ -103,51 +25,10 @@ static void read_from_stdin(struct shortlog *log)
 		while (fgets(oneline, sizeof(oneline), stdin) &&
 		       oneline[0] == '\n')
 			; /* discard blanks */
-		insert_one_record(log, author + 8, oneline);
+		shortlog_insert_one_record(log, author + 8, oneline);
 	}
 }
 
-void shortlog_add_commit(struct shortlog *log, struct commit *commit)
-{
-	const char *author = NULL, *buffer;
-	struct strbuf buf = STRBUF_INIT;
-	struct strbuf ufbuf = STRBUF_INIT;
-
-	pp_commit_easy(CMIT_FMT_RAW, commit, &buf);
-	buffer = buf.buf;
-	while (*buffer && *buffer != '\n') {
-		const char *eol = strchr(buffer, '\n');
-
-		if (eol == NULL)
-			eol = buffer + strlen(buffer);
-		else
-			eol++;
-
-		if (!prefixcmp(buffer, "author "))
-			author = buffer + 7;
-		buffer = eol;
-	}
-	if (!author)
-		die(_("Missing author: %s"),
-		    sha1_to_hex(commit->object.sha1));
-	if (log->user_format) {
-		struct pretty_print_context ctx = {0};
-		ctx.fmt = CMIT_FMT_USERFORMAT;
-		ctx.abbrev = log->abbrev;
-		ctx.subject = "";
-		ctx.after_subject = "";
-		ctx.date_mode = DATE_NORMAL;
-		ctx.output_encoding = get_log_output_encoding();
-		pretty_print_commit(&ctx, commit, &ufbuf);
-		buffer = ufbuf.buf;
-	} else if (*buffer) {
-		buffer++;
-	}
-	insert_one_record(log, author, !*buffer ? "<none>" : buffer);
-	strbuf_release(&ufbuf);
-	strbuf_release(&buf);
-}
-
 static void get_from_rev(struct rev_info *rev, struct shortlog *log)
 {
 	struct commit *commit;
@@ -175,9 +56,6 @@ static int parse_uint(char const **arg, int comma, int defval)
 }
 
 static const char wrap_arg_usage[] = "-w[<width>[,<indent1>[,<indent2>]]]";
-#define DEFAULT_WRAPLEN 76
-#define DEFAULT_INDENT1 6
-#define DEFAULT_INDENT2 9
 
 static int parse_wrap_args(const struct option *opt, const char *arg, int unset)
 {
@@ -205,18 +83,6 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-void shortlog_init(struct shortlog *log)
-{
-	memset(log, 0, sizeof(*log));
-
-	read_mailmap(&log->mailmap, &log->common_repo_prefix);
-
-	log->list.strdup_strings = 1;
-	log->wrap = DEFAULT_WRAPLEN;
-	log->in1 = DEFAULT_INDENT1;
-	log->in2 = DEFAULT_INDENT2;
-}
-
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
 {
 	static struct shortlog log;
@@ -277,51 +143,3 @@ parse_done:
 	shortlog_output(&log);
 	return 0;
 }
-
-static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,
-				     const struct shortlog *log)
-{
-	strbuf_add_wrapped_text(sb, s, log->in1, log->in2, log->wrap);
-	strbuf_addch(sb, '\n');
-}
-
-void shortlog_output(struct shortlog *log)
-{
-	int i, j;
-	struct strbuf sb = STRBUF_INIT;
-
-	if (log->sort_by_number)
-		qsort(log->list.items, log->list.nr, sizeof(struct string_list_item),
-			compare_by_number);
-	for (i = 0; i < log->list.nr; i++) {
-		struct string_list *onelines = log->list.items[i].util;
-
-		if (log->summary) {
-			printf("%6d\t%s\n", onelines->nr, log->list.items[i].string);
-		} else {
-			printf("%s (%d):\n", log->list.items[i].string, onelines->nr);
-			for (j = onelines->nr - 1; j >= 0; j--) {
-				const char *msg = onelines->items[j].string;
-
-				if (log->wrap_lines) {
-					strbuf_reset(&sb);
-					add_wrapped_shortlog_msg(&sb, msg, log);
-					fwrite(sb.buf, sb.len, 1, stdout);
-				}
-				else
-					printf("      %s\n", msg);
-			}
-			putchar('\n');
-		}
-
-		onelines->strdup_strings = 1;
-		string_list_clear(onelines, 0);
-		free(onelines);
-		log->list.items[i].util = NULL;
-	}
-
-	strbuf_release(&sb);
-	log->list.strdup_strings = 1;
-	string_list_clear(&log->list, 1);
-	clear_mailmap(&log->mailmap);
-}
diff --git a/shortlog.c b/shortlog.c
new file mode 100644
index 0000000..89a839f
--- /dev/null
+++ b/shortlog.c
@@ -0,0 +1,181 @@
+#include "cache.h"
+#include "shortlog.h"
+#include "commit.h"
+#include "mailmap.h"
+#include "utf8.h"
+
+void shortlog_init(struct shortlog *log)
+{
+	memset(log, 0, sizeof(*log));
+
+	read_mailmap(&log->mailmap, &log->common_repo_prefix);
+
+	log->list.strdup_strings = 1;
+	log->wrap = DEFAULT_WRAPLEN;
+	log->in1 = DEFAULT_INDENT1;
+	log->in2 = DEFAULT_INDENT2;
+}
+
+void shortlog_insert_one_record(struct shortlog *log,
+		const char *author,
+		const char *oneline)
+{
+	const char *dot3 = log->common_repo_prefix;
+	char *buffer, *p;
+	struct string_list_item *item;
+	const char *mailbuf, *namebuf;
+	size_t namelen, maillen;
+	const char *eol;
+	struct strbuf subject = STRBUF_INIT;
+	struct strbuf namemailbuf = STRBUF_INIT;
+	struct ident_split ident;
+
+	if (split_ident_line(&ident, author, strlen(author)))
+		return;
+
+	namebuf = ident.name_begin;
+	mailbuf = ident.mail_begin;
+	namelen = ident.name_end - ident.name_begin;
+	maillen = ident.mail_end - ident.mail_begin;
+
+	map_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
+	strbuf_add(&namemailbuf, namebuf, namelen);
+
+	if (log->email)
+		strbuf_addf(&namemailbuf, " <%.*s>", (int)maillen, mailbuf);
+
+	item = string_list_insert(&log->list, namemailbuf.buf);
+	if (item->util == NULL)
+		item->util = xcalloc(1, sizeof(struct string_list));
+
+	/* Skip any leading whitespace, including any blank lines. */
+	while (*oneline && isspace(*oneline))
+		oneline++;
+	eol = strchr(oneline, '\n');
+	if (!eol)
+		eol = oneline + strlen(oneline);
+	if (!prefixcmp(oneline, "[PATCH")) {
+		char *eob = strchr(oneline, ']');
+		if (eob && (!eol || eob < eol))
+			oneline = eob + 1;
+	}
+	while (*oneline && isspace(*oneline) && *oneline != '\n')
+		oneline++;
+	format_subject(&subject, oneline, " ");
+	buffer = strbuf_detach(&subject, NULL);
+
+	if (dot3) {
+		int dot3len = strlen(dot3);
+		if (dot3len > 5) {
+			while ((p = strstr(buffer, dot3)) != NULL) {
+				int taillen = strlen(p) - dot3len;
+				memcpy(p, "/.../", 5);
+				memmove(p + 5, p + dot3len, taillen + 1);
+			}
+		}
+	}
+
+	string_list_append(item->util, buffer);
+}
+
+void shortlog_add_commit(struct shortlog *log, struct commit *commit)
+{
+	const char *author = NULL, *buffer;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf ufbuf = STRBUF_INIT;
+
+	pp_commit_easy(CMIT_FMT_RAW, commit, &buf);
+	buffer = buf.buf;
+	while (*buffer && *buffer != '\n') {
+		const char *eol = strchr(buffer, '\n');
+
+		if (eol == NULL)
+			eol = buffer + strlen(buffer);
+		else
+			eol++;
+
+		if (!prefixcmp(buffer, "author "))
+			author = buffer + 7;
+		buffer = eol;
+	}
+	if (!author)
+		die(_("Missing author: %s"),
+		    sha1_to_hex(commit->object.sha1));
+	if (log->user_format) {
+		struct pretty_print_context ctx = {0};
+		ctx.fmt = CMIT_FMT_USERFORMAT;
+		ctx.abbrev = log->abbrev;
+		ctx.subject = "";
+		ctx.after_subject = "";
+		ctx.date_mode = DATE_NORMAL;
+		ctx.output_encoding = get_log_output_encoding();
+		pretty_print_commit(&ctx, commit, &ufbuf);
+		buffer = ufbuf.buf;
+	} else if (*buffer) {
+		buffer++;
+	}
+	shortlog_insert_one_record(log, author, !*buffer ? "<none>" : buffer);
+	strbuf_release(&ufbuf);
+	strbuf_release(&buf);
+}
+
+static int compare_by_number(const void *a1, const void *a2)
+{
+	const struct string_list_item *i1 = a1, *i2 = a2;
+	const struct string_list *l1 = i1->util, *l2 = i2->util;
+
+	if (l1->nr < l2->nr)
+		return 1;
+	else if (l1->nr == l2->nr)
+		return 0;
+	else
+		return -1;
+}
+
+static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,
+				     const struct shortlog *log)
+{
+	strbuf_add_wrapped_text(sb, s, log->in1, log->in2, log->wrap);
+	strbuf_addch(sb, '\n');
+}
+
+void shortlog_output(struct shortlog *log)
+{
+	int i, j;
+	struct strbuf sb = STRBUF_INIT;
+
+	if (log->sort_by_number)
+		qsort(log->list.items, log->list.nr, sizeof(struct string_list_item),
+			compare_by_number);
+	for (i = 0; i < log->list.nr; i++) {
+		struct string_list *onelines = log->list.items[i].util;
+
+		if (log->summary) {
+			printf("%6d\t%s\n", onelines->nr, log->list.items[i].string);
+		} else {
+			printf("%s (%d):\n", log->list.items[i].string, onelines->nr);
+			for (j = onelines->nr - 1; j >= 0; j--) {
+				const char *msg = onelines->items[j].string;
+
+				if (log->wrap_lines) {
+					strbuf_reset(&sb);
+					add_wrapped_shortlog_msg(&sb, msg, log);
+					fwrite(sb.buf, sb.len, 1, stdout);
+				}
+				else
+					printf("      %s\n", msg);
+			}
+			putchar('\n');
+		}
+
+		onelines->strdup_strings = 1;
+		string_list_clear(onelines, 0);
+		free(onelines);
+		log->list.items[i].util = NULL;
+	}
+
+	strbuf_release(&sb);
+	log->list.strdup_strings = 1;
+	string_list_clear(&log->list, 1);
+	clear_mailmap(&log->mailmap);
+}
diff --git a/shortlog.h b/shortlog.h
index 54bc07c..cf14d8b 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -3,6 +3,10 @@
 
 #include "string-list.h"
 
+#define DEFAULT_WRAPLEN 76
+#define DEFAULT_INDENT1 6
+#define DEFAULT_INDENT2 9
+
 struct shortlog {
 	struct string_list list;
 	int summary;
@@ -25,6 +29,8 @@ void shortlog_init(struct shortlog *log);
 
 void shortlog_add_commit(struct shortlog *log, struct commit *commit);
 
+void shortlog_insert_one_record(struct shortlog *log, const char *author, const char *oneline);
+
 void shortlog_output(struct shortlog *log);
 
 #endif
-- 
1.8.4-fc

[PATCH v2 35/44] ruby: add RevInfo and DiffOptions

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    This time we use an initializer and alloc function, so we could do:
    
     ri = Git::RevInfo.new()
     ri.setup(args)
    
    Or:
    
     ri = Git::RevInfo.setup(args)
    
    The effect is the same.
    
    Also, we use the extremely useful 'method_missing' method, which allows us to
    do the binding at run-time, saving us precious finger typing time and code,
    with price of a bit of performance loss, which is not that important in a
    script like this.
    
    We could do something fancier like saving the Ruby internal identifiers in an
    array to map them to a C enum, or we could even generate the missing methods at
    run-time, but for now let's stick to the simpler option and just make it work
    with the less possible code.

 ruby.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)

diff --git a/ruby.c b/ruby.c
index 360515f..d9ee985 100644
--- a/ruby.c
+++ b/ruby.c
@@ -5,6 +5,8 @@
 #include "commit.h"
 #include "remote.h"
 #include "transport.h"
+#include "revision.h"
+#include "diff.h"
 
 #undef NORETURN
 #undef PATH_SEP
@@ -17,6 +19,8 @@ static VALUE git_rb_commit_list;
 static VALUE git_rb_remote;
 static VALUE git_rb_transport;
 static VALUE git_rb_ref;
+static VALUE git_rb_rev_info;
+static VALUE git_rb_diff_opt;
 
 static inline VALUE sha1_to_str(const unsigned char *sha1)
 {
@@ -277,6 +281,107 @@ static VALUE git_rb_read_sha1_file(VALUE self, VALUE sha1, VALUE type)
 	return rb_ary_new3(2, rb_str_new(buffer, size), INT2FIX(g_type));
 }
 
+static VALUE git_rb_rev_info_alloc(VALUE class)
+{
+	struct rev_info *revs;
+	return Data_Make_Struct(class, struct rev_info, NULL, free, revs);
+}
+
+static VALUE git_rb_rev_info_init(VALUE self, VALUE prefix)
+{
+	struct rev_info *revs;
+	Data_Get_Struct(self, struct rev_info, revs);
+	init_revisions(revs, str_to_cstr(prefix));
+	return self;
+}
+
+static VALUE git_rb_rev_info_setup(VALUE self, VALUE args, VALUE opts)
+{
+	struct rev_info *revs;
+	const char *argv[RARRAY_LEN(args) + 2];
+	int i, r;
+
+	argv[0] = "";
+	Data_Get_Struct(self, struct rev_info, revs);
+	for (i = 0; i < RARRAY_LEN(args); i++)
+		argv[i + 1] = RSTRING_PTR(RARRAY_PTR(args)[i]);
+	argv[i + 1] = NULL;
+	r = setup_revisions(RARRAY_LEN(args) + 1, argv, revs, NULL);
+	return INT2FIX(r - 1);
+}
+
+static VALUE git_rb_rev_info_single_setup(VALUE class, VALUE prefix, VALUE args, VALUE opts)
+{
+	struct rev_info *revs;
+	VALUE self;
+	self = Data_Make_Struct(class, struct rev_info, NULL, free, revs);
+	init_revisions(revs, str_to_cstr(prefix));
+	git_rb_rev_info_setup(self, args, opts);
+	return self;
+}
+
+static VALUE git_rb_rev_info_each_revision(VALUE self, VALUE args, VALUE opts)
+{
+	struct commit *commit;
+	struct rev_info *revs;
+
+	Data_Get_Struct(self, struct rev_info, revs);
+	if (prepare_revision_walk(revs))
+		return Qnil;
+	while ((commit = get_revision(revs))) {
+		VALUE c;
+		c = Data_Wrap_Struct(git_rb_commit, NULL, NULL, commit);
+		rb_yield(c);
+	}
+	return Qnil;
+}
+
+static VALUE git_rb_rev_info_diffopt(VALUE self)
+{
+	struct rev_info *revs;
+
+	Data_Get_Struct(self, struct rev_info, revs);
+	return Data_Wrap_Struct(git_rb_diff_opt, NULL, NULL, &revs->diffopt);
+}
+
+static VALUE git_rb_diff_opt_new(VALUE class)
+{
+	struct diff_options *opt;
+	VALUE self;
+	self = Data_Make_Struct(class, struct diff_options, NULL, free, opt);
+
+	diff_setup(opt);
+
+	return self;
+}
+
+static VALUE git_rb_diff_opt_method_missing(int argc, VALUE *argv, VALUE self)
+{
+	struct diff_options *opt;
+	ID id;
+
+	id = rb_to_id(argv[0]);
+	Data_Get_Struct(self, struct diff_options, opt);
+
+	if (id == rb_intern("stat_width="))
+		opt->stat_width = NUM2INT(argv[1]);
+	else if (id == rb_intern("stat_graph_width="))
+		opt->stat_graph_width = NUM2INT(argv[1]);
+	else if (id == rb_intern("output_format="))
+		opt->output_format = NUM2INT(argv[1]);
+	else if (id == rb_intern("detect_rename="))
+		opt->detect_rename = NUM2INT(argv[1]);
+	else if (id == rb_intern("flags="))
+		opt->flags = NUM2INT(argv[1]);
+	else if (id == rb_intern("output_format"))
+		return INT2FIX(opt->output_format);
+	else if (id == rb_intern("flags"))
+		return INT2FIX(opt->flags);
+	else
+		return rb_call_super(argc, argv);
+	return Qnil;
+}
+
 static void git_ruby_init(void)
 {
 	VALUE mod;
@@ -296,6 +401,53 @@ static void git_ruby_init(void)
 
 	rb_define_global_const("DEFAULT_ABBREV", INT2FIX(DEFAULT_ABBREV));
 
+	rb_define_global_const("DIFF_FORMAT_RAW", INT2FIX(DIFF_FORMAT_RAW));
+	rb_define_global_const("DIFF_FORMAT_DIFFSTAT", INT2FIX(DIFF_FORMAT_DIFFSTAT));
+	rb_define_global_const("DIFF_FORMAT_NUMSTAT", INT2FIX(DIFF_FORMAT_NUMSTAT));
+	rb_define_global_const("DIFF_FORMAT_SUMMARY", INT2FIX(DIFF_FORMAT_SUMMARY));
+	rb_define_global_const("DIFF_FORMAT_PATCH", INT2FIX(DIFF_FORMAT_PATCH));
+	rb_define_global_const("DIFF_FORMAT_SHORTSTAT", INT2FIX(DIFF_FORMAT_SHORTSTAT));
+	rb_define_global_const("DIFF_FORMAT_DIRSTAT", INT2FIX(DIFF_FORMAT_DIRSTAT));
+	rb_define_global_const("DIFF_FORMAT_NAME", INT2FIX(DIFF_FORMAT_NAME));
+	rb_define_global_const("DIFF_FORMAT_NAME_STATUS", INT2FIX(DIFF_FORMAT_NAME_STATUS));
+	rb_define_global_const("DIFF_FORMAT_CHECKDIFF", INT2FIX(DIFF_FORMAT_CHECKDIFF));
+	rb_define_global_const("DIFF_FORMAT_NO_OUTPUT", INT2FIX(DIFF_FORMAT_NO_OUTPUT));
+	rb_define_global_const("DIFF_FORMAT_CALLBACK", INT2FIX(DIFF_FORMAT_CALLBACK));
+
+	rb_define_global_const("DIFF_DETECT_RENAME", INT2FIX(DIFF_DETECT_RENAME));
+	rb_define_global_const("DIFF_DETECT_COPY", INT2FIX(DIFF_DETECT_COPY));
+
+	rb_define_global_const("DIFF_OPT_RECURSIVE", INT2FIX(DIFF_OPT_RECURSIVE));
+	rb_define_global_const("DIFF_OPT_TREE_IN_RECURSIVE", INT2FIX(DIFF_OPT_TREE_IN_RECURSIVE));
+	rb_define_global_const("DIFF_OPT_BINARY", INT2FIX(DIFF_OPT_BINARY));
+	rb_define_global_const("DIFF_OPT_TEXT", INT2FIX(DIFF_OPT_TEXT));
+	rb_define_global_const("DIFF_OPT_FULL_INDEX", INT2FIX(DIFF_OPT_FULL_INDEX));
+	rb_define_global_const("DIFF_OPT_SILENT_ON_REMOVE", INT2FIX(DIFF_OPT_SILENT_ON_REMOVE));
+	rb_define_global_const("DIFF_OPT_FIND_COPIES_HARDER", INT2FIX(DIFF_OPT_FIND_COPIES_HARDER));
+	rb_define_global_const("DIFF_OPT_FOLLOW_RENAMES", INT2FIX(DIFF_OPT_FOLLOW_RENAMES));
+	rb_define_global_const("DIFF_OPT_RENAME_EMPTY", INT2FIX(DIFF_OPT_RENAME_EMPTY));
+	rb_define_global_const("DIFF_OPT_HAS_CHANGES", INT2FIX(DIFF_OPT_HAS_CHANGES));
+	rb_define_global_const("DIFF_OPT_QUICK", INT2FIX(DIFF_OPT_QUICK));
+	rb_define_global_const("DIFF_OPT_NO_INDEX", INT2FIX(DIFF_OPT_NO_INDEX));
+	rb_define_global_const("DIFF_OPT_ALLOW_EXTERNAL", INT2FIX(DIFF_OPT_ALLOW_EXTERNAL));
+	rb_define_global_const("DIFF_OPT_EXIT_WITH_STATUS", INT2FIX(DIFF_OPT_EXIT_WITH_STATUS));
+	rb_define_global_const("DIFF_OPT_REVERSE_DIFF", INT2FIX(DIFF_OPT_REVERSE_DIFF));
+	rb_define_global_const("DIFF_OPT_CHECK_FAILED", INT2FIX(DIFF_OPT_CHECK_FAILED));
+	rb_define_global_const("DIFF_OPT_RELATIVE_NAME", INT2FIX(DIFF_OPT_RELATIVE_NAME));
+	rb_define_global_const("DIFF_OPT_IGNORE_SUBMODULES", INT2FIX(DIFF_OPT_IGNORE_SUBMODULES));
+	rb_define_global_const("DIFF_OPT_DIRSTAT_CUMULATIVE", INT2FIX(DIFF_OPT_DIRSTAT_CUMULATIVE));
+	rb_define_global_const("DIFF_OPT_DIRSTAT_BY_FILE", INT2FIX(DIFF_OPT_DIRSTAT_BY_FILE));
+	rb_define_global_const("DIFF_OPT_ALLOW_TEXTCONV", INT2FIX(DIFF_OPT_ALLOW_TEXTCONV));
+	rb_define_global_const("DIFF_OPT_DIFF_FROM_CONTENTS", INT2FIX(DIFF_OPT_DIFF_FROM_CONTENTS));
+	rb_define_global_const("DIFF_OPT_SUBMODULE_LOG", INT2FIX(DIFF_OPT_SUBMODULE_LOG));
+	rb_define_global_const("DIFF_OPT_DIRTY_SUBMODULES", INT2FIX(DIFF_OPT_DIRTY_SUBMODULES));
+	rb_define_global_const("DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES", INT2FIX(DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES));
+	rb_define_global_const("DIFF_OPT_IGNORE_DIRTY_SUBMODULES", INT2FIX(DIFF_OPT_IGNORE_DIRTY_SUBMODULES));
+	rb_define_global_const("DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG", INT2FIX(DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG));
+	rb_define_global_const("DIFF_OPT_DIRSTAT_BY_LINE", INT2FIX(DIFF_OPT_DIRSTAT_BY_LINE));
+	rb_define_global_const("DIFF_OPT_FUNCCONTEXT", INT2FIX(DIFF_OPT_FUNCCONTEXT));
+	rb_define_global_const("DIFF_OPT_PICKAXE_IGNORE_CASE", INT2FIX(DIFF_OPT_PICKAXE_IGNORE_CASE));
+
 	rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
 	rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
 	rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
@@ -334,6 +486,19 @@ static void git_ruby_init(void)
 	rb_define_method(git_rb_ref, "each", git_rb_ref_each, 0);
 	rb_define_method(git_rb_ref, "name", git_rb_ref_name, 0);
 	rb_define_method(git_rb_ref, "old_sha1", git_rb_ref_old_sha1, 0);
+
+	git_rb_rev_info = rb_define_class_under(mod, "RevInfo", rb_cData);
+	rb_include_module(git_rb_rev_info, rb_mEnumerable);
+	rb_define_alloc_func(git_rb_rev_info, git_rb_rev_info_alloc);
+	rb_define_method(git_rb_rev_info, "initialize", git_rb_rev_info_init, 1);
+	rb_define_singleton_method(git_rb_rev_info, "setup", git_rb_rev_info_single_setup, 3);
+	rb_define_method(git_rb_rev_info, "setup", git_rb_rev_info_setup, 2);
+	rb_define_method(git_rb_rev_info, "each", git_rb_rev_info_each_revision, 0);
+	rb_define_method(git_rb_rev_info, "diffopt", git_rb_rev_info_diffopt, 0);
+
+	git_rb_diff_opt = rb_define_class_under(mod, "DiffOptions", rb_cData);
+	rb_define_singleton_method(git_rb_diff_opt, "new", git_rb_diff_opt_new, 0);
+	rb_define_method(git_rb_diff_opt, "method_missing", git_rb_diff_opt_method_missing, -1);
 }
 
 static int run_ruby_command(const char *cmd, int argc, const char **argv)
-- 
1.8.4-fc

[PATCH v2 36/44] ruby: bind shortlog()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    There's no need to bind all the shortlog functions, we can easily pass all the
    commits we want in Ruby.

 ruby.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ruby.c b/ruby.c
index d9ee985..d59aafd 100644
--- a/ruby.c
+++ b/ruby.c
@@ -7,6 +7,7 @@
 #include "transport.h"
 #include "revision.h"
 #include "diff.h"
+#include "shortlog.h"
 
 #undef NORETURN
 #undef PATH_SEP
@@ -344,6 +345,21 @@ static VALUE git_rb_rev_info_diffopt(VALUE self)
 	return Data_Wrap_Struct(git_rb_diff_opt, NULL, NULL, &revs->diffopt);
 }
 
+static VALUE git_rb_shortlog(VALUE self, VALUE commits)
+{
+	struct shortlog log;
+	int i;
+
+	shortlog_init(&log);
+	for (i = 0; i < RARRAY_LEN(commits); i++) {
+		struct commit *commit;
+		Data_Get_Struct(rb_ary_entry(commits, i), struct commit, commit);
+		shortlog_add_commit(&log, commit);
+	}
+	shortlog_output(&log);
+	return Qnil;
+}
+
 static VALUE git_rb_diff_opt_new(VALUE class)
 {
 	struct diff_options *opt;
@@ -460,6 +476,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("transport_get", git_rb_transport_get, 2);
 	rb_define_global_function("find_unique_abbrev", git_rb_find_unique_abbrev, 2);
 	rb_define_global_function("read_sha1_file", git_rb_read_sha1_file, 1);
+	rb_define_global_function("shortlog", git_rb_shortlog, 1);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
-- 
1.8.4-fc

[PATCH v2 37/44] ruby: request-pull: use shortlog()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 233981f..0db64c3 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -79,6 +79,11 @@ def parse_buffer(buffer)
   return [summary, date]
 end
 
+def show_shortlog(base, head)
+  rev = Git::RevInfo.setup(nil, ['^' + base, head], nil)
+  shortlog(rev.to_a)
+end
+
 until ARGV.empty?
   case ARGV.first
   when '-p'
@@ -174,7 +179,7 @@ for you to fetch changes up to %s:
     puts "----------------------------------------------------------------"
   end
 
-  run(%[git shortlog ^#{base} #{head}])
+  show_shortlog(base, head)
   run(%[git diff -M --stat --summary #{patch} ^#{merge_base_commit} #{head}])
 
   if ! ref
-- 
1.8.4-fc

[PATCH v2 38/44] ruby: bind diff_tree_sha1()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/ruby.c b/ruby.c
index d59aafd..b491754 100644
--- a/ruby.c
+++ b/ruby.c
@@ -360,6 +360,17 @@ static VALUE git_rb_shortlog(VALUE self, VALUE commits)
 	return Qnil;
 }
 
+static VALUE git_rb_diff_tree_sha1(VALUE self, VALUE old, VALUE new, VALUE base, VALUE opt)
+{
+	struct diff_options *g_opt;
+	int r;
+
+	Data_Get_Struct(opt, struct diff_options, g_opt);
+
+	r = diff_tree_sha1(str_to_sha1(old), str_to_sha1(new), str_to_cstr(base), g_opt);
+	return INT2FIX(r);
+}
+
 static VALUE git_rb_diff_opt_new(VALUE class)
 {
 	struct diff_options *opt;
@@ -477,6 +488,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("find_unique_abbrev", git_rb_find_unique_abbrev, 2);
 	rb_define_global_function("read_sha1_file", git_rb_read_sha1_file, 1);
 	rb_define_global_function("shortlog", git_rb_shortlog, 1);
+	rb_define_global_function("diff_tree_sha1", git_rb_diff_tree_sha1, 4);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
-- 
1.8.4-fc

[PATCH v2 39/44] ruby: bind log_tree_diff_flush()

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ruby.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/ruby.c b/ruby.c
index b491754..71d5cb6 100644
--- a/ruby.c
+++ b/ruby.c
@@ -8,6 +8,7 @@
 #include "revision.h"
 #include "diff.h"
 #include "shortlog.h"
+#include "log-tree.h"
 
 #undef NORETURN
 #undef PATH_SEP
@@ -371,6 +372,16 @@ static VALUE git_rb_diff_tree_sha1(VALUE self, VALUE old, VALUE new, VALUE base,
 	return INT2FIX(r);
 }
 
+static VALUE git_rb_log_tree_diff_flush(VALUE self, VALUE revs)
+{
+	struct rev_info *g_revs;
+	int r;
+
+	Data_Get_Struct(revs, struct rev_info, g_revs);
+	r = log_tree_diff_flush(g_revs);
+	return INT2FIX(r);
+}
+
 static VALUE git_rb_diff_opt_new(VALUE class)
 {
 	struct diff_options *opt;
@@ -489,6 +500,7 @@ static void git_ruby_init(void)
 	rb_define_global_function("read_sha1_file", git_rb_read_sha1_file, 1);
 	rb_define_global_function("shortlog", git_rb_shortlog, 1);
 	rb_define_global_function("diff_tree_sha1", git_rb_diff_tree_sha1, 4);
+	rb_define_global_function("log_tree_diff_flush", git_rb_log_tree_diff_flush, 1);
 
 	git_rb_object = rb_define_class_under(mod, "Object", rb_cData);
 	rb_define_singleton_method(git_rb_object, "get", git_rb_object_get, 1);
-- 
1.8.4-fc

[PATCH v2 40/44] ruby: request-pull: use native diff_tree stuff

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    We finally got rid of all the system() calls, and thus all the forks.

 git-request-pull.rb | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index 0db64c3..e884d0d 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -4,7 +4,7 @@ require 'date'
 
 ENV['GIT_PAGER'] =
 
-patch = ''
+patch = nil
 
 def usage
   puts <<EOF
@@ -84,6 +84,19 @@ def show_shortlog(base, head)
   shortlog(rev.to_a)
 end
 
+def show_diff(patch, base, head)
+  rev = Git::RevInfo.setup(nil, ['^' + sha1_to_hex(base), sha1_to_hex(head)], nil)
+  rev.diffopt.stat_width = -1
+  rev.diffopt.stat_graph_width = -1
+  rev.diffopt.output_format = patch ? DIFF_FORMAT_PATCH : DIFF_FORMAT_DIFFSTAT
+  rev.diffopt.output_format |= DIFF_FORMAT_SUMMARY
+  rev.diffopt.detect_rename = DIFF_DETECT_RENAME
+  rev.diffopt.flags |= DIFF_OPT_RECURSIVE
+
+  diff_tree_sha1(base, head, "", rev.diffopt)
+  log_tree_diff_flush(rev)
+end
+
 until ARGV.empty?
   case ARGV.first
   when '-p'
@@ -180,7 +193,7 @@ for you to fetch changes up to %s:
   end
 
   show_shortlog(base, head)
-  run(%[git diff -M --stat --summary #{patch} ^#{merge_base_commit} #{head}])
+  show_diff(patch, merge_base_id, head_id)
 
   if ! ref
     $stderr.puts "warn: No branch of #{url} is at:"
-- 
1.8.4-fc

[PATCH v2 41/44] ruby: request-pull: remove rescue block

[Felipe Contreras]

We are not calling any git commands any more.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 61 ++++++++++++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index e884d0d..fde8d1a 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -115,7 +115,6 @@ end
 base = ARGV[0]
 url = ARGV[1]
 head = ARGV[2] || 'HEAD'
-status = 0
 branch_name = branch_desc = nil
 
 usage unless base or url
@@ -154,16 +153,15 @@ url = remote.url.first
 merge_base_summary, merge_base_date = parse_buffer(merge_base_commit.buffer)
 head_summary, head_date = parse_buffer(head_commit.buffer)
 
-begin
-  puts "The following changes since commit %s:
+puts "The following changes since commit %s:
 
   %s (%s)
 
 are available in the git repository at:
 
 " % [merge_base_commit, merge_base_summary, merge_base_date]
-  puts "  #{url}" + (ref ? " #{ref}" : "")
-  puts "
+puts "  #{url}" + (ref ? " #{ref}" : "")
+puts "
 for you to fetch changes up to %s:
 
   %s (%s)
@@ -171,38 +169,33 @@ for you to fetch changes up to %s:
 ----------------------------------------------------------------
 " % [head_commit, head_summary, head_date]
 
-  if branch_name
-    puts "(from the branch description for #{branch_name} local branch)"
-    puts
-    puts branch_desc
-  end
+if branch_name
+  puts "(from the branch description for #{branch_name} local branch)"
+  puts
+  puts branch_desc
+end
 
-  if tag_name
-    if ref != "tags/#{tag_name}"
-      $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
-      $stderr.puts "warn: appear to be at #{url}"
-      $stderr.puts "warn: Do you want to push it there, perhaps?"
-    end
-    buffer, _ = read_sha1_file(get_sha1(tag_name))
-    puts buffer.scan(/(?:\n\n)(.+)(?:-----BEGIN PGP )?/m).first
-    puts
+if tag_name
+  if ref != "tags/#{tag_name}"
+    $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
+    $stderr.puts "warn: appear to be at #{url}"
+    $stderr.puts "warn: Do you want to push it there, perhaps?"
   end
+  buffer, _ = read_sha1_file(get_sha1(tag_name))
+  puts buffer.scan(/(?:\n\n)(.+)(?:-----BEGIN PGP )?/m).first
+  puts
+end
 
-  if branch_name || tag_name
-    puts "----------------------------------------------------------------"
-  end
+if branch_name || tag_name
+  puts "----------------------------------------------------------------"
+end
 
-  show_shortlog(base, head)
-  show_diff(patch, merge_base_id, head_id)
+show_shortlog(base, head)
+show_diff(patch, merge_base_id, head_id)
 
-  if ! ref
-    $stderr.puts "warn: No branch of #{url} is at:"
-    $stderr.puts "warn:   %s: %s'" % [find_unique_abbrev(head_id, DEFAULT_ABBREV), head_summary]
-    $stderr.puts "warn: Are you sure you pushed '#{abbr(head_ref)}' there?"
-    status = 1
-  end
-rescue CommandError
-  status = 1
+if ! ref
+  $stderr.puts "warn: No branch of #{url} is at:"
+  $stderr.puts "warn:   %s: %s'" % [find_unique_abbrev(head_id, DEFAULT_ABBREV), head_summary]
+  $stderr.puts "warn: Are you sure you pushed '#{abbr(head_ref)}' there?"
+  exit 1
 end
-
-exit status
-- 
1.8.4-fc

[PATCH v2 42/44] ruby: remove GIT_PAGER from environment

[Felipe Contreras]

We are not calling any git commands any more.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-request-pull.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/git-request-pull.rb b/git-request-pull.rb
index fde8d1a..941ff72 100755
--- a/git-request-pull.rb
+++ b/git-request-pull.rb
@@ -2,8 +2,6 @@
 
 require 'date'
 
-ENV['GIT_PAGER'] =
-
 patch = nil
 
 def usage
-- 
1.8.4-fc

[PATCH v2 43/44] ruby: add simpler option parser

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Notes:
    We could use Git's internap parseopt, but the resulting code in Ruby wouldn't
    look very good, and the complexity to bind it wouldn't be trivial either (I
    tried).
    
    Instead, let's use the advantage of Ruby blocks to set the variables to their
    right values and try to keep the code simple, and add more features as they are
    needed later on.

 git-rb-setup.rb  |  75 ++++++++++++++++++++++++++++++++
 t/t10000-ruby.sh | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 203 insertions(+)

diff --git a/git-rb-setup.rb b/git-rb-setup.rb
index 6f283da..cd2502f 100644
--- a/git-rb-setup.rb
+++ b/git-rb-setup.rb
@@ -32,3 +32,78 @@ class String
     return self[prefix.length..-1]
   end
 end
+
+class ParseOpt
+  attr_writer :usage
+
+  class Option
+    attr_reader :short, :long, :help
+
+    def initialize(short, long, help, &block)
+      @block = block
+      @short = short
+      @long = long
+      @help = help
+    end
+
+    def call(v)
+      @block.call(v)
+    end
+  end
+
+  def initialize
+    @list = {}
+  end
+
+  def on(short = nil, long = nil, help: nil, &block)
+    opt = Option.new(short, long, help, &block)
+    @list[short] = opt if short
+    @list[long] = opt if long
+  end
+
+  def parse
+    if ARGV.member?('-h') or ARGV.member?('--help')
+      usage
+      exit 0
+    end
+    seen_dash = false
+    ARGV.delete_if do |cur|
+      opt = val = nil
+      next false if cur[0] != '-' or seen_dash
+      case cur
+      when '--'
+        seen_dash = true
+        next true
+      when /^--no-(.+)$/
+        opt = @list[$1]
+        val = false
+      when /^-([^-])(.+)?$/, /^--(.+?)(?:=(.+))?$/
+        opt = @list[$1]
+        val = $2 || true
+      end
+      if opt
+        opt.call(val)
+        true
+      else
+        usage
+        exit 1
+      end
+    end
+  end
+
+  def usage
+    def fmt(prefix, str)
+      return str ? prefix + str : nil
+    end
+    puts 'usage: %s' % @usage
+    @list.values.uniq.each do |opt|
+      s = '    '
+      s << ''
+      s << [fmt('-', opt.short), fmt('--', opt.long)].compact.join(', ')
+      s << ''
+      s << '%*s%s' % [26 - s.size, '', opt.help] if opt.help
+      puts s
+    end
+  end
+
+end
diff --git a/t/t10000-ruby.sh b/t/t10000-ruby.sh
index e5a397c..a365849 100755
--- a/t/t10000-ruby.sh
+++ b/t/t10000-ruby.sh
@@ -102,4 +102,132 @@ test_expect_success 'test Commit' '
 	test_cmp expected actual
 '
 
+test_expect_success 'test ParseOpt' '
+	cat > parse-script <<"EOF"
+	$str = "default"
+	$num = 0
+	$bool = false
+
+	opts = ParseOpt.new
+	opts.usage = "git foo"
+
+	opts.on("b", "bool", help: "Boolean") do |v|
+	  $bool = v
+	end
+
+	opts.on("s", "string", help: "String") do |v|
+	  $str = v
+	end
+
+	opts.on("n", "number", help: "Number") do |v|
+	  $num = v.to_i
+	end
+
+	opts.parse
+
+	p(ARGV)
+	p({ :bool => $bool, :str => $str, :num => $num })
+	EOF
+
+	git ruby parse-script > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>false, :str=>"default", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --bool > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>true, :str=>"default", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script -b > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>true, :str=>"default", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --string=foo > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>false, :str=>"foo", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script -sfoo > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>false, :str=>"foo", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --number=10 > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>false, :str=>"default", :num=>10}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --bool --string=bar --number=-20 > actual &&
+	cat > expected <<-EOF &&
+	[]
+	{:bool=>true, :str=>"bar", :num=>-20}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --help > actual &&
+	cat > expected <<-EOF &&
+	usage: git foo
+	    -b, --bool            Boolean
+	    -s, --string          String
+	    -n, --number          Number
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script --help > actual &&
+	cat > expected <<-EOF &&
+	usage: git foo
+	    -b, --bool            Boolean
+	    -s, --string          String
+	    -n, --number          Number
+	EOF
+	test_cmp expected actual &&
+
+	test_must_fail git ruby parse-script --bad > actual &&
+	cat > expected <<-EOF &&
+	usage: git foo
+	    -b, --bool            Boolean
+	    -s, --string          String
+	    -n, --number          Number
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script one --bool two --string=bar three --number=-20 mambo > actual &&
+	cat > expected <<-EOF &&
+	["one", "two", "three", "mambo"]
+	{:bool=>true, :str=>"bar", :num=>-20}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script one --bool two -- --three four > actual &&
+	cat > expected <<-EOF &&
+	["one", "two", "--three", "four"]
+	{:bool=>true, :str=>"default", :num=>0}
+	EOF
+	test_cmp expected actual &&
+
+	git ruby parse-script one --bool --no-bool > actual &&
+	cat > expected <<-EOF &&
+	["one"]
+	{:bool=>false, :str=>"default", :num=>0}
+	EOF
+	cat actual
+	test_cmp expected actual &&
+
+	true
+'
+
 test_done
-- 
1.8.4-fc

[PATCH v2 44/44] request-pull: rewrite to C

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile               |   3 +-
 builtin.h              |   1 +
 builtin/request-pull.c | 275 +++++++++++++++++++++++++++++++++++++++++++++++++
 git-request-pull.rb    | 199 -----------------------------------
 git.c                  |   1 +
 5 files changed, 278 insertions(+), 201 deletions(-)
 create mode 100644 builtin/request-pull.c
 delete mode 100755 git-request-pull.rb

diff --git a/Makefile b/Makefile
index 709c087..edbceea 100644
--- a/Makefile
+++ b/Makefile
@@ -492,9 +492,7 @@ SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
 SCRIPT_RUBY += git-rb-setup.rb
-SCRIPT_RUBY += git-request-pull.rb
 
-RUBY_PROGRAMS += git-request-pull$X
 PROGRAMS += $(RUBY_PROGRAMS)
 
 NO_INSTALL += git-remote-testgit
@@ -983,6 +981,7 @@ BUILTIN_OBJS += builtin/remote.o
 BUILTIN_OBJS += builtin/remote-ext.o
 BUILTIN_OBJS += builtin/remote-fd.o
 BUILTIN_OBJS += builtin/replace.o
+BUILTIN_OBJS += builtin/request-pull.o
 BUILTIN_OBJS += builtin/rerere.o
 BUILTIN_OBJS += builtin/reset.o
 BUILTIN_OBJS += builtin/rev-list.o
diff --git a/builtin.h b/builtin.h
index 8afa2de..7db356e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -103,6 +103,7 @@ extern int cmd_remote(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
 extern int cmd_repo_config(int argc, const char **argv, const char *prefix);
+extern int cmd_request_pull(int argc, const char **argv, const char *prefix);
 extern int cmd_rerere(int argc, const char **argv, const char *prefix);
 extern int cmd_reset(int argc, const char **argv, const char *prefix);
 extern int cmd_rev_list(int argc, const char **argv, const char *prefix);
diff --git a/builtin/request-pull.c b/builtin/request-pull.c
new file mode 100644
index 0000000..a8f7f04
--- /dev/null
+++ b/builtin/request-pull.c
@@ -0,0 +1,275 @@
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "revision.h"
+#include "refs.h"
+#include "remote.h"
+#include "transport.h"
+#include "branch.h"
+#include "shortlog.h"
+#include "diff.h"
+#include "log-tree.h"
+
+static const char *tag_name;
+
+static const char *const pull_request_usage[] = {
+	N_("git request-pull [options] start url [end]"),
+	NULL
+};
+
+static int describe_cb(const char *name, const unsigned char *sha1, int flags, void *cb_data)
+{
+	unsigned char peel_sha1[20];
+	if (prefixcmp(name, "refs/tags/"))
+		return 0;
+	peel_ref(name, peel_sha1);
+	if (hashcmp(peel_sha1, cb_data))
+		return 0;
+	tag_name = skip_prefix(name, "refs/tags/");
+	return 1;
+}
+
+const char *describe(const char *head)
+{
+	unsigned char sha1[20];
+	get_sha1(head, sha1);
+	for_each_ref(describe_cb, sha1);
+	return tag_name;
+}
+
+const char *abbr(const char *name)
+{
+	return name + (
+			!prefixcmp(name, "refs/heads/") ? 11 :
+			!prefixcmp(name, "refs/tags/") ? 5 :
+			0);
+}
+
+const char *get_ref(struct transport *transport, const char *head_ref, const unsigned char *head_id)
+{
+	const struct ref *refs, *e;
+	const char *found = NULL;
+	int deref;
+
+	refs = transport_get_remote_refs(transport);
+
+	for (e = refs; e; e = e->next) {
+		if (hashcmp(e->old_sha1, head_id))
+			continue;
+
+		deref = !suffixcmp(e->name, "^{}");
+		found = abbr(e->name);
+		if (deref && tag_name && !prefixcmp(e->name + 10, tag_name))
+			break;
+		if (head_ref && !strcmp(e->name, head_ref))
+			break;
+	}
+	if (!found)
+		return NULL;
+	return xstrndup(found, strlen(found) - (deref ? 3 : 0));
+}
+
+static const char *show_ident_date(const struct ident_split *ident,
+				   enum date_mode mode)
+{
+	unsigned long date = 0;
+	int tz = 0;
+
+	if (ident->date_begin && ident->date_end)
+		date = strtoul(ident->date_begin, NULL, 10);
+	if (ident->tz_begin && ident->tz_end)
+		tz = strtol(ident->tz_begin, NULL, 10);
+	return show_date(date, tz, mode);
+}
+
+static void parse_buffer(const char *buffer, const char **summary, const char **date)
+{
+	struct strbuf subject = STRBUF_INIT;
+	struct ident_split s;
+	const char *c, *e;
+
+	c = strstr(buffer, "\ncommitter ") + 11;
+	e = strchr(c, '\n');
+	if (!split_ident_line(&s, c, e - c))
+		*date = show_ident_date(&s, DATE_ISO8601);
+
+	c = strstr(c, "\n\n") + 2;
+	format_subject(&subject, c, " ");
+	*summary = strbuf_detach(&subject, NULL);
+}
+
+static void show_shortlog(const char *base, const char *head)
+{
+	struct commit *commit;
+	struct rev_info revs;
+	struct shortlog log;
+	const char *args[3];
+	struct strbuf tmp = STRBUF_INIT;
+
+	strbuf_addf(&tmp, "^%s", base);
+
+	args[1] = tmp.buf;
+	args[2] = head;
+
+	init_revisions(&revs, NULL);
+	setup_revisions(3, args, &revs, NULL);
+
+	strbuf_release(&tmp);
+
+	shortlog_init(&log);
+	prepare_revision_walk(&revs);
+	while ((commit = get_revision(&revs)))
+		shortlog_add_commit(&log, commit);
+	shortlog_output(&log);
+}
+
+static void show_diff(int patch, const unsigned char *base, const unsigned char *head)
+{
+	struct rev_info revs;
+	const char *args[3];
+	struct strbuf tmp = STRBUF_INIT;
+
+	strbuf_addf(&tmp, "^%s", sha1_to_hex(base));
+
+	args[1] = tmp.buf;
+	args[2] = sha1_to_hex(head);
+
+	init_revisions(&revs, NULL);
+	setup_revisions(3, args, &revs, NULL);
+	revs.diffopt.stat_width = -1;
+	revs.diffopt.stat_graph_width = -1;
+	revs.diffopt.output_format = patch ? DIFF_FORMAT_PATCH : DIFF_FORMAT_DIFFSTAT;
+	revs.diffopt.output_format |= DIFF_FORMAT_SUMMARY;
+	revs.diffopt.detect_rename = DIFF_DETECT_RENAME;
+	revs.diffopt.flags |= DIFF_OPT_RECURSIVE;
+
+	strbuf_release(&tmp);
+	diff_tree_sha1(base, head, "", &revs.diffopt);
+	log_tree_diff_flush(&revs);
+}
+
+int cmd_request_pull(int argc, const char **argv, const char *prefix)
+{
+	int patch;
+	const char *base, *url, *head;
+	char *head_ref;
+	char *branch_name = NULL;
+	struct strbuf branch_desc = STRBUF_INIT;
+	const char *ref;
+	int status = 0;
+	unsigned char head_id[20], base_id[20];
+	unsigned char *merge_base_id;
+	struct commit *base_commit, *head_commit, *merge_base_commit;
+	struct commit_list *merge_bases;
+	const char *merge_base_summary, *merge_base_date;
+	const char *head_summary, *head_date;
+	struct remote *remote;
+	struct transport *transport;
+
+	const struct option options[] = {
+		OPT_BOOL('p', NULL, &patch, N_("show patch text as well")),
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, prefix, options, pull_request_usage, 0);
+
+	base = argv[0];
+	url = argv[1];
+	head = argv[2] ? argv[2] : "HEAD";
+	status = 0;
+
+	if (!base || !url)
+		usage_with_options(pull_request_usage, options);
+
+	if (dwim_ref(head, strlen(head), head_id, &head_ref) == 1) {
+		if (!prefixcmp(head_ref, "refs/heads/")) {
+			branch_name = head_ref + 11;
+			if (read_branch_desc(&branch_desc, branch_name) || !branch_desc.len)
+				branch_name = NULL;
+		}
+	}
+
+	describe(head);
+
+	get_sha1(base, base_id);
+	base_commit = lookup_commit_reference(base_id);
+	if (!base_commit)
+		die("Not a valid revision: %s", base);
+
+	head_commit = lookup_commit_reference(head_id);
+	if (!head_commit)
+		die("Not a valid revision: %s", head);
+
+	merge_bases = get_merge_bases(base_commit, head_commit, 0);
+	if (!merge_bases)
+		die("No commits in common between %s and %s", base, head);
+
+	merge_base_commit = merge_bases->item;
+	merge_base_id = merge_base_commit->object.sha1;
+
+	remote = remote_get(url);
+	url = remote->url[0];
+	transport = transport_get(remote, url);
+	ref = get_ref(transport, strcmp(head_ref, "HEAD") ? head_ref : NULL, head_id);
+	transport_disconnect(transport);
+
+	parse_buffer(merge_base_commit->buffer, &merge_base_summary, &merge_base_date);
+	parse_buffer(head_commit->buffer, &head_summary, &head_date);
+
+	printf("The following changes since commit %s:\n"
+			"\n"
+			"  %s (%s)\n"
+			"\n"
+			"are available in the git repository at:\n"
+			"\n",
+			sha1_to_hex(merge_base_id), merge_base_summary, merge_base_date);
+	printf("  %s", url);
+	if (ref)
+		printf(" %s", ref);
+	printf("\n");
+	printf("\n"
+			"for you to fetch changes up to %s:\n"
+			"\n"
+			"  %s (%s)\n"
+			"\n"
+			"----------------------------------------------------------------\n",
+			sha1_to_hex(head_id), head_summary, head_date);
+
+	if (branch_name)
+		printf("(from the branch description for %s local branch)\n\n%s\n", branch_name, branch_desc.buf);
+
+	if (tag_name) {
+		void *buffer;
+		enum object_type type;
+		unsigned long size;
+		unsigned char sha1[20];
+		const char *begin, *end;
+
+		if (!ref || prefixcmp(ref, "tags/") || strcmp(ref + 5, tag_name)) {
+			fprintf(stderr, "warn: You locally have %s but it does not (yet)\n", tag_name);
+			fprintf(stderr, "warn: appear to be at %s\n", url);
+			fprintf(stderr, "warn: Do you want to push it there, perhaps?\n");
+		}
+		get_sha1(tag_name, sha1);
+		buffer = read_sha1_file(sha1, &type, &size);
+		begin = strstr(buffer, "\n\n") + 2;
+		end = strstr(begin, "-----BEGIN PGP ");
+		if (!end)
+			end = begin + strlen(begin);
+		printf("%.*s\n", (int)(end - begin), begin);
+	}
+
+	if (branch_name || tag_name)
+		puts("----------------------------------------------------------------");
+
+	show_shortlog(base, head);
+	show_diff(patch, merge_base_id, head_id);
+
+	if (!ref) {
+		fprintf(stderr, "warn: No branch of %s is at:\n", url);
+		fprintf(stderr, "warn:   %s: %s\n", find_unique_abbrev(head_id, DEFAULT_ABBREV), head_summary);
+		fprintf(stderr, "warn: Are you sure you pushed '%s' there?\n", head);
+		status = 1;
+	}
+	return status;
+}
diff --git a/git-request-pull.rb b/git-request-pull.rb
deleted file mode 100755
index 941ff72..0000000
--- a/git-request-pull.rb
+++ /dev/null
@@ -1,199 +0,0 @@
-#!git ruby
-
-require 'date'
-
-patch = nil
-
-def usage
-  puts <<EOF
-usage: git request-pull [options] start url [end]
-
-    -p                    show patch text as well
-
-EOF
-  exit 1
-end
-
-def read_branch_desc(name)
-  git_config() do |key, value|
-    return value if key == "branch.#{name}.description"
-  end
-  return nil
-end
-
-def describe(rev)
-  for_each_ref() do |name, sha1, flags|
-    next unless name.start_with?('refs/tags/')
-    next unless peel_ref(name) == get_sha1(rev)
-    return name.skip_prefix('refs/tags/')
-  end
-  return nil
-end
-
-def abbr(ref)
-  if (ref =~ %r{^refs/heads/(.*)} || ref =~ %r{^refs/(tags/.*)})
-    return $1
-  end
-  return ref
-end
-
-# $head is the token given from the command line, and $tag_name, if
-# exists, is the tag we are going to show the commit information for.
-# If that tag exists at the remote and it points at the commit, use it.
-# Otherwise, if a branch with the same name as $head exists at the remote
-# and their values match, use that instead.
-#
-# Otherwise find a random ref that matches $head_id.
-
-def get_ref(transport, head_ref, head_id, tag_name)
-  found = nil
-  transport.get_remote_refs().each do |e|
-    sha1 = e.old_sha1
-    next unless sha1 == head_id
-    ref, deref = e.name.scan(/^(\S+?)(\^\{\})?$/).first
-    found = abbr(ref)
-    break if (deref && ref == "refs/tags/#{tag_name}")
-    break if ref == head_ref
-  end
-  return found
-end
-
-def parse_buffer(buffer)
-  summary = []
-  date = msg = nil
-  header, body = buffer.split("\n\n", 2)
-  header.each_line do |line|
-    case line
-    when /^committer ([^<>]+) <(\S+)> (.+)$/
-      date = DateTime.strptime($3, '%s %z')
-    end
-  end
-  body.each_line do |l|
-    break if (l.strip.empty?)
-    summary << l.chomp
-  end
-  summary = summary.join(' ')
-  date = date.strftime('%F %T %z')
-  return [summary, date]
-end
-
-def show_shortlog(base, head)
-  rev = Git::RevInfo.setup(nil, ['^' + base, head], nil)
-  shortlog(rev.to_a)
-end
-
-def show_diff(patch, base, head)
-  rev = Git::RevInfo.setup(nil, ['^' + sha1_to_hex(base), sha1_to_hex(head)], nil)
-  rev.diffopt.stat_width = -1
-  rev.diffopt.stat_graph_width = -1
-  rev.diffopt.output_format = patch ? DIFF_FORMAT_PATCH : DIFF_FORMAT_DIFFSTAT
-  rev.diffopt.output_format |= DIFF_FORMAT_SUMMARY
-  rev.diffopt.detect_rename = DIFF_DETECT_RENAME
-  rev.diffopt.flags |= DIFF_OPT_RECURSIVE
-
-  diff_tree_sha1(base, head, "", rev.diffopt)
-  log_tree_diff_flush(rev)
-end
-
-until ARGV.empty?
-  case ARGV.first
-  when '-p'
-    patch = '-p'
-  when '--'
-    ARGV.shift
-    break
-  when /^-/
-    usage
-  else
-    break
-  end
-  ARGV.shift
-end
-
-base = ARGV[0]
-url = ARGV[1]
-head = ARGV[2] || 'HEAD'
-branch_name = branch_desc = nil
-
-usage unless base or url
-
-_, _, head_ref = dwim_ref(head)
-
-if head_ref.start_with?('refs/heads')
-  branch_name = head_ref[11..-1]
-  branch_desc = read_branch_desc(branch_name)
-  branch_name = nil if not branch_desc
-end
-
-tag_name = describe(head)
-
-base_id = get_sha1("#{base}^0")
-die "Not a valid revision: #{base}" unless base_id
-
-head_id = get_sha1("#{head}^0")
-die "Not a valid revision: #{head}" unless head_id
-
-base_commit = Git::Commit.get(base_id)
-head_commit = Git::Commit.get(head_id)
-
-merge_bases = get_merge_bases([base_commit, head_commit], 0);
-die "No commits in common between #{base} and #{head}" unless merge_bases
-
-merge_base_id = merge_bases.first.sha1
-merge_base_commit = Git::Commit.get(merge_base_id)
-
-remote = remote_get(url)
-transport = transport_get(remote, nil)
-
-ref = get_ref(transport, head_ref != "HEAD" ? head_ref : nil, head_id, tag_name)
-url = remote.url.first
-
-merge_base_summary, merge_base_date = parse_buffer(merge_base_commit.buffer)
-head_summary, head_date = parse_buffer(head_commit.buffer)
-
-puts "The following changes since commit %s:
-
-  %s (%s)
-
-are available in the git repository at:
-
-" % [merge_base_commit, merge_base_summary, merge_base_date]
-puts "  #{url}" + (ref ? " #{ref}" : "")
-puts "
-for you to fetch changes up to %s:
-
-  %s (%s)
-
-----------------------------------------------------------------
-" % [head_commit, head_summary, head_date]
-
-if branch_name
-  puts "(from the branch description for #{branch_name} local branch)"
-  puts
-  puts branch_desc
-end
-
-if tag_name
-  if ref != "tags/#{tag_name}"
-    $stderr.puts "warn: You locally have #{tag_name} but it does not (yet)"
-    $stderr.puts "warn: appear to be at #{url}"
-    $stderr.puts "warn: Do you want to push it there, perhaps?"
-  end
-  buffer, _ = read_sha1_file(get_sha1(tag_name))
-  puts buffer.scan(/(?:\n\n)(.+)(?:-----BEGIN PGP )?/m).first
-  puts
-end
-
-if branch_name || tag_name
-  puts "----------------------------------------------------------------"
-end
-
-show_shortlog(base, head)
-show_diff(patch, merge_base_id, head_id)
-
-if ! ref
-  $stderr.puts "warn: No branch of #{url} is at:"
-  $stderr.puts "warn:   %s: %s'" % [find_unique_abbrev(head_id, DEFAULT_ABBREV), head_summary]
-  $stderr.puts "warn: Are you sure you pushed '#{abbr(head_ref)}' there?"
-  exit 1
-end
diff --git a/git.c b/git.c
index 2025f77..d67d3fb 100644
--- a/git.c
+++ b/git.c
@@ -398,6 +398,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "remote-fd", cmd_remote_fd },
 		{ "replace", cmd_replace, RUN_SETUP },
 		{ "repo-config", cmd_repo_config, RUN_SETUP_GENTLY },
+		{ "request-pull", cmd_request_pull, RUN_SETUP_GENTLY },
 		{ "rerere", cmd_rerere, RUN_SETUP },
 		{ "reset", cmd_reset, RUN_SETUP },
 		{ "rev-list", cmd_rev_list, RUN_SETUP },
-- 
1.8.4-fc

Re: [PATCH v2 42/44] ruby: remove GIT_PAGER from environment

[Stefan Beller]

On 09/29/2013 12:04 AM, Felipe Contreras wrote:
> We are not calling any git commands any more.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  git-request-pull.rb | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/git-request-pull.rb b/git-request-pull.rb
> index fde8d1a..941ff72 100755
> --- a/git-request-pull.rb
> +++ b/git-request-pull.rb
> @@ -2,8 +2,6 @@
>  
>  require 'date'
>  
> -ENV['GIT_PAGER'] =
> -
>  patch = nil
>  
>  def usage
> 

Could this be squashed into the patch introducing the ENV?
(I assume that patch is among the 44 patches?)

Thanks,
Stefan

Re: [PATCH v2 42/44] ruby: remove GIT_PAGER from environment

[Felipe Contreras]

On Sat, Sep 28, 2013 at 6:27 PM, Stefan Beller
<stefanbeller@googlemail.com> wrote:
> On 09/29/2013 12:04 AM, Felipe Contreras wrote:
>> We are not calling any git commands any more.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  git-request-pull.rb | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/git-request-pull.rb b/git-request-pull.rb
>> index fde8d1a..941ff72 100755
>> --- a/git-request-pull.rb
>> +++ b/git-request-pull.rb
>> @@ -2,8 +2,6 @@
>>
>>  require 'date'
>>
>> -ENV['GIT_PAGER'] =
>> -
>>  patch = nil
>>
>>  def usage
>>
>
> Could this be squashed into the patch introducing the ENV?
> (I assume that patch is among the 44 patches?)

It could be, but when ENV['GIT_PAGER'] was introduced, there were 'git
foo' commands being called, so they would get a different $GIT_PAGER
if we do so, which would be a change from the original script.

To be honest I don't know what that code is trying to achieve, but to
err on the side of caution I'm leaving the code close to the original.

-- 
Felipe Contreras

Re: [PATCH v2 00/44] Ruby support

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> Felipe Contreras (44):

Thanks! In the meantime, can you publish this work somewhere so we can
easily try it out?

Re: [PATCH v2 00/44] Ruby support

[Felipe Contreras]

On Sat, Sep 28, 2013 at 11:13 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> Felipe Contreras (44):
>
> Thanks! In the meantime, can you publish this work somewhere so we can
> easily try it out?

Sure:
https://github.com/felipec/git/commits/fc/ruby

-- 
Felipe Contreras

Re: [PATCH v2 01/44] Add support for ruby commands

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> +test_expect_success 'argument passing' '
> +       cat > script <<-"EOF" &&
> +       p($0)
> +       p(ARGV)
> +       EOF
> +       git ruby script foo bar > actual &&
> +       cat > expected <<-EOF &&
> +       "script"
> +       ["foo", "bar"]
> +       EOF
> +       test_cmp expected actual
> +'

So, git-ruby is a new builtin that's just like a ruby interpreter,
except that it has certain git-related functions defined: you pass it
source code on stdin, and it returns the result on stdout. I wonder
what the purpose of passing arguments to it is though.

> +test_expect_success 'test for_each_ref()' '
> +       test_commit foo &&
> +       git ruby > actual <<-EOF &&
> +       for_each_ref() do |name, sha1, flags|
> +               puts "%s: %s" % [name, sha1_to_hex(sha1)]
> +       end
> +       EOF
> +       git for-each-ref --format="%(refname): %(objectname)" > expected &&
> +       test_cmp expected actual
> +'

I find it interesting that you chose to implement for-each-ref in the
initial version. It currently takes no arguments, but we can always
extend it to do ref-hierarchy filtering like f-e-r in the future.

Re: [PATCH v2 02/44] ruby: add support for internal ruby programs

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> +$(RUBY_PROGRAMS): git-ruby$X
> +       $(QUIET_BUILT_IN)$(RM) $@ && \
> +       ln >lt; $@ 2>/dev/null || \
> +       ln -s >lt; $@ 2>/dev/null || \
> +       cp >lt; $@

Why so many fallbacks? Will the hard-link (the first ln) ever fail?

> diff --git a/ruby.c b/ruby.c
> index ee6a0e7..339e376 100644
> --- a/ruby.c
> +++ b/ruby.c
> @@ -52,5 +52,22 @@ static int run_ruby_command(const char *cmd, int argc, const char **argv)
>
>  int main(int argc, const char **argv)
>  {
> -       return run_ruby_command(argv[1], argc, argv);
> +       if (!strcmp(argv[0], "git-ruby")) {
> +               return run_ruby_command(argv[1], argc, argv);
> +       } else {
> +               const char *cmd = argv[0];
> +               static char buf[PATH_MAX + 1];
> +               const char *args[argc + 1];
> +               int i;
> +
> +               snprintf(buf, PATH_MAX, "%s/%s.rb",
> +                               git_exec_path(), basename((char *)cmd));
> +
> +               args[0] = "git";
> +               args[1] = buf;
> +               for (i = 0; i < argc - 1; i++)
> +                       args[i + 2] = (char *)argv[i + 1];
> +
> +               return run_ruby_command(cmd, argc + 1, args);
> +       }
>  }

Can you explain this in greater detail in your commit message? When I
pass an argument,

  $ git ruby foo
  git-ruby: No such file or directory -- foo (LoadError)

I get this as before. How exactly will new ruby scripts be executed?
(I can only guess at this point)

Re: [PATCH v2 03/44] request-pull: fix annotated tag check

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index ebf1269..6348dac 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -89,7 +89,7 @@ find_matching_ref='
>                 my ($sha1, $ref, $deref) = /^(\S+)\s+(\S+?)(\^\{\})?$/;
>                 next unless ($sha1 eq $ARGV[1]);
>                 $found = abbr($ref);
> -               if ($deref && $ref eq "tags/$ARGV[2]") {
> +               if ($deref && $ref eq "refs/tags/$ARGV[2]") {

I've never used request-pull, but this seems correct.

Re: [PATCH v2 01/44] Add support for ruby commands

[Felipe Contreras]

On Sat, Sep 28, 2013 at 11:30 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> +test_expect_success 'argument passing' '
>> +       cat > script <<-"EOF" &&
>> +       p($0)
>> +       p(ARGV)
>> +       EOF
>> +       git ruby script foo bar > actual &&
>> +       cat > expected <<-EOF &&
>> +       "script"
>> +       ["foo", "bar"]
>> +       EOF
>> +       test_cmp expected actual
>> +'
>
> So, git-ruby is a new builtin that's just like a ruby interpreter,
> except that it has certain git-related functions defined: you pass it
> source code on stdin, and it returns the result on stdout. I wonder
> what the purpose of passing arguments to it is though.

git ruby is just like ruby, so you can pass the script from stdin, or
you can specify the script name.

>> +test_expect_success 'test for_each_ref()' '
>> +       test_commit foo &&
>> +       git ruby > actual <<-EOF &&
>> +       for_each_ref() do |name, sha1, flags|
>> +               puts "%s: %s" % [name, sha1_to_hex(sha1)]
>> +       end
>> +       EOF
>> +       git for-each-ref --format="%(refname): %(objectname)" > expected &&
>> +       test_cmp expected actual
>> +'
>
> I find it interesting that you chose to implement for-each-ref in the
> initial version. It currently takes no arguments, but we can always
> extend it to do ref-hierarchy filtering like f-e-r in the future.

It takes no arguments because for_each_ref()(refs.h) takes no
arguments (that Ruby would need). builtin/for-each-ref.c uses
for_each_rawref()(refs.h), which is basically the same.

-- 
Felipe Contreras

Re: [PATCH v2 02/44] ruby: add support for internal ruby programs

[Felipe Contreras]

On Sat, Sep 28, 2013 at 11:40 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> +$(RUBY_PROGRAMS): git-ruby$X
>> +       $(QUIET_BUILT_IN)$(RM) $@ && \
>> +       ln >lt; $@ 2>/dev/null || \
>> +       ln -s >lt; $@ 2>/dev/null || \
>> +       cp >lt; $@
>
> Why so many fallbacks? Will the hard-link (the first ln) ever fail?

Because that's what the code right on top of this is doing. I presume
it would fail in file-systems that don't have hard-links.

>> diff --git a/ruby.c b/ruby.c
>> index ee6a0e7..339e376 100644
>> --- a/ruby.c
>> +++ b/ruby.c
>> @@ -52,5 +52,22 @@ static int run_ruby_command(const char *cmd, int argc, const char **argv)
>>
>>  int main(int argc, const char **argv)
>>  {
>> -       return run_ruby_command(argv[1], argc, argv);
>> +       if (!strcmp(argv[0], "git-ruby")) {
>> +               return run_ruby_command(argv[1], argc, argv);
>> +       } else {
>> +               const char *cmd = argv[0];
>> +               static char buf[PATH_MAX + 1];
>> +               const char *args[argc + 1];
>> +               int i;
>> +
>> +               snprintf(buf, PATH_MAX, "%s/%s.rb",
>> +                               git_exec_path(), basename((char *)cmd));
>> +
>> +               args[0] = "git";
>> +               args[1] = buf;
>> +               for (i = 0; i < argc - 1; i++)
>> +                       args[i + 2] = (char *)argv[i + 1];
>> +
>> +               return run_ruby_command(cmd, argc + 1, args);
>> +       }
>>  }
>
> Can you explain this in greater detail in your commit message? When I
> pass an argument,
>
>   $ git ruby foo
>   git-ruby: No such file or directory -- foo (LoadError)
>
> I get this as before. How exactly will new ruby scripts be executed?
> (I can only guess at this point)

If you do:

% git ruby foo

It's the same as

% ruby foo

You need the script right there, as a file named "foo".

However, this already works before this patch.

What this patch does is enable:

% git foo

But for this you need two things:

1) A binary named git-foo in your path, that is a link to git-ruby,
which is what the variable RUBY_PROGRAMS is for in the Makefile

2) A script named git-foo.rb in your exec-path.

So basically "git-foo" is the same as "git ruby $GIT_EXEC_PATH/git-foo.rb".

-- 
Felipe Contreras

Re: [PATCH v2 10/44] ruby: rewrite 'request-pull'

[Ramkumar Ramachandra]

Felipe Contreras wrote:
>  Makefile            |   3 +-
>  git-rb-setup.rb     |  17 ++++++
>  git-request-pull.rb | 146 +++++++++++++++++++++++++++++++++++++++++++++++
>  git-request-pull.sh | 160 ----------------------------------------------------

Excellent: git-request-pull is now an ELF executable! Normally, one
would move the older script (git-request-pull.sh) into
contrib/examples during such a rewrite, although I don't see the point
of doing that.

The proof is the pudding: all the request-pull tests pass.

Re: [PATCH v2 11/44] ruby: request-pull: rewrite perl script

[Ramkumar Ramachandra]

Felipe Contreras wrote:
>  git-request-pull.rb | 65 +++++++++++++++++++++++++----------------------------
>  1 file changed, 30 insertions(+), 35 deletions(-)

Arguably, more pretty and terse. Again, all tests pass.

Looks reasonable.

Re: [PATCH v2 02/44] ruby: add support for internal ruby programs

[Felipe Contreras]

On Sat, Sep 28, 2013 at 11:56 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Sep 28, 2013 at 11:40 PM, Ramkumar Ramachandra

>>> diff --git a/ruby.c b/ruby.c
>>> index ee6a0e7..339e376 100644
>>> --- a/ruby.c
>>> +++ b/ruby.c
>>> @@ -52,5 +52,22 @@ static int run_ruby_command(const char *cmd, int argc, const char **argv)
>>>
>>>  int main(int argc, const char **argv)
>>>  {
>>> -       return run_ruby_command(argv[1], argc, argv);
>>> +       if (!strcmp(argv[0], "git-ruby")) {
>>> +               return run_ruby_command(argv[1], argc, argv);
>>> +       } else {
>>> +               const char *cmd = argv[0];
>>> +               static char buf[PATH_MAX + 1];
>>> +               const char *args[argc + 1];
>>> +               int i;
>>> +
>>> +               snprintf(buf, PATH_MAX, "%s/%s.rb",
>>> +                               git_exec_path(), basename((char *)cmd));
>>> +
>>> +               args[0] = "git";
>>> +               args[1] = buf;
>>> +               for (i = 0; i < argc - 1; i++)
>>> +                       args[i + 2] = (char *)argv[i + 1];
>>> +
>>> +               return run_ruby_command(cmd, argc + 1, args);
>>> +       }
>>>  }
>>
>> Can you explain this in greater detail in your commit message? When I
>> pass an argument,
>>
>>   $ git ruby foo
>>   git-ruby: No such file or directory -- foo (LoadError)
>>
>> I get this as before. How exactly will new ruby scripts be executed?
>> (I can only guess at this point)
>
> If you do:
>
> % git ruby foo
>
> It's the same as
>
> % ruby foo
>
> You need the script right there, as a file named "foo".
>
> However, this already works before this patch.
>
> What this patch does is enable:
>
> % git foo
>
> But for this you need two things:
>
> 1) A binary named git-foo in your path, that is a link to git-ruby,
> which is what the variable RUBY_PROGRAMS is for in the Makefile
>
> 2) A script named git-foo.rb in your exec-path.
>
> So basically "git-foo" is the same as "git ruby $GIT_EXEC_PATH/git-foo.rb".

Note that this is for _internal_ Ruby programs, if you want "git foo"
to run yours, or a third party script, you can do it like this:

cat > git-foo <<EOF
#!git ruby

#ruby code
EOF

chmod +x and put it on your PATH, done :)

-- 
Felipe Contreras

Re: [PATCH v2 13/44] ruby: bind setup_git_directory()

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> +static inline VALUE cstr_to_str(const char *str)
> +{
> +       if (str == NULL)
> +               return Qnil;
> +       return rb_str_new2(str);
> +}
> +
> +static VALUE git_rb_setup_git_directory(VALUE self)
> +{
> +       int nongit_ok;
> +       const char *prefix;
> +       prefix = setup_git_directory_gently(&nongit_ok);
> +       return rb_ary_new3(2, cstr_to_str(prefix), INT2FIX(nongit_ok));
> +}

Most excellent. Goes to show how well ruby.h is written.

I'm not very familiar with the interface, and am referring to
http://www.ruby-doc.org/docs/ProgrammingRuby/html/ext_ruby.html -- let
me know if there are some other sources.

Re: [PATCH v2 14/44] ruby: bind dwim_ref()

[Ramkumar Ramachandra]

Felipe Contreras wrote:
>  static void git_ruby_init(void)
>  {
>         rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
>         rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
> +       rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
>  }

At this point, global functions is probably the way to go. We might
like to put them in classes to create a beautiful Rugged-inspired API
in the future.

Re: [PATCH v2 14/44] ruby: bind dwim_ref()

[Felipe Contreras]

On Sun, Sep 29, 2013 at 12:17 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>>  static void git_ruby_init(void)
>>  {
>>         rb_define_global_function("setup_git_directory", git_rb_setup_git_directory, 0);
>>         rb_define_global_function("for_each_ref", git_rb_for_each_ref, 0);
>> +       rb_define_global_function("dwim_ref", git_rb_dwim_ref, 1);
>>  }
>
> At this point, global functions is probably the way to go. We might
> like to put them in classes to create a beautiful Rugged-inspired API
> in the future.

We might, for certain things, but as I explained before, we can't
really do the same.

test1 = Rugged::Repository.new("test1")
test2 = Rugged::Repository.new("test2")

How are we supposed to do this with Git's code? If Git's code can only
work with one repository at a time, I think the Ruby bindings should
reflect that as well.

-- 
Felipe Contreras

Re: [PATCH v2 13/44] ruby: bind setup_git_directory()

[Felipe Contreras]

On Sun, Sep 29, 2013 at 12:11 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> +static inline VALUE cstr_to_str(const char *str)
>> +{
>> +       if (str == NULL)
>> +               return Qnil;
>> +       return rb_str_new2(str);
>> +}
>> +
>> +static VALUE git_rb_setup_git_directory(VALUE self)
>> +{
>> +       int nongit_ok;
>> +       const char *prefix;
>> +       prefix = setup_git_directory_gently(&nongit_ok);
>> +       return rb_ary_new3(2, cstr_to_str(prefix), INT2FIX(nongit_ok));
>> +}
>
> Most excellent. Goes to show how well ruby.h is written.

Indeed. I find it easier than using GLib.

> I'm not very familiar with the interface, and am referring to
> http://www.ruby-doc.org/docs/ProgrammingRuby/html/ext_ruby.html -- let
> me know if there are some other sources.

That one is really good and got me almost as far as I am.

Others might give a bit more information, but nothing too critical.

http://stackoverflow.com/tags/ruby-c-api/info

-- 
Felipe Contreras

Re: [PATCH v2 44/44] request-pull: rewrite to C

[Ramkumar Ramachandra]

Felipe Contreras wrote:
>  builtin/request-pull.c | 275 +++++++++++++++++++++++++++++++++++++++++++++++++
>  git-request-pull.rb    | 199 -----------------------------------

My head started hurting beyond part 15: I need more coffee ;)

Right now, I'm looking at these two side by side:
https://github.com/felipec/git/blob/724e759e777e21ddc1e1a77b5bad16a301dffe38/git-request-pull.rb
https://github.com/felipec/git/blob/68dd43654e265d8ebe42daf43646c636d3f0ff99/builtin/request-pull.c

Truly beautiful. No more
kitchen-sink-dependency-embedded-lua-language-war-flame fud.

I look forward to more eyes, and a clean merge. Part 2 is missing a
commit message, but the rest of your series looks very reasonable.
Thanks a lot for doing this! You've opened the door for an incredible
amount of learning opportunities.