Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/5] Rework diff options
From: Johannes Schindelin @ 2006-06-23 22:28 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: Junio C Hamano, git
In-Reply-To: <20060624011538.9bb179e7.tihirvon@gmail.com>

Hi,

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

> This patch series cleans up diff output format options.

Very good.

Although I understand that to convert all users to the new convention, it 
is sensible to rename the constants, I think it is not good to change 
something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

IMHO it is an unnecessary change, and accounts for a lot of the diffstat.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/5] Rework diff options
From: Timo Hirvonen @ 2006-06-23 22:40 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: junkio, git
In-Reply-To: <20060624005252.c694e421.tihirvon@gmail.com>

Timo Hirvonen <tihirvon@gmail.com> wrote:

> @@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c
>  			num_paths++;
>  	}
>  	if (num_paths) {
> -		if (opt->with_raw) {
> -			int saved_format = opt->output_format;
> -			opt->output_format = DIFF_FORMAT_RAW;
> +		if (opt->output_fmt & OUTPUT_FMT_RAW) {
> +			int saved_fmt = opt->output_fmt;
> +			opt->output_fmt |= OUTPUT_FMT_RAW;

I have no idea if this is more right than this:

                        opt->output_fmt = OUTPUT_FMT_RAW;

> @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char
>  	if (revs->combine_merges) {
>  		revs->ignore_merges = 0;
>  		if (revs->dense_combined_merges &&
> -		    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))
> -			revs->diffopt.output_format = DIFF_FORMAT_PATCH;
> +		   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))
> +			revs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;

I'm not sure about this. Didn't really understand the code :)

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Timo Hirvonen @ 2006-06-23 22:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: junkio, git
In-Reply-To: <Pine.LNX.4.63.0606240024460.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Hi,
> 
> On Sat, 24 Jun 2006, Timo Hirvonen wrote:
> 
> > This patch series cleans up diff output format options.
> 
> Very good.
> 
> Although I understand that to convert all users to the new convention, it 
> is sensible to rename the constants, I think it is not good to change 
> something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.
> 
> IMHO it is an unnecessary change, and accounts for a lot of the diffstat.

I did it because you can't have many DIFF_FORMAT_* options at the same
time but OUTPUT_FMT_* can be combined.

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* Re: [PATCH] git-merge --squash
From: Junio C Hamano @ 2006-06-23 23:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Glanzmann, git
In-Reply-To: <Pine.LNX.4.63.0606231433370.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Isn't this the same as 'git-cherry-pick -n'? I often do a poor man's StGIT 
> by cherry picking my way through a messy branch, often combining patches 
> by '-n'.

Operationally, it probably is equivalent to the repeated use of
'cherry-pick -n' for all commits on a topic, but that would risk
you having to resolve conflicts unnecessarily when you are
shooting for as the result is a single commit, because you would
have to do N merges with that workflow.  Squashing is about
merging the tip of the topic into mainline, so the conflict
resolution needs to be done only once.

^ permalink raw reply

* Re: [PATCH] Customizable error handlers
From: Junio C Hamano @ 2006-06-23 23:14 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060623133847.GN21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Fri, Jun 23, 2006 at 03:32:27PM CEST, I got a letter
> where Petr Baudis <pasky@suse.cz> said that...
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 5d543d2..e954002 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -36,9 +36,13 @@ #endif
>>  #endif
>>  
>>  /* General helper functions */
>> -extern void usage(const char *err) NORETURN;
>> -extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
>> -extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
>> +void usage(const char *err) NORETURN;
>> +void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
>> +int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
>
> Wah, this kind of slipped through. Below is a patch without the
> externs removed.

Oh, I first thought you did that on purpose as a cleanup.  After
all don't they mean the same thing?

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Linus Torvalds @ 2006-06-23 23:16 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: Junio C Hamano, git
In-Reply-To: <20060624011538.9bb179e7.tihirvon@gmail.com>



On Sat, 24 Jun 2006, Timo Hirvonen wrote:
>
> This patch series cleans up diff output format options.
> 
> This makes it possible to use any combination of --raw, -p, --stat and
> --summary options and they work as you would expect.

Looks good to me. I'll be very happy never having to remember the option 
(or type) --patch-with-stat ever again. Doing just "-p --stat" is just 
_so_ much better.

		Linus

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Johannes Schindelin @ 2006-06-23 23:18 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: junkio, git
In-Reply-To: <20060624014420.2c3df276.tihirvon@gmail.com>

Hi,

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > Although I understand that to convert all users to the new convention, it 
> > is sensible to rename the constants, I think it is not good to change 
> > something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

Note that I understand this for the purpose of not forgetting to change 
things over to "|=" and "&": the compiler will warn you about that now.

But after it compiles, you can change the names back to reduce patch size 
and to avoid confusing of dumb people like me.

> > IMHO it is an unnecessary change, and accounts for a lot of the diffstat.
> 
> I did it because you can't have many DIFF_FORMAT_* options at the same
> time but OUTPUT_FMT_* can be combined.

But you just renamed them! The name alone does not say "you cannot combine 
them".

-- snip --
@@ -150,15 +162,6 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "                show all files diff when -S is used and hit is found.\n"
 
 extern int diff_queue_is_empty(void);
-
-#define DIFF_FORMAT_RAW                1
-#define DIFF_FORMAT_PATCH      2
-#define DIFF_FORMAT_NO_OUTPUT  3
-#define DIFF_FORMAT_NAME       4
-#define DIFF_FORMAT_NAME_STATUS        5
-#define DIFF_FORMAT_DIFFSTAT   6
-#define DIFF_FORMAT_CHECKDIFF  7
-
-- snap --

You also sneak in some other things, such as renaming output_format to 
output_fmt in struct diff_options, making a function static, and expanding 
a "(a ? b : c)", without accounting for it in the commit message.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Junio C Hamano @ 2006-06-23 23:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Timo Hirvonen, git
In-Reply-To: <Pine.LNX.4.63.0606240024460.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sat, 24 Jun 2006, Timo Hirvonen wrote:
>
>> This patch series cleans up diff output format options.
>
> Very good.
>
> Although I understand that to convert all users to the new convention, it 
> is sensible to rename the constants, I think it is not good to change 
> something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

I personally feel that the benefit of being able to make sure
you covered everything outweighs the size of initial diff.

Thanks Timo.  Will take a look.

^ permalink raw reply

* Re: [PATCH] Customizable error handlers
From: Petr Baudis @ 2006-06-23 23:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsllv8ny8.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 01:14:07AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Dear diary, on Fri, Jun 23, 2006 at 03:32:27PM CEST, I got a letter
> > where Petr Baudis <pasky@suse.cz> said that...
> >> diff --git a/git-compat-util.h b/git-compat-util.h
> >> index 5d543d2..e954002 100644
> >> --- a/git-compat-util.h
> >> +++ b/git-compat-util.h
> >> @@ -36,9 +36,13 @@ #endif
> >>  #endif
> >>  
> >>  /* General helper functions */
> >> -extern void usage(const char *err) NORETURN;
> >> -extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
> >> -extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> >> +void usage(const char *err) NORETURN;
> >> +void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
> >> +int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> >
> > Wah, this kind of slipped through. Below is a patch without the
> > externs removed.
> 
> Oh, I first thought you did that on purpose as a cleanup.  After
> all don't they mean the same thing?

AFAIK they do, but we use extern everywhere and I personally consider
it good style to externize declarations of, well, external functions.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ permalink raw reply

* Re: [PATCH] git-commit: allow -e option anywhere on command line
From: Junio C Hamano @ 2006-06-23 23:56 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20060623134338.GA12630@coredump.intra.peff.net>

Makes sense.  Thanks.

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Johannes Schindelin @ 2006-06-24  0:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Timo Hirvonen, git
In-Reply-To: <7vodwj8n9s.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 23 Jun 2006, Junio C Hamano wrote:

> I personally feel that the benefit of being able to make sure you 
> covered everything outweighs the size of initial diff.

IMHO the difficulty of finding bugs is proportional to the square of the 
diff size, while the number of people willing to review it is proportional 
to its square root. So, if it is not difficult (which it is not at all in 
this case), I politely ask to cut the patch size down.

Ciao,
Dscho

^ permalink raw reply

* Re: x86 asm SHA1 (draft)
From: Junio C Hamano @ 2006-06-24  0:18 UTC (permalink / raw)
  To: git; +Cc: linux
In-Reply-To: <20060623171809.5689.qmail@science.horizon.com>

The series to revamp SHA1 is good but judging the merit of each
is outside my expertise, so I'd appreciate help to evaluate
these changes.  For example, I cannot choose between competing
three implementations for ppc without having access to a variety
of ppc machines, and even if I did, ppc is not what I normally
use, so incentive to try picking the best one for everybody is
relatively low on my part.

What we would want are obviously (1) no regression, (2) an easy
way for people who build git for their own machine to decide
which one suits them the best, and (3) an easy way to tell the
build mechanism to use the one that is chosen.

The only external interface for the set of SHA1 implementation
alternatives to the outside world is a well established SHA_CTX
type, and three functions SHA1_Init(), SHA1_Update() and
SHA1_Final(), and the alternative implementations are supposed
to be drop-in replaceable.

So let's do something like this:

 - When adding a new SHA1 implementation, we need to come up
   with a Makefile symbol (similar to PPC_SHA1, MOZILLA_SHA1 and
   friends) and set up the build machinery to use the one that
   is chosen;

 - We need a test program and a build rule in the Makefile that
   links with the chosen SHA1 implementation.

 - We need a test script that feeds the above program with known
   vectors to validate the SHA1 implementation (make sure the
   test covers large input to avoid the recent half-gig-limit
   problem), and bench the speed on the platform it was built.

 - If we wanted to go fancier, another script that builds all
   applicable alternatives on the building platform, run the
   bench for all of them and automatically pick the right one
   for the platform would be a plus.

We probably would want to collect the benchmark results from
popular platforms, have a summary to help people to choose a
sensible one in the toplevel INSTALL file, and include the raw
numbers in Documentation/technical/sha1-implementations.txt.

Once we go the above path, we may want to include both of the
the two new ppc implementations as separate choices as I
understand their performance depends on which ppc you are
talking about.

Any takers?

^ permalink raw reply

* Re: [PATCH] git-commit: filter out log message lines only when editor was run.
From: Junio C Hamano @ 2006-06-24  0:21 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git
In-Reply-To: <20060623220405.1915.28636.stgit@gandelf.nowhere.earth>

Yann Dirson <ydirson@altern.org> writes:

> The current behaviour strips out lines starting with a # even when fed
> through stdin or -m.  This is particularly bad when importing history from
> another SCM (tailor 0.9.23 uses git-commit).  In the best cases all lines
> are stripped and the commit fails with a confusing "empty log message"
> error, but in many cases the commit is done, with loss of information.

I agree with this in principle but we would need to make sure
that our own scripts do not expect that the message is cleaned
up when feeding a commit log message via stdin, -m or -F, and if
they do fix them before applying this patch.
 

^ permalink raw reply

* Re: [PATCH] Introduce Git.pm (v3)
From: Junio C Hamano @ 2006-06-24  0:39 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Eric W. Biederman, git
In-Reply-To: <20060623123904.GL21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> There's a patch to build libgit.so, would you take it as an excuse to
> always compile with -fPIC? ;-)

The only benerit we might gain from libgit.so in the current
shape, aside from making it easier to do .xs, is that multiple
git processes would share it in core, and the disk consumption
by 100+ little git commands are reduced -- which largely has
become non-issue with the recent spurt of making everything
built-ins.  But that "aside from" is in itself a big plus, so,
well, I am torn. 

^ permalink raw reply

* Re: [PATCH] Introduce Git.pm (v3)
From: Junio C Hamano @ 2006-06-24  1:01 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060623123904.GL21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

>> By the way, you also need to adjust the testsuite so that it
>> finds the Perl modules from freshly built tree before
>> installing.  I think (but haven't checked yet) the stuff written
>> in Python does that already, so you might want to mimic it.
>
> It should be enough to -I../perl/blib/lib -I../perl/blib/arch/auto/Git.

-- >8 --
[PATCH] Perl interface: add build-time configuration to allow building with -fPIC

On x86-64 it seems that Git.xs does not link without compiling
the main git objects with -fPIC.

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

 * Also I moved explanation of MOZILLA_SHA1 closer to other SHA1
   choices.

 Makefile |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 7842195..c42521e 100644
--- a/Makefile
+++ b/Makefile
@@ -1,11 +1,6 @@
 # The default target of this Makefile is...
 all:
 
-# Define MOZILLA_SHA1 environment variable when running make to make use of
-# a bundled SHA1 routine coming from Mozilla. It is GPL'd and should be fast
-# on non-x86 architectures (e.g. PowerPC), while the OpenSSL version (default
-# choice) has very fast version optimized for i586.
-#
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 # This also implies MOZILLA_SHA1.
 #
@@ -37,6 +32,14 @@ #
 # Define ARM_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for ARM.
 #
+# Define MOZILLA_SHA1 environment variable when running make to make use of
+# a bundled SHA1 routine coming from Mozilla. It is GPL'd and should be fast
+# on non-x86 architectures (e.g. PowerPC), while the OpenSSL version (default
+# choice) has very fast version optimized for i586.
+#
+# Define USE_PIC if you need the main git objects to be built with -fPIC
+# in order to build and link perl/Git.so.  x86-64 seems to need this.
+#
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lcrypto with -lssl (Darwin).
 #
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
@@ -63,13 +66,13 @@ #
 # Define COLLISION_CHECK below if you believe that SHA1's
 # 1461501637330902918203684832716283019655932542976 hashes do not give you
 # sufficient guarantee that no collisions between objects will ever happen.
-
+#
 # Define USE_NSEC below if you want git to care about sub-second file mtimes
 # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
 # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
 # randomly break unless your underlying filesystem supports those sub-second
 # times (my ext3 doesn't).
-
+#
 # Define USE_STDEV below if you want git to care about the underlying device
 # change being considered an inode change from the update-cache perspective.
 
@@ -450,6 +453,9 @@ else
 endif
 endif
 endif
+ifdef USE_PIC
+	ALL_CFLAGS += -fPIC
+endif
 ifdef NO_ACCURATE_DIFF
 	ALL_CFLAGS += -DNO_ACCURATE_DIFF
 endif
-- 
1.4.1.rc1.gf2641

^ permalink raw reply related

* Re: [PATCH] Introduce Git.pm (v3)
From: Junio C Hamano @ 2006-06-24  1:04 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060623123904.GL21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

>> By the way, you also need to adjust the testsuite so that it
>> finds the Perl modules from freshly built tree before
>> installing.  I think (but haven't checked yet) the stuff written
>> in Python does that already, so you might want to mimic it.
>
> It should be enough to -I../perl/blib/lib -I../perl/blib/arch/auto/Git.

-- >8 --
[PATCH] Perl interface: make testsuite work again.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 t/test-lib.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 05f6e79..fba0c51 100755
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -206,6 +206,8 @@ PYTHON=`sed -e '1{
 	PYTHONPATH=$(pwd)/../compat
 	export PYTHONPATH
 }
+PERL5LIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
+export PERL5LIB
 test -d ../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
-- 
1.4.1.rc1.gf2641

^ permalink raw reply related

* Re: [PATCH] Introduce Git.pm (v3)
From: Junio C Hamano @ 2006-06-24  1:07 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis, Johannes Schindelin
In-Reply-To: <7vejxf74e3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

>> It should be enough to -I../perl/blib/lib -I../perl/blib/arch/auto/Git.
>
> -- >8 --
> [PATCH] Perl interface: add build-time configuration to allow building with -fPIC
>
> On x86-64 it seems that Git.xs does not link without compiling
> the main git objects with -fPIC.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>

Eh, sorry this was in response to this part of Pasky's message:

>> Well, for the .xs you do need an .so and for that you apparently need
>> -fPIC on most architectures, so there's no way around it.
>> 
>> There's a patch to build libgit.so, would you take it as an excuse to
>> always compile with -fPIC? ;-)

By the way, I noticed NO_ACCURATE_DIFF is a compile time option
to cause git-apply to accept diff output from implementations
that botch "\No newline at the end of file", and I think it is
wrong -- it should be a run time option to git-apply if we would
want to support it, because the version of diff you have does
not have much to do with which implementations of diff were used
to generate patches you would receive and apply.

Thoughts?

^ permalink raw reply

* [Patch] trap: exit: invalid signal specification
From: S.Çağlar Onur @ 2006-06-24  1:10 UTC (permalink / raw)
  To: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

Hi;

This tiny patch is needed to solve

caglar@zangetsu ~ $ git-clone 
/usr/bin/git-clone: line 121: trap: exit: invalid signal specification

error under tr_TR.UTF-8 (and other Turkic locales). For these locales 
upper(i) != I .

P.S: If needed please CC me, i'm not subscribed to list

Signed-off-by: S.Çağlar Onur <caglar@pardus.org.tr>
---

diff -ur git-1.4.0.orig/git-clone.sh git-1.4.0/git-clone.sh
--- git-1.4.0.orig/git-clone.sh 2006-06-10 22:41:54.000000000 +0300
+++ git-1.4.0/git-clone.sh      2006-06-24 03:54:49.000000000 +0300
@@ -205,7 +205,7 @@
 [ -e "$dir" ] && echo "$dir already exists." && usage
 mkdir -p "$dir" &&
 D=$(cd "$dir" && pwd) &&
-trap 'err=$?; cd ..; rm -r "$D"; exit $err' 0
+trap 'err=$?; cd ..; rm -r "$D"; EXIT $err' 0
 case "$bare" in
 yes)
        GIT_DIR="$D" ;;

-- 
S.Çağlar Onur <caglar@pardus.org.tr>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This patch introduces a very basic and barebone Git.pm module
with a sketch of how the generic interface would look like;
most functions are missing, but this should give some good base.
I will continue expanding it.

Most desirable now is more careful error reporting, generic_in() for feeding
input to Git commands and the repository() constructor doing some poking
with git-rev-parse to get the git directory and subdirectory prefix.
Those three are basically the prerequisities for converting git-mv.
I will send them as follow-ups to this patch.

Currently Git.pm just wraps up exec()s of Git commands, but even that
is not trivial to get right and various Git perl scripts do it in
various inconsistent ways. In addition to Git.pm, there is now also
Git.xs which provides barebone Git.xs for directly interfacing with
libgit.a, and as an example providing the hash_object() function using
libgit.

This adds the Git module, integrates it to the build system and as
an example converts the git-fmt-merge-msg.perl script to it (the result
is not very impressive since its advantage is not quite apparent in this
one, but I just picked up the simplest Git user around).

Compared to v3, only very minor things were fixed in this patch (some
whitespaces, a missing export, tiny bug in git-fmt-merge-msg.perl);
at first I wanted to post them as a separate patch but since this
is still only in pu, I decided that it will be cleaner to just resend
the patch.

My current working state is available all the time at

	http://pasky.or.cz/~xpasky/git-perl/Git.pm

and an irregularily updated API documentation is at

	http://pasky.or.cz/~xpasky/git-perl/Git.html

Many thanks to Jakub Narebski, Junio and others for their feedback.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile               |   12 +
 git-fmt-merge-msg.perl |   13 +-
 perl/.gitignore        |    7 +
 perl/Git.pm            |  408 ++++++++++++++++++++++++++++++++++++++++++++++++
 perl/Git.xs            |   64 ++++++++
 perl/Makefile.PL       |   21 ++
 6 files changed, 516 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index a5b6784..4d20b22 100644
--- a/Makefile
+++ b/Makefile
@@ -476,7 +476,8 @@ ### Build rules
 
 all: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk
 
-all:
+all: perl/Makefile
+	$(MAKE) -C perl
 	$(MAKE) -C templates
 
 strip: $(PROGRAMS) git$X
@@ -508,7 +509,7 @@ common-cmds.h: Documentation/git-*.txt
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)) : % : %.perl
 	rm -f $@ $@+
-	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
+	sed -e '1s|#!.*perl\(.*\)|#!$(PERL_PATH_SQ)\1 -I'"$$(make -s -C perl instlibdir)"'|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    $@.perl >$@+
 	chmod +x $@+
@@ -594,6 +595,9 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare
 	rm -f $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
 
+perl/Makefile:	perl/Git.pm perl/Makefile.PL
+	(cd perl && $(PERL_PATH) Makefile.PL PREFIX="$(prefix)" DEFINE="$(ALL_CFLAGS)" LIBS="$(LIBS)")
+
 doc:
 	$(MAKE) -C Documentation all
 
@@ -649,6 +653,7 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates install
+	$(MAKE) -C perl install
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(GIT_PYTHON_DIR_SQ)'
 	$(INSTALL) $(PYMODULES) '$(DESTDIR_SQ)$(GIT_PYTHON_DIR_SQ)'
 	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
@@ -716,7 +721,8 @@ clean:
 	rm -f $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	rm -f $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
-	$(MAKE) -C templates clean
+	[ ! -e perl/Makefile ] || $(MAKE) -C perl/ clean
+	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS
 
diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
index 5986e54..be2a48c 100755
--- a/git-fmt-merge-msg.perl
+++ b/git-fmt-merge-msg.perl
@@ -6,6 +6,9 @@ # Read .git/FETCH_HEAD and make a human 
 # by grouping branches and tags together to form a single line.
 
 use strict;
+use Git;
+
+my $repo = Git->repository();
 
 my @src;
 my %src;
@@ -28,13 +31,12 @@ sub andjoin {
 }
 
 sub repoconfig {
-	my ($val) = qx{git-repo-config --get merge.summary};
+	my ($val) = $repo->command_oneline('repo-config', '--get', 'merge.summary');
 	return $val;
 }
 
 sub current_branch {
-	my ($bra) = qx{git-symbolic-ref HEAD};
-	chomp($bra);
+	my ($bra) = $repo->command_oneline('symbolic-ref', 'HEAD');
 	$bra =~ s|^refs/heads/||;
 	if ($bra ne 'master') {
 		$bra = " into $bra";
@@ -47,11 +49,10 @@ sub current_branch {
 sub shortlog {
 	my ($tip) = @_;
 	my @result;
-	foreach ( qx{git-log --no-merges --topo-order --pretty=oneline $tip ^HEAD} ) {
+	foreach ($repo->command('log', '--no-merges', '--topo-order', '--pretty=oneline', $tip, '^HEAD')) {
 		s/^[0-9a-f]{40}\s+//;
 		push @result, $_;
 	}
-	die "git-log failed\n" if $?;
 	return @result;
 }
 
@@ -168,6 +169,6 @@ for (@origin) {
 			print "  ...\n";
 			last;
 		}
-		print "  $log";
+		print "  $log\n";
 	}
 }
diff --git a/perl/.gitignore b/perl/.gitignore
new file mode 100644
index 0000000..6d778f3
--- /dev/null
+++ b/perl/.gitignore
@@ -0,0 +1,7 @@
+Git.bs
+Git.c
+Makefile
+blib
+blibdirs
+pm_to_blib
+ppport.h
diff --git a/perl/Git.pm b/perl/Git.pm
new file mode 100644
index 0000000..8fff785
--- /dev/null
+++ b/perl/Git.pm
@@ -0,0 +1,408 @@
+=head1 NAME
+
+Git - Perl interface to the Git version control system
+
+=cut
+
+
+package Git;
+
+use strict;
+
+
+BEGIN {
+
+our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
+
+# Totally unstable API.
+$VERSION = '0.01';
+
+
+=head1 SYNOPSIS
+
+  use Git;
+
+  my $version = Git::command_oneline('version');
+
+  Git::command_noisy('update-server-info');
+
+  my $repo = Git->repository (Directory => '/srv/git/cogito.git');
+
+
+  my @revs = $repo->command('rev-list', '--since=last monday', '--all');
+
+  my $fh = $repo->command_pipe('rev-list', '--since=last monday', '--all');
+  my $lastrev = <$fh>; chomp $lastrev;
+  close $fh; # You may want to test rev-list exit status here
+
+  my $lastrev = $repo->command_oneline('rev-list', '--all');
+
+=cut
+
+
+require Exporter;
+
+@ISA = qw(Exporter);
+
+@EXPORT = qw();
+
+# Methods which can be called as standalone functions as well:
+@EXPORT_OK = qw(command command_oneline command_pipe command_noisy
+                hash_object);
+
+
+=head1 DESCRIPTION
+
+This module provides Perl scripts easy way to interface the Git version control
+system. The modules have an easy and well-tested way to call arbitrary Git
+commands; in the future, the interface will also provide specialized methods
+for doing easily operations which are not totally trivial to do over
+the generic command interface.
+
+While some commands can be executed outside of any context (e.g. 'version'
+or 'init-db'), most operations require a repository context, which in practice
+means getting an instance of the Git object using the repository() constructor.
+(In the future, we will also get a new_repository() constructor.) All commands
+called as methods of the object are then executed in the context of the
+repository.
+
+TODO: In the future, we might also do
+
+	my $subdir = $repo->subdir('Documentation');
+	# Gets called in the subdirectory context:
+	$subdir->command('status');
+
+	my $remoterepo = $repo->remote_repository (Name => 'cogito', Branch => 'master');
+	$remoterepo ||= Git->remote_repository ('http://git.or.cz/cogito.git/');
+	my @refs = $remoterepo->refs();
+
+So far, all functions just die if anything goes wrong. If you don't want that,
+make appropriate provisions to catch the possible deaths. Better error recovery
+mechanisms will be provided in the future.
+
+Currently, the module merely wraps calls to external Git tools. In the future,
+it will provide a much faster way to interact with Git by linking directly
+to libgit. This should be completely opaque to the user, though (performance
+increate nonwithstanding).
+
+=cut
+
+
+use Carp qw(carp croak);
+
+require XSLoader;
+XSLoader::load('Git', $VERSION);
+
+}
+
+
+=head1 CONSTRUCTORS
+
+=over 4
+
+=item repository ( OPTIONS )
+
+=item repository ( DIRECTORY )
+
+=item repository ()
+
+Construct a new repository object.
+C<OPTIONS> are passed in a hash like fashion, using key and value pairs.
+Possible options are:
+
+B<Repository> - Path to the Git repository.
+
+B<WorkingCopy> - Path to the associated working copy; not strictly required
+as many commands will happily crunch on a bare repository.
+
+B<Directory> - Path to the Git working directory in its usual setup. This
+is just for convenient setting of both C<Repository> and C<WorkingCopy>
+at once: If the directory as a C<.git> subdirectory, C<Repository> is pointed
+to the subdirectory and the directory is assumed to be the working copy.
+If the directory does not have the subdirectory, C<WorkingCopy> is left
+undefined and C<Repository> is pointed to the directory itself.
+
+B<GitPath> - Path to the C<git> binary executable. By default the C<$PATH>
+is searched for it.
+
+You should not use both C<Directory> and either of C<Repository> and
+C<WorkingCopy> - the results of that are undefined.
+
+Alternatively, a directory path may be passed as a single scalar argument
+to the constructor; it is equivalent to setting only the C<Directory> option
+field.
+
+Calling the constructor with no options whatsoever is equivalent to
+calling it with C<< Directory => '.' >>.
+
+=cut
+
+sub repository {
+	my $class = shift;
+	my @args = @_;
+	my %opts = ();
+	my $self;
+
+	if (defined $args[0]) {
+		if ($#args % 2 != 1) {
+			# Not a hash.
+			$#args == 0 or croak "bad usage";
+			%opts = (Directory => $args[0]);
+		} else {
+			%opts = @args;
+		}
+
+		if ($opts{Directory}) {
+			-d $opts{Directory} or croak "Directory not found: $!";
+			if (-d $opts{Directory}."/.git") {
+				# TODO: Might make this more clever
+				$opts{WorkingCopy} = $opts{Directory};
+				$opts{Repository} = $opts{Directory}."/.git";
+			} else {
+				$opts{Repository} = $opts{Directory};
+			}
+			delete $opts{Directory};
+		}
+	}
+
+	$self = { opts => \%opts };
+	bless $self, $class;
+}
+
+
+=back
+
+=head1 METHODS
+
+=over 4
+
+=item command ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given Git C<COMMAND> (specify it without the 'git-'
+prefix), optionally with the specified extra C<ARGUMENTS>.
+
+The method can be called without any instance or on a specified Git repository
+(in that case the command will be run in the repository context).
+
+In scalar context, it returns all the command output in a single string
+(verbatim).
+
+In array context, it returns an array containing lines printed to the
+command's stdout (without trailing newlines).
+
+In both cases, the command's stdin and stderr are the same as the caller's.
+
+=cut
+
+sub command {
+	my $fh = command_pipe(@_);
+
+	if (not defined wantarray) {
+		_cmd_close($fh);
+
+	} elsif (not wantarray) {
+		local $/;
+		my $text = <$fh>;
+		_cmd_close($fh);
+		return $text;
+
+	} else {
+		my @lines = <$fh>;
+		_cmd_close($fh);
+		chomp @lines;
+		return @lines;
+	}
+}
+
+
+=item command_oneline ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given C<COMMAND> in the same way as command()
+does but always return a scalar string containing the first line
+of the command's standard output.
+
+=cut
+
+sub command_oneline {
+	my $fh = command_pipe(@_);
+
+	my $line = <$fh>;
+	_cmd_close($fh);
+
+	chomp $line;
+	return $line;
+}
+
+
+=item command_pipe ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given C<COMMAND> in the same way as command()
+does but return a pipe filehandle from which the command output can be
+read.
+
+=cut
+
+sub command_pipe {
+	my ($self, $cmd, @args) = _maybe_self(@_);
+
+	$cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+
+	my $pid = open(my $fh, "-|");
+	if (not defined $pid) {
+		croak "open failed: $!";
+	} elsif ($pid == 0) {
+		_cmd_exec($self, $cmd, @args);
+	}
+	return $fh;
+}
+
+
+=item command_noisy ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given C<COMMAND> in the same way as command() does but do not
+capture the command output - the standard output is not redirected and goes
+to the standard output of the caller application.
+
+While the method is called command_noisy(), you might want to as well use
+it for the most silent Git commands which you know will never pollute your
+stdout but you want to avoid the overhead of the pipe setup when calling them.
+
+The function returns only after the command has finished running.
+
+=cut
+
+sub command_noisy {
+	my ($self, $cmd, @args) = _maybe_self(@_);
+
+	$cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+
+	my $pid = fork;
+	if (not defined $pid) {
+		croak "fork failed: $!";
+	} elsif ($pid == 0) {
+		_cmd_exec($self, $cmd, @args);
+	}
+	if (waitpid($pid, 0) > 0 and $? != 0) {
+		croak "exit status: $?";
+	}
+}
+
+
+=item hash_object ( FILENAME [, TYPE ] )
+
+=item hash_object ( FILEHANDLE [, TYPE ] )
+
+Compute the SHA1 object id of the given C<FILENAME> (or data waiting in
+C<FILEHANDLE>) considering it is of the C<TYPE> object type (C<blob>
+(default), C<commit>, C<tree>).
+
+In case of C<FILEHANDLE> passed instead of file name, all the data
+available are read and hashed, and the filehandle is automatically
+closed. The file handle should be freshly opened - if you have already
+read anything from the file handle, the results are undefined (since
+this function works directly with the file descriptor and internal
+PerlIO buffering might have messed things up).
+
+The method can be called without any instance or on a specified Git repository,
+it makes zero difference.
+
+The function returns the SHA1 hash.
+
+Implementation of this function is very fast; no external command calls
+are involved.
+
+=cut
+
+# Implemented in Git.xs.
+
+
+=back
+
+=head1 TODO
+
+This is still fairly crude.
+We need some good way to report errors back except just dying.
+
+=head1 COPYRIGHT
+
+Copyright 2006 by Petr Baudis E<lt>pasky@suse.czE<gt>.
+
+This module is free software; it may be used, copied, modified
+and distributed under the terms of the GNU General Public Licence,
+either version 2, or (at your option) any later version.
+
+=cut
+
+
+# Take raw method argument list and return ($obj, @args) in case
+# the method was called upon an instance and (undef, @args) if
+# it was called directly.
+sub _maybe_self {
+	# This breaks inheritance. Oh well.
+	ref $_[0] eq 'Git' ? @_ : (undef, @_);
+}
+
+# When already in the subprocess, set up the appropriate state
+# for the given repository and execute the git command.
+sub _cmd_exec {
+	my ($self, @args) = @_;
+	if ($self) {
+		$self->{opts}->{Repository} and $ENV{'GIT_DIR'} = $self->{opts}->{Repository};
+		$self->{opts}->{WorkingCopy} and chdir($self->{opts}->{WorkingCopy});
+	}
+	my $git = $self->{opts}->{GitPath};
+	$git ||= 'git';
+	exec ($git, @args) or croak "exec failed: $!";
+}
+
+# Close pipe to a subprocess.
+sub _cmd_close {
+	my ($fh) = @_;
+	if (not close $fh) {
+		if ($!) {
+			# It's just close, no point in fatalities
+			carp "error closing pipe: $!";
+		} elsif ($? >> 8) {
+			croak "exit status: ".($? >> 8);
+		}
+		# else we might e.g. closed a live stream; the command
+		# dying of SIGPIPE would drive us here.
+	}
+}
+
+
+# Trickery for .xs routines: In order to avoid having some horrid
+# C code trying to do stuff with undefs and hashes, we gate all
+# xs calls through the following and in case we are being ran upon
+# an instance call a C part of the gate which will set up the
+# environment properly.
+sub _call_gate {
+	my $xsfunc = shift;
+	my ($self, @args) = _maybe_self(@_);
+
+	if (defined $self) {
+		# XXX: We ignore the WorkingCopy! To properly support
+		# that will require heavy changes in libgit.
+
+		# XXX: And we ignore everything else as well. libgit
+		# at least needs to be extended to let us specify
+		# the $GIT_DIR instead of looking it up in environment.
+		#xs_call_gate($self->{opts}->{Repository});
+	}
+
+	&$xsfunc(@args);
+}
+
+sub AUTOLOAD {
+	my $xsname;
+	our $AUTOLOAD;
+	($xsname = $AUTOLOAD) =~ s/.*:://;
+	croak "&Git::$xsname not defined" if $xsname =~ /^xs_/;
+	$xsname = 'xs_'.$xsname;
+	_call_gate(\&$xsname, @_);
+}
+
+sub DESTROY { }
+
+
+1; # Famous last words
diff --git a/perl/Git.xs b/perl/Git.xs
new file mode 100644
index 0000000..9885e2c
--- /dev/null
+++ b/perl/Git.xs
@@ -0,0 +1,64 @@
+/* By carefully stacking #includes here (even if WE don't really need them)
+ * we strive to make the thing actually compile. Git header files aren't very
+ * nice. Perl headers are one of the signs of the coming apocalypse. */
+#include <ctype.h>
+/* Ok, it hasn't been so bad so far. */
+
+/* libgit interface */
+#include "../cache.h"
+
+/* XS and Perl interface */
+#include "EXTERN.h"
+#include "perl.h"
+#include "XSUB.h"
+
+#include "ppport.h"
+
+
+MODULE = Git		PACKAGE = Git		
+
+PROTOTYPES: DISABLE
+
+# /* TODO: xs_call_gate(). See Git.pm. */
+
+char *
+xs_hash_object(file, type = "blob")
+	SV *file;
+	char *type;
+CODE:
+{
+	unsigned char sha1[20];
+
+	if (SvTYPE(file) == SVt_RV)
+		file = SvRV(file);
+
+	if (SvTYPE(file) == SVt_PVGV) {
+		/* Filehandle */
+		PerlIO *pio;
+
+		pio = IoIFP(sv_2io(file));
+		if (!pio)
+			croak("You passed me something weird - a dir glob?");
+		/* XXX: I just hope PerlIO didn't read anything from it yet.
+		 * --pasky */
+		if (index_pipe(sha1, PerlIO_fileno(pio), type, 0))
+			croak("Unable to hash given filehandle");
+		/* Avoid any nasty surprises. */
+		PerlIO_close(pio);
+
+	} else {
+		/* String */
+		char *path = SvPV_nolen(file);
+		int fd = open(path, O_RDONLY);
+		struct stat st;
+
+		if (fd < 0 ||
+		    fstat(fd, &st) < 0 ||
+		    index_fd(sha1, fd, &st, 0, type))
+			croak("Unable to hash %s", path);
+		close(fd);
+	}
+	RETVAL = sha1_to_hex(sha1);
+}
+OUTPUT:
+	RETVAL
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
new file mode 100644
index 0000000..dd61056
--- /dev/null
+++ b/perl/Makefile.PL
@@ -0,0 +1,21 @@
+use ExtUtils::MakeMaker;
+
+sub MY::postamble {
+	return <<'MAKE_FRAG';
+instlibdir:
+	@echo $(INSTALLSITELIB)
+
+MAKE_FRAG
+}
+
+WriteMakefile(
+	NAME            => 'Git',
+	VERSION_FROM    => 'Git.pm',
+	MYEXTLIB        => '../libgit.a',
+	INC             => '-I. -I..',
+);
+
+
+use Devel::PPPort;
+
+-s 'ppport.h' or Devel::PPPort::WriteFile();

^ permalink raw reply related

* [PATCH 02/12] Git.pm: Implement Git::exec_path()
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

This patch implements Git::exec_path() (as a direct XS call).

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 perl/Git.pm |   15 ++++++++++++++-
 perl/Git.xs |   12 ++++++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 8fff785..5c5ae12 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -48,7 +48,7 @@ require Exporter;
 
 # Methods which can be called as standalone functions as well:
 @EXPORT_OK = qw(command command_oneline command_pipe command_noisy
-                hash_object);
+                exec_path hash_object);
 
 
 =head1 DESCRIPTION
@@ -288,6 +288,19 @@ sub command_noisy {
 }
 
 
+=item exec_path ()
+
+Return path to the git sub-command executables (the same as
+C<git --exec-path>). Useful mostly only internally.
+
+Implementation of this function is very fast; no external command calls
+are involved.
+
+=cut
+
+# Implemented in Git.xs.
+
+
 =item hash_object ( FILENAME [, TYPE ] )
 
 =item hash_object ( FILEHANDLE [, TYPE ] )
diff --git a/perl/Git.xs b/perl/Git.xs
index 9885e2c..b6f6d13 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -6,6 +6,7 @@ #include <ctype.h>
 
 /* libgit interface */
 #include "../cache.h"
+#include "../exec_cmd.h"
 
 /* XS and Perl interface */
 #include "EXTERN.h"
@@ -21,6 +22,17 @@ PROTOTYPES: DISABLE
 
 # /* TODO: xs_call_gate(). See Git.pm. */
 
+
+const char *
+xs_exec_path()
+CODE:
+{
+	RETVAL = git_exec_path();
+}
+OUTPUT:
+	RETVAL
+
+
 char *
 xs_hash_object(file, type = "blob")
 	SV *file;

^ permalink raw reply related

* [PATCH 03/12] Git.pm: Call external commands using execv_git_cmd()
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

Instead of explicitly using the git wrapper to call external commands,
use the execv_git_cmd() function which will directly call whatever
needs to be called. GitBin option becomes useless so drop it.

This actually means the exec_path() thing I planned to use worthless
internally, but Jakub wants it in anyway and I don't mind, so...

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 perl/Git.pm |   12 ++++++------
 perl/Git.xs |   22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 5c5ae12..212337e 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -122,9 +122,6 @@ to the subdirectory and the directory is
 If the directory does not have the subdirectory, C<WorkingCopy> is left
 undefined and C<Repository> is pointed to the directory itself.
 
-B<GitPath> - Path to the C<git> binary executable. By default the C<$PATH>
-is searched for it.
-
 You should not use both C<Directory> and either of C<Repository> and
 C<WorkingCopy> - the results of that are undefined.
 
@@ -363,11 +360,14 @@ sub _cmd_exec {
 		$self->{opts}->{Repository} and $ENV{'GIT_DIR'} = $self->{opts}->{Repository};
 		$self->{opts}->{WorkingCopy} and chdir($self->{opts}->{WorkingCopy});
 	}
-	my $git = $self->{opts}->{GitPath};
-	$git ||= 'git';
-	exec ($git, @args) or croak "exec failed: $!";
+	xs__execv_git_cmd(@args);
+	croak "exec failed: $!";
 }
 
+# Execute the given Git command ($_[0]) with arguments ($_[1..])
+# by searching for it at proper places.
+# _execv_git_cmd(), implemented in Git.xs.
+
 # Close pipe to a subprocess.
 sub _cmd_close {
 	my ($fh) = @_;
diff --git a/perl/Git.xs b/perl/Git.xs
index b6f6d13..c8220e2 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -33,6 +33,28 @@ OUTPUT:
 	RETVAL
 
 
+void
+xs__execv_git_cmd(...)
+CODE:
+{
+	const char **argv;
+	int i;
+
+	argv = malloc(sizeof(const char *) * (items + 1));
+	if (!argv)
+		croak("malloc failed");
+	for (i = 0; i < items; i++)
+		argv[i] = strdup(SvPV_nolen(ST(i)));
+	argv[i] = NULL;
+
+	execv_git_cmd(argv);
+
+	for (i = 0; i < items; i++)
+		if (argv[i])
+			free((char *) argv[i]);
+	free((char **) argv);
+}
+
 char *
 xs_hash_object(file, type = "blob")
 	SV *file;

^ permalink raw reply related

* [PATCH 05/12] Customizable error handlers
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

This patch makes the usage(), die() and error() handlers customizable.
Nothing in the git code itself uses that but many other libgit users
(like Git.pm) will.

This is implemented using the mutator functions primarily because you
cannot directly modifying global variables of libgit from a program that
dlopen()ed it, apparently. But having functions for that is a better API
anyway.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 git-compat-util.h |    4 ++++
 usage.c           |   46 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 5d543d2..b3d4cf5 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -40,6 +40,10 @@ extern void usage(const char *err) NORET
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
+extern void set_usage_routine(void (*routine)(const char *err) NORETURN);
+extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
+extern void set_error_routine(void (*routine)(const char *err, va_list params));
+
 #ifdef NO_MMAP
 
 #ifndef PROT_READ
diff --git a/usage.c b/usage.c
index 1fa924c..b781b00 100644
--- a/usage.c
+++ b/usage.c
@@ -12,20 +12,58 @@ static void report(const char *prefix, c
 	fputs("\n", stderr);
 }
 
-void usage(const char *err)
+void usage_builtin(const char *err)
 {
 	fprintf(stderr, "usage: %s\n", err);
 	exit(129);
 }
 
+void die_builtin(const char *err, va_list params)
+{
+	report("fatal: ", err, params);
+	exit(128);
+}
+
+void error_builtin(const char *err, va_list params)
+{
+	report("error: ", err, params);
+}
+
+
+/* If we are in a dlopen()ed .so write to a global variable would segfault
+ * (ugh), so keep things static. */
+static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
+static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
+static void (*error_routine)(const char *err, va_list params) = error_builtin;
+
+void set_usage_routine(void (*routine)(const char *err) NORETURN)
+{
+	usage_routine = routine;
+}
+
+void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
+{
+	die_routine = routine;
+}
+
+void set_error_routine(void (*routine)(const char *err, va_list params))
+{
+	error_routine = routine;
+}
+
+
+void usage(const char *err)
+{
+	usage_routine(err);
+}
+
 void die(const char *err, ...)
 {
 	va_list params;
 
 	va_start(params, err);
-	report("fatal: ", err, params);
+	die_routine(err, params);
 	va_end(params);
-	exit(128);
 }
 
 int error(const char *err, ...)
@@ -33,7 +71,7 @@ int error(const char *err, ...)
 	va_list params;
 
 	va_start(params, err);
-	report("error: ", err, params);
+	error_routine(err, params);
 	va_end(params);
 	return -1;
 }

^ permalink raw reply related

* [PATCH 04/12] Git.pm: Implement Git::version()
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

Git::version() returns the Git version string.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile    |    5 ++++-
 perl/Git.pm |   14 +++++++++++++-
 perl/Git.xs |   10 ++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 4d20b22..7842195 100644
--- a/Makefile
+++ b/Makefile
@@ -596,7 +596,10 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare
 
 
 perl/Makefile:	perl/Git.pm perl/Makefile.PL
-	(cd perl && $(PERL_PATH) Makefile.PL PREFIX="$(prefix)" DEFINE="$(ALL_CFLAGS)" LIBS="$(LIBS)")
+	(cd perl && $(PERL_PATH) Makefile.PL \
+		PREFIX="$(prefix)" \
+		DEFINE="$(ALL_CFLAGS) -DGIT_VERSION=\\\"$(GIT_VERSION)\\\"" \
+		LIBS="$(LIBS)")
 
 doc:
 	$(MAKE) -C Documentation all
diff --git a/perl/Git.pm b/perl/Git.pm
index 212337e..dcd769b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -48,7 +48,7 @@ require Exporter;
 
 # Methods which can be called as standalone functions as well:
 @EXPORT_OK = qw(command command_oneline command_pipe command_noisy
-                exec_path hash_object);
+                version exec_path hash_object);
 
 
 =head1 DESCRIPTION
@@ -285,6 +285,18 @@ sub command_noisy {
 }
 
 
+=item version ()
+
+Return the Git version in use.
+
+Implementation of this function is very fast; no external command calls
+are involved.
+
+=cut
+
+# Implemented in Git.xs.
+
+
 =item exec_path ()
 
 Return path to the git sub-command executables (the same as
diff --git a/perl/Git.xs b/perl/Git.xs
index c8220e2..9623fdd 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -24,6 +24,16 @@ # /* TODO: xs_call_gate(). See Git.pm. *
 
 
 const char *
+xs_version()
+CODE:
+{
+	RETVAL = GIT_VERSION;
+}
+OUTPUT:
+	RETVAL
+
+
+const char *
 xs_exec_path()
 CODE:
 {

^ permalink raw reply related

* [PATCH 06/12] Add Error.pm to the distribution
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

I have been thinking about how to do the error reporting the best
way and after scraping various overcomplicated concepts, I have
decided that by far the most elegant way is to throw Error exceptions;
the closest sane alternative is to catch the dies in Git.pm by
enclosing the calls in eval{}s and that's really _quite_ ugly.

The only "small" trouble is that Error.pm turns out sadly not to be
part of the standard distribution, and installation from CPAN is
a bother, especially if you can't install it system-wide. But since
it is very small, I've decided to just bundle it.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 perl/Error.pm    |  821 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 perl/Makefile.PL |   10 +
 2 files changed, 831 insertions(+), 0 deletions(-)

diff --git a/perl/Error.pm b/perl/Error.pm
new file mode 100644
index 0000000..17ad70a
--- /dev/null
+++ b/perl/Error.pm
@@ -0,0 +1,821 @@
+# Error.pm
+#
+# Copyright (c) 1997-8 Graham Barr <gbarr@ti.com>. All rights reserved.
+# This program is free software; you can redistribute it and/or
+# modify it under the same terms as Perl itself.
+#
+# Based on my original Error.pm, and Exceptions.pm by Peter Seibel
+# <peter@weblogic.com> and adapted by Jesse Glick <jglick@sig.bsh.com>.
+#
+# but modified ***significantly***
+
+package Error;
+
+use strict;
+use vars qw($VERSION);
+use 5.004;
+
+$VERSION = "0.15009"; 
+
+use overload (
+	'""'	   =>	'stringify',
+	'0+'	   =>	'value',
+	'bool'     =>	sub { return 1; },
+	'fallback' =>	1
+);
+
+$Error::Depth = 0;	# Depth to pass to caller()
+$Error::Debug = 0;	# Generate verbose stack traces
+@Error::STACK = ();	# Clause stack for try
+$Error::THROWN = undef;	# last error thrown, a workaround until die $ref works
+
+my $LAST;		# Last error created
+my %ERROR;		# Last error associated with package
+
+sub throw_Error_Simple
+{
+    my $args = shift;
+    return Error::Simple->new($args->{'text'});
+}
+
+$Error::ObjectifyCallback = \&throw_Error_Simple;
+
+
+# Exported subs are defined in Error::subs
+
+use Scalar::Util ();
+
+sub import {
+    shift;
+    local $Exporter::ExportLevel = $Exporter::ExportLevel + 1;
+    Error::subs->import(@_);
+}
+
+# I really want to use last for the name of this method, but it is a keyword
+# which prevent the syntax  last Error
+
+sub prior {
+    shift; # ignore
+
+    return $LAST unless @_;
+
+    my $pkg = shift;
+    return exists $ERROR{$pkg} ? $ERROR{$pkg} : undef
+	unless ref($pkg);
+
+    my $obj = $pkg;
+    my $err = undef;
+    if($obj->isa('HASH')) {
+	$err = $obj->{'__Error__'}
+	    if exists $obj->{'__Error__'};
+    }
+    elsif($obj->isa('GLOB')) {
+	$err = ${*$obj}{'__Error__'}
+	    if exists ${*$obj}{'__Error__'};
+    }
+
+    $err;
+}
+
+sub flush {
+    shift; #ignore
+    
+    unless (@_) {
+       $LAST = undef;
+       return;
+    }
+    
+    my $pkg = shift;
+    return unless ref($pkg);
+   
+    undef $ERROR{$pkg} if defined $ERROR{$pkg}; 
+} 
+
+# Return as much information as possible about where the error
+# happened. The -stacktrace element only exists if $Error::DEBUG
+# was set when the error was created
+
+sub stacktrace {
+    my $self = shift;
+
+    return $self->{'-stacktrace'}
+	if exists $self->{'-stacktrace'};
+
+    my $text = exists $self->{'-text'} ? $self->{'-text'} : "Died";
+
+    $text .= sprintf(" at %s line %d.\n", $self->file, $self->line)
+	unless($text =~ /\n$/s);
+
+    $text;
+}
+
+# Allow error propagation, ie
+#
+# $ber->encode(...) or
+#    return Error->prior($ber)->associate($ldap);
+
+sub associate {
+    my $err = shift;
+    my $obj = shift;
+
+    return unless ref($obj);
+
+    if($obj->isa('HASH')) {
+	$obj->{'__Error__'} = $err;
+    }
+    elsif($obj->isa('GLOB')) {
+	${*$obj}{'__Error__'} = $err;
+    }
+    $obj = ref($obj);
+    $ERROR{ ref($obj) } = $err;
+
+    return;
+}
+
+sub new {
+    my $self = shift;
+    my($pkg,$file,$line) = caller($Error::Depth);
+
+    my $err = bless {
+	'-package' => $pkg,
+	'-file'    => $file,
+	'-line'    => $line,
+	@_
+    }, $self;
+
+    $err->associate($err->{'-object'})
+	if(exists $err->{'-object'});
+
+    # To always create a stacktrace would be very inefficient, so
+    # we only do it if $Error::Debug is set
+
+    if($Error::Debug) {
+	require Carp;
+	local $Carp::CarpLevel = $Error::Depth;
+	my $text = defined($err->{'-text'}) ? $err->{'-text'} : "Error";
+	my $trace = Carp::longmess($text);
+	# Remove try calls from the trace
+	$trace =~ s/(\n\s+\S+__ANON__[^\n]+)?\n\s+eval[^\n]+\n\s+Error::subs::try[^\n]+(?=\n)//sog;
+	$trace =~ s/(\n\s+\S+__ANON__[^\n]+)?\n\s+eval[^\n]+\n\s+Error::subs::run_clauses[^\n]+\n\s+Error::subs::try[^\n]+(?=\n)//sog;
+	$err->{'-stacktrace'} = $trace
+    }
+
+    $@ = $LAST = $ERROR{$pkg} = $err;
+}
+
+# Throw an error. this contains some very gory code.
+
+sub throw {
+    my $self = shift;
+    local $Error::Depth = $Error::Depth + 1;
+
+    # if we are not rethrow-ing then create the object to throw
+    $self = $self->new(@_) unless ref($self);
+    
+    die $Error::THROWN = $self;
+}
+
+# syntactic sugar for
+#
+#    die with Error( ... );
+
+sub with {
+    my $self = shift;
+    local $Error::Depth = $Error::Depth + 1;
+
+    $self->new(@_);
+}
+
+# syntactic sugar for
+#
+#    record Error( ... ) and return;
+
+sub record {
+    my $self = shift;
+    local $Error::Depth = $Error::Depth + 1;
+
+    $self->new(@_);
+}
+
+# catch clause for
+#
+# try { ... } catch CLASS with { ... }
+
+sub catch {
+    my $pkg = shift;
+    my $code = shift;
+    my $clauses = shift || {};
+    my $catch = $clauses->{'catch'} ||= [];
+
+    unshift @$catch,  $pkg, $code;
+
+    $clauses;
+}
+
+# Object query methods
+
+sub object {
+    my $self = shift;
+    exists $self->{'-object'} ? $self->{'-object'} : undef;
+}
+
+sub file {
+    my $self = shift;
+    exists $self->{'-file'} ? $self->{'-file'} : undef;
+}
+
+sub line {
+    my $self = shift;
+    exists $self->{'-line'} ? $self->{'-line'} : undef;
+}
+
+sub text {
+    my $self = shift;
+    exists $self->{'-text'} ? $self->{'-text'} : undef;
+}
+
+# overload methods
+
+sub stringify {
+    my $self = shift;
+    defined $self->{'-text'} ? $self->{'-text'} : "Died";
+}
+
+sub value {
+    my $self = shift;
+    exists $self->{'-value'} ? $self->{'-value'} : undef;
+}
+
+package Error::Simple;
+
+@Error::Simple::ISA = qw(Error);
+
+sub new {
+    my $self  = shift;
+    my $text  = "" . shift;
+    my $value = shift;
+    my(@args) = ();
+
+    local $Error::Depth = $Error::Depth + 1;
+
+    @args = ( -file => $1, -line => $2)
+	if($text =~ s/\s+at\s+(\S+)\s+line\s+(\d+)(?:,\s*<[^>]*>\s+line\s+\d+)?\.?\n?$//s);
+    push(@args, '-value', 0 + $value)
+	if defined($value);
+
+    $self->SUPER::new(-text => $text, @args);
+}
+
+sub stringify {
+    my $self = shift;
+    my $text = $self->SUPER::stringify;
+    $text .= sprintf(" at %s line %d.\n", $self->file, $self->line)
+	unless($text =~ /\n$/s);
+    $text;
+}
+
+##########################################################################
+##########################################################################
+
+# Inspired by code from Jesse Glick <jglick@sig.bsh.com> and
+# Peter Seibel <peter@weblogic.com>
+
+package Error::subs;
+
+use Exporter ();
+use vars qw(@EXPORT_OK @ISA %EXPORT_TAGS);
+
+@EXPORT_OK   = qw(try with finally except otherwise);
+%EXPORT_TAGS = (try => \@EXPORT_OK);
+
+@ISA = qw(Exporter);
+
+sub run_clauses ($$$\@) {
+    my($clauses,$err,$wantarray,$result) = @_;
+    my $code = undef;
+
+    $err = $Error::ObjectifyCallback->({'text' =>$err}) unless ref($err);
+
+    CATCH: {
+
+	# catch
+	my $catch;
+	if(defined($catch = $clauses->{'catch'})) {
+	    my $i = 0;
+
+	    CATCHLOOP:
+	    for( ; $i < @$catch ; $i += 2) {
+		my $pkg = $catch->[$i];
+		unless(defined $pkg) {
+		    #except
+		    splice(@$catch,$i,2,$catch->[$i+1]->());
+		    $i -= 2;
+		    next CATCHLOOP;
+		}
+		elsif(Scalar::Util::blessed($err) && $err->isa($pkg)) {
+		    $code = $catch->[$i+1];
+		    while(1) {
+			my $more = 0;
+			local($Error::THROWN);
+			my $ok = eval {
+			    if($wantarray) {
+				@{$result} = $code->($err,\$more);
+			    }
+			    elsif(defined($wantarray)) {
+			        @{$result} = ();
+				$result->[0] = $code->($err,\$more);
+			    }
+			    else {
+				$code->($err,\$more);
+			    }
+			    1;
+			};
+			if( $ok ) {
+			    next CATCHLOOP if $more;
+			    undef $err;
+			}
+			else {
+			    $err = defined($Error::THROWN)
+				    ? $Error::THROWN : $@;
+                $err = $Error::ObjectifyCallback->({'text' =>$err})
+                    unless ref($err);
+			}
+			last CATCH;
+		    };
+		}
+	    }
+	}
+
+	# otherwise
+	my $owise;
+	if(defined($owise = $clauses->{'otherwise'})) {
+	    my $code = $clauses->{'otherwise'};
+	    my $more = 0;
+	    my $ok = eval {
+		if($wantarray) {
+		    @{$result} = $code->($err,\$more);
+		}
+		elsif(defined($wantarray)) {
+		    @{$result} = ();
+		    $result->[0] = $code->($err,\$more);
+		}
+		else {
+		    $code->($err,\$more);
+		}
+		1;
+	    };
+	    if( $ok ) {
+		undef $err;
+	    }
+	    else {
+		$err = defined($Error::THROWN)
+			? $Error::THROWN : $@;
+            
+        $err = $Error::ObjectifyCallback->({'text' =>$err}) 
+            unless ref($err);
+	    }
+	}
+    }
+    $err;
+}
+
+sub try (&;$) {
+    my $try = shift;
+    my $clauses = @_ ? shift : {};
+    my $ok = 0;
+    my $err = undef;
+    my @result = ();
+
+    unshift @Error::STACK, $clauses;
+
+    my $wantarray = wantarray();
+
+    do {
+	local $Error::THROWN = undef;
+    local $@ = undef;
+
+	$ok = eval {
+	    if($wantarray) {
+		@result = $try->();
+	    }
+	    elsif(defined $wantarray) {
+		$result[0] = $try->();
+	    }
+	    else {
+		$try->();
+	    }
+	    1;
+	};
+
+	$err = defined($Error::THROWN) ? $Error::THROWN : $@
+	    unless $ok;
+    };
+
+    shift @Error::STACK;
+
+    $err = run_clauses($clauses,$err,wantarray,@result)
+	unless($ok);
+
+    $clauses->{'finally'}->()
+	if(defined($clauses->{'finally'}));
+
+    if (defined($err))
+    {
+        if (Scalar::Util::blessed($err) && $err->can('throw'))
+        {
+            throw $err;
+        }
+        else
+        {
+            die $err;
+        }
+    }
+
+    wantarray ? @result : $result[0];
+}
+
+# Each clause adds a sub to the list of clauses. The finally clause is
+# always the last, and the otherwise clause is always added just before
+# the finally clause.
+#
+# All clauses, except the finally clause, add a sub which takes one argument
+# this argument will be the error being thrown. The sub will return a code ref
+# if that clause can handle that error, otherwise undef is returned.
+#
+# The otherwise clause adds a sub which unconditionally returns the users
+# code reference, this is why it is forced to be last.
+#
+# The catch clause is defined in Error.pm, as the syntax causes it to
+# be called as a method
+
+sub with (&;$) {
+    @_
+}
+
+sub finally (&) {
+    my $code = shift;
+    my $clauses = { 'finally' => $code };
+    $clauses;
+}
+
+# The except clause is a block which returns a hashref or a list of
+# key-value pairs, where the keys are the classes and the values are subs.
+
+sub except (&;$) {
+    my $code = shift;
+    my $clauses = shift || {};
+    my $catch = $clauses->{'catch'} ||= [];
+    
+    my $sub = sub {
+	my $ref;
+	my(@array) = $code->($_[0]);
+	if(@array == 1 && ref($array[0])) {
+	    $ref = $array[0];
+	    $ref = [ %$ref ]
+		if(UNIVERSAL::isa($ref,'HASH'));
+	}
+	else {
+	    $ref = \@array;
+	}
+	@$ref
+    };
+
+    unshift @{$catch}, undef, $sub;
+
+    $clauses;
+}
+
+sub otherwise (&;$) {
+    my $code = shift;
+    my $clauses = shift || {};
+
+    if(exists $clauses->{'otherwise'}) {
+	require Carp;
+	Carp::croak("Multiple otherwise clauses");
+    }
+
+    $clauses->{'otherwise'} = $code;
+
+    $clauses;
+}
+
+1;
+__END__
+
+=head1 NAME
+
+Error - Error/exception handling in an OO-ish way
+
+=head1 SYNOPSIS
+
+    use Error qw(:try);
+
+    throw Error::Simple( "A simple error");
+
+    sub xyz {
+        ...
+	record Error::Simple("A simple error")
+	    and return;
+    }
+ 
+    unlink($file) or throw Error::Simple("$file: $!",$!);
+
+    try {
+	do_some_stuff();
+	die "error!" if $condition;
+	throw Error::Simple -text => "Oops!" if $other_condition;
+    }
+    catch Error::IO with {
+	my $E = shift;
+	print STDERR "File ", $E->{'-file'}, " had a problem\n";
+    }
+    except {
+	my $E = shift;
+	my $general_handler=sub {send_message $E->{-description}};
+	return {
+	    UserException1 => $general_handler,
+	    UserException2 => $general_handler
+	};
+    }
+    otherwise {
+	print STDERR "Well I don't know what to say\n";
+    }
+    finally {
+	close_the_garage_door_already(); # Should be reliable
+    }; # Don't forget the trailing ; or you might be surprised
+
+=head1 DESCRIPTION
+
+The C<Error> package provides two interfaces. Firstly C<Error> provides
+a procedural interface to exception handling. Secondly C<Error> is a
+base class for errors/exceptions that can either be thrown, for
+subsequent catch, or can simply be recorded.
+
+Errors in the class C<Error> should not be thrown directly, but the
+user should throw errors from a sub-class of C<Error>.
+
+=head1 PROCEDURAL INTERFACE
+
+C<Error> exports subroutines to perform exception handling. These will
+be exported if the C<:try> tag is used in the C<use> line.
+
+=over 4
+
+=item try BLOCK CLAUSES
+
+C<try> is the main subroutine called by the user. All other subroutines
+exported are clauses to the try subroutine.
+
+The BLOCK will be evaluated and, if no error is throw, try will return
+the result of the block.
+
+C<CLAUSES> are the subroutines below, which describe what to do in the
+event of an error being thrown within BLOCK.
+
+=item catch CLASS with BLOCK
+
+This clauses will cause all errors that satisfy C<$err-E<gt>isa(CLASS)>
+to be caught and handled by evaluating C<BLOCK>.
+
+C<BLOCK> will be passed two arguments. The first will be the error
+being thrown. The second is a reference to a scalar variable. If this
+variable is set by the catch block then, on return from the catch
+block, try will continue processing as if the catch block was never
+found.
+
+To propagate the error the catch block may call C<$err-E<gt>throw>
+
+If the scalar reference by the second argument is not set, and the
+error is not thrown. Then the current try block will return with the
+result from the catch block.
+
+=item except BLOCK
+
+When C<try> is looking for a handler, if an except clause is found
+C<BLOCK> is evaluated. The return value from this block should be a
+HASHREF or a list of key-value pairs, where the keys are class names
+and the values are CODE references for the handler of errors of that
+type.
+
+=item otherwise BLOCK
+
+Catch any error by executing the code in C<BLOCK>
+
+When evaluated C<BLOCK> will be passed one argument, which will be the
+error being processed.
+
+Only one otherwise block may be specified per try block
+
+=item finally BLOCK
+
+Execute the code in C<BLOCK> either after the code in the try block has
+successfully completed, or if the try block throws an error then
+C<BLOCK> will be executed after the handler has completed.
+
+If the handler throws an error then the error will be caught, the
+finally block will be executed and the error will be re-thrown.
+
+Only one finally block may be specified per try block
+
+=back
+
+=head1 CLASS INTERFACE
+
+=head2 CONSTRUCTORS
+
+The C<Error> object is implemented as a HASH. This HASH is initialized
+with the arguments that are passed to it's constructor. The elements
+that are used by, or are retrievable by the C<Error> class are listed
+below, other classes may add to these.
+
+	-file
+	-line
+	-text
+	-value
+	-object
+
+If C<-file> or C<-line> are not specified in the constructor arguments
+then these will be initialized with the file name and line number where
+the constructor was called from.
+
+If the error is associated with an object then the object should be
+passed as the C<-object> argument. This will allow the C<Error> package
+to associate the error with the object.
+
+The C<Error> package remembers the last error created, and also the
+last error associated with a package. This could either be the last
+error created by a sub in that package, or the last error which passed
+an object blessed into that package as the C<-object> argument.
+
+=over 4
+
+=item throw ( [ ARGS ] )
+
+Create a new C<Error> object and throw an error, which will be caught
+by a surrounding C<try> block, if there is one. Otherwise it will cause
+the program to exit.
+
+C<throw> may also be called on an existing error to re-throw it.
+
+=item with ( [ ARGS ] )
+
+Create a new C<Error> object and returns it. This is defined for
+syntactic sugar, eg
+
+    die with Some::Error ( ... );
+
+=item record ( [ ARGS ] )
+
+Create a new C<Error> object and returns it. This is defined for
+syntactic sugar, eg
+
+    record Some::Error ( ... )
+	and return;
+
+=back
+
+=head2 STATIC METHODS
+
+=over 4
+
+=item prior ( [ PACKAGE ] )
+
+Return the last error created, or the last error associated with
+C<PACKAGE>
+
+=item flush ( [ PACKAGE ] )
+
+Flush the last error created, or the last error associated with
+C<PACKAGE>.It is necessary to clear the error stack before exiting the
+package or uncaught errors generated using C<record> will be reported.
+
+     $Error->flush;
+
+=cut
+
+=back
+
+=head2 OBJECT METHODS
+
+=over 4
+
+=item stacktrace
+
+If the variable C<$Error::Debug> was non-zero when the error was
+created, then C<stacktrace> returns a string created by calling
+C<Carp::longmess>. If the variable was zero the C<stacktrace> returns
+the text of the error appended with the filename and line number of
+where the error was created, providing the text does not end with a
+newline.
+
+=item object
+
+The object this error was associated with
+
+=item file
+
+The file where the constructor of this error was called from
+
+=item line
+
+The line where the constructor of this error was called from
+
+=item text
+
+The text of the error
+
+=back
+
+=head2 OVERLOAD METHODS
+
+=over 4
+
+=item stringify
+
+A method that converts the object into a string. This method may simply
+return the same as the C<text> method, or it may append more
+information. For example the file name and line number.
+
+By default this method returns the C<-text> argument that was passed to
+the constructor, or the string C<"Died"> if none was given.
+
+=item value
+
+A method that will return a value that can be associated with the
+error. For example if an error was created due to the failure of a
+system call, then this may return the numeric value of C<$!> at the
+time.
+
+By default this method returns the C<-value> argument that was passed
+to the constructor.
+
+=back
+
+=head1 PRE-DEFINED ERROR CLASSES
+
+=over 4
+
+=item Error::Simple
+
+This class can be used to hold simple error strings and values. It's
+constructor takes two arguments. The first is a text value, the second
+is a numeric value. These values are what will be returned by the
+overload methods.
+
+If the text value ends with C<at file line 1> as $@ strings do, then
+this infomation will be used to set the C<-file> and C<-line> arguments
+of the error object.
+
+This class is used internally if an eval'd block die's with an error
+that is a plain string. (Unless C<$Error::ObjectifyCallback> is modified)
+
+=back
+
+=head1 $Error::ObjectifyCallback
+
+This variable holds a reference to a subroutine that converts errors that
+are plain strings to objects. It is used by Error.pm to convert textual
+errors to objects, and can be overrided by the user.
+
+It accepts a single argument which is a hash reference to named parameters. 
+Currently the only named parameter passed is C<'text'> which is the text
+of the error, but others may be available in the future.
+
+For example the following code will cause Error.pm to throw objects of the
+class MyError::Bar by default:
+
+    sub throw_MyError_Bar
+    {
+        my $args = shift;
+        my $err = MyError::Bar->new();
+        $err->{'MyBarText'} = $args->{'text'};
+        return $err;
+    }
+
+    {
+        local $Error::ObjectifyCallback = \&throw_MyError_Bar;
+
+        # Error handling here.
+    }
+
+=head1 KNOWN BUGS
+
+None, but that does not mean there are not any.
+
+=head1 AUTHORS
+
+Graham Barr <gbarr@pobox.com>
+
+The code that inspired me to write this was originally written by
+Peter Seibel <peter@weblogic.com> and adapted by Jesse Glick
+<jglick@sig.bsh.com>.
+
+=head1 MAINTAINER
+
+Shlomi Fish <shlomif@iglu.org.il>
+
+=head1 PAST MAINTAINERS
+
+Arun Kumar U <u_arunkumar@yahoo.com>
+
+=cut
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index dd61056..54e8b20 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -8,9 +8,19 @@ instlibdir:
 MAKE_FRAG
 }
 
+my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');
+
+# We come with our own bundled Error.pm. It's not in the set of default
+# Perl modules so install it if it's not available on the system yet.
+eval { require 'Error' };
+if ($@) {
+	$pm{'Error.pm'} = '$(INST_LIBDIR)/Error.pm';
+}
+
 WriteMakefile(
 	NAME            => 'Git',
 	VERSION_FROM    => 'Git.pm',
+	PM		=> \%pm,
 	MYEXTLIB        => '../libgit.a',
 	INC             => '-I. -I..',
 );

^ permalink raw reply related

* [PATCH 07/12] Git.pm: Better error handling
From: Petr Baudis @ 2006-06-24  2:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

So far, errors just killed the whole program and in case of an error
inside of libgit it would be totally uncatchable. This patch makes
Git.pm throw standard Perl exceptions instead. In the future we might
subclass Error to Git::Error or something but for now Error::Simple
is more than enough.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 perl/Git.pm |   37 +++++++++++++++++++++----------------
 perl/Git.xs |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index dcd769b..733fec9 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -88,7 +88,8 @@ increate nonwithstanding).
 =cut
 
 
-use Carp qw(carp croak);
+use Carp qw(carp); # croak is bad - throw instead
+use Error qw(:try);
 
 require XSLoader;
 XSLoader::load('Git', $VERSION);
@@ -143,14 +144,14 @@ sub repository {
 	if (defined $args[0]) {
 		if ($#args % 2 != 1) {
 			# Not a hash.
-			$#args == 0 or croak "bad usage";
-			%opts = (Directory => $args[0]);
+			$#args == 0 or throw Error::Simple("bad usage");
+			%opts = ( Directory => $args[0] );
 		} else {
 			%opts = @args;
 		}
 
 		if ($opts{Directory}) {
-			-d $opts{Directory} or croak "Directory not found: $!";
+			-d $opts{Directory} or throw Error::Simple("Directory not found: $!");
 			if (-d $opts{Directory}."/.git") {
 				# TODO: Might make this more clever
 				$opts{WorkingCopy} = $opts{Directory};
@@ -242,11 +243,11 @@ read.
 sub command_pipe {
 	my ($self, $cmd, @args) = _maybe_self(@_);
 
-	$cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+	$cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
 
 	my $pid = open(my $fh, "-|");
 	if (not defined $pid) {
-		croak "open failed: $!";
+		throw Error::Simple("open failed: $!");
 	} elsif ($pid == 0) {
 		_cmd_exec($self, $cmd, @args);
 	}
@@ -271,16 +272,17 @@ The function returns only after the comm
 sub command_noisy {
 	my ($self, $cmd, @args) = _maybe_self(@_);
 
-	$cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+	$cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
 
 	my $pid = fork;
 	if (not defined $pid) {
-		croak "fork failed: $!";
+		throw Error::Simple("fork failed: $!");
 	} elsif ($pid == 0) {
 		_cmd_exec($self, $cmd, @args);
 	}
 	if (waitpid($pid, 0) > 0 and $? != 0) {
-		croak "exit status: $?";
+		# This is the best candidate for a custom exception class.
+		throw Error::Simple("exit status: $?");
 	}
 }
 
@@ -340,10 +342,10 @@ # Implemented in Git.xs.
 
 =back
 
-=head1 TODO
+=head1 ERROR HANDLING
 
-This is still fairly crude.
-We need some good way to report errors back except just dying.
+All functions are supposed to throw Perl exceptions in case of errors.
+See L<Error>.
 
 =head1 COPYRIGHT
 
@@ -372,8 +374,8 @@ sub _cmd_exec {
 		$self->{opts}->{Repository} and $ENV{'GIT_DIR'} = $self->{opts}->{Repository};
 		$self->{opts}->{WorkingCopy} and chdir($self->{opts}->{WorkingCopy});
 	}
-	xs__execv_git_cmd(@args);
-	croak "exec failed: $!";
+	_execv_git_cmd(@args);
+	die "exec failed: $!";
 }
 
 # Execute the given Git command ($_[0]) with arguments ($_[1..])
@@ -388,7 +390,8 @@ sub _cmd_close {
 			# It's just close, no point in fatalities
 			carp "error closing pipe: $!";
 		} elsif ($? >> 8) {
-			croak "exit status: ".($? >> 8);
+			# This is the best candidate for a custom exception class.
+			throw Error::Simple("exit status: ".($? >> 8));
 		}
 		# else we might e.g. closed a live stream; the command
 		# dying of SIGPIPE would drive us here.
@@ -415,6 +418,8 @@ sub _call_gate {
 		#xs_call_gate($self->{opts}->{Repository});
 	}
 
+	# Having to call throw from the C code is a sure path to insanity.
+	local $SIG{__DIE__} = sub { throw Error::Simple("@_"); };
 	&$xsfunc(@args);
 }
 
@@ -422,7 +427,7 @@ sub AUTOLOAD {
 	my $xsname;
 	our $AUTOLOAD;
 	($xsname = $AUTOLOAD) =~ s/.*:://;
-	croak "&Git::$xsname not defined" if $xsname =~ /^xs_/;
+	throw Error::Simple("&Git::$xsname not defined") if $xsname =~ /^xs_/;
 	$xsname = 'xs_'.$xsname;
 	_call_gate(\&$xsname, @_);
 }
diff --git a/perl/Git.xs b/perl/Git.xs
index 9623fdd..ebaac4b 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -8,6 +8,8 @@ #include <ctype.h>
 #include "../cache.h"
 #include "../exec_cmd.h"
 
+#define die perlyshadow_die__
+
 /* XS and Perl interface */
 #include "EXTERN.h"
 #include "perl.h"
@@ -15,11 +17,48 @@ #include "XSUB.h"
 
 #include "ppport.h"
 
+#undef die
+
+
+static char *
+report_xs(const char *prefix, const char *err, va_list params)
+{
+	static char buf[4096];
+	strcpy(buf, prefix);
+	vsnprintf(buf + strlen(prefix), 4096 - strlen(prefix), err, params);
+	return buf;
+}
+
+void
+die_xs(const char *err, va_list params)
+{
+	char *str;
+	str = report_xs("fatal: ", err, params);
+	croak(str);
+}
+
+int
+error_xs(const char *err, va_list params)
+{
+	char *str;
+	str = report_xs("error: ", err, params);
+	warn(str);
+	return -1;
+}
+
 
 MODULE = Git		PACKAGE = Git		
 
 PROTOTYPES: DISABLE
 
+
+BOOT:
+{
+	set_error_routine(error_xs);
+	set_die_routine(die_xs);
+}
+
+
 # /* TODO: xs_call_gate(). See Git.pm. */
 
 

^ permalink raw reply related


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