[PATCH] Introduce Git.pm (v3)

[PATCH] Introduce Git.pm (v3)

[Petr Baudis]

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.

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).

The changes since v1 are:

	* s/generic/command/ in the API, added command_pipe() and
	  changed behaviour of command() in the scalar context
	* Added hash_object() and Git.xs providing the fast implementation
	* Better error reporting
	* Plenty of random minor stuff

It's a good thing v2 didn't make it to the list since it contained a huge
file which turns out that can be autogenerated at the build time, so v3
just changes that (and adds .gitignore file and makes some other minor
changes).

I consider this patch "stable" now. Further enhancements will be posted
as patches on top of this.

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 |   10 +
 perl/.gitignore        |    7 +
 perl/Git.pm            |  401 ++++++++++++++++++++++++++++++++++++++++++++++++
 perl/Git.xs            |   60 +++++++
 perl/Makefile.PL       |   21 +++
 6 files changed, 504 insertions(+), 7 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..c32cafb 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,7 +49,7 @@ 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, $_;
 	}
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..4bb7c50
--- /dev/null
+++ b/perl/Git.pm
@@ -0,0 +1,401 @@
+=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);
+
+
+=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: $!";
+		} else {
+			croak "exit status: $?";
+		}
+	}
+}
+
+
+# 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..33bb3ca
--- /dev/null
+++ b/perl/Git.xs
@@ -0,0 +1,60 @@
+/* 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		
+
+# /* 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. */
+		Perl_io_close(sv_2io(file), 1);
+
+	} 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();

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> 	* Added hash_object() and Git.xs providing the fast implementation
> 	* Better error reporting
> 	* Plenty of random minor stuff
>
> It's a good thing v2 didn't make it to the list since it contained a huge
> file which turns out that can be autogenerated at the build time, so v3
> just changes that (and adds .gitignore file and makes some other minor
> changes).
>
> I consider this patch "stable" now. Further enhancements will be posted
> as patches on top of this.

Eek.  It does not compile for me -- maybe there is more
dependencies that need to be listed in INSTALL file?

make -C perl
make[1]: Entering directory `/opt/git/git.git/perl'
cc -c  -I. -I.. -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2   -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC "-I/usr/lib/perl/5.8/CORE"  -O2 -Wall -Wdeclaration-after-statement -g -DSHA1_HEADER='<openssl/sha.h>'  Git.c
Git.xs: In function 'XS_Git_xs_hash_object':
Git.xs:27: warning: ISO C90 forbids mixed declarations and code
Git.xs:44: warning: passing argument 1 of 'Perl_io_close' from incompatible pointer type
Git.xs:44: warning: passing argument 2 of 'Perl_io_close' makes pointer from integer without a cast
Git.xs:44: error: too few arguments to function 'Perl_io_close'
make[1]: *** [Git.o] Error 1
make[1]: Leaving directory `/opt/git/git.git/perl'
make: *** [all] Error 2

Re: [PATCH] Introduce Git.pm (v3)

[Petr Baudis]

Dear diary, on Fri, Jun 23, 2006 at 01:18:10AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > 	* Added hash_object() and Git.xs providing the fast implementation
> > 	* Better error reporting
> > 	* Plenty of random minor stuff
> >
> > It's a good thing v2 didn't make it to the list since it contained a huge
> > file which turns out that can be autogenerated at the build time, so v3
> > just changes that (and adds .gitignore file and makes some other minor
> > changes).
> >
> > I consider this patch "stable" now. Further enhancements will be posted
> > as patches on top of this.
> 
> Eek.  It does not compile for me -- maybe there is more
> dependencies that need to be listed in INSTALL file?
> 
> make -C perl
> make[1]: Entering directory `/opt/git/git.git/perl'
> cc -c  -I. -I.. -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2   -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC "-I/usr/lib/perl/5.8/CORE"  -O2 -Wall -Wdeclaration-after-statement -g -DSHA1_HEADER='<openssl/sha.h>'  Git.c
> Git.xs: In function 'XS_Git_xs_hash_object':
> Git.xs:27: warning: ISO C90 forbids mixed declarations and code
> Git.xs:44: warning: passing argument 1 of 'Perl_io_close' from incompatible pointer type
> Git.xs:44: warning: passing argument 2 of 'Perl_io_close' makes pointer from integer without a cast
> Git.xs:44: error: too few arguments to function 'Perl_io_close'

Oops, sorry. Apparently, I've digged too deep and unleashed a monster of
unstable API. Now I've finally discovered perlapio(1) and know I
should've just called PerlIO_close(). Below comes a fixed patch, I
didn't bother to bump the version number.

---

[PATCH] Introduce Git.pm (v3)

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.

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).

The changes since v1 are:

	* s/generic/command/ in the API, added command_pipe() and
	  changed behaviour of command() in the scalar context
	* Added hash_object() and Git.xs providing the fast implementation
	* Better error reporting
	* Plenty of random minor stuff

It's a good thing v2 didn't make it to the list since it contained a huge
file which turns out that can be autogenerated at the build time, so v3
just changes that (and adds .gitignore file and makes some other minor
changes).

I consider this patch "stable" now. Further enhancements will be posted
as patches on top of this.

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 |   10 +
 perl/.gitignore        |    7 +
 perl/Git.pm            |  401 ++++++++++++++++++++++++++++++++++++++++++++++++
 perl/Git.xs            |   60 +++++++
 perl/Makefile.PL       |   21 +++
 6 files changed, 504 insertions(+), 7 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..c32cafb 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,7 +49,7 @@ 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, $_;
 	}
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..4bb7c50
--- /dev/null
+++ b/perl/Git.pm
@@ -0,0 +1,401 @@
+=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);
+
+
+=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: $!";
+		} else {
+			croak "exit status: $?";
+		}
+	}
+}
+
+
+# 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..3799ee9
--- /dev/null
+++ b/perl/Git.xs
@@ -0,0 +1,60 @@
+/* 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		
+
+# /* 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();

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

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> Eek.  It does not compile for me -- maybe there is more
>> dependencies that need to be listed in INSTALL file?
>> ...
>> Git.xs: In function 'XS_Git_xs_hash_object':
>> Git.xs:27: warning: ISO C90 forbids mixed declarations and code
>> Git.xs:44: warning: passing argument 1 of 'Perl_io_close' from incompatible pointer type
>> Git.xs:44: warning: passing argument 2 of 'Perl_io_close' makes pointer from integer without a cast
>> Git.xs:44: error: too few arguments to function 'Perl_io_close'
>
> Oops, sorry. Apparently, I've digged too deep and unleashed a monster of
> unstable API. Now I've finally discovered perlapio(1) and know I
> should've just called PerlIO_close(). Below comes a fixed patch, I
> didn't bother to bump the version number.

Thanks; it compiles now with a few more glitches.

            /usr/bin/perl /usr/share/perl/5.8/ExtUtils/xsubpp  \
                    -typemap /usr/share/perl/5.8/ExtUtils/typemap  Git.xs > Git.xsc && \
                    mv Git.xsc Git.c
            Please specify prototyping behavior for Git.xs (see perlxs manual)

Says xsubpp.

        cc -c  -I. -I.. -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS \
                -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include \
                -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 \
                -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC \
                "-I/usr/lib/perl/5.8/CORE" -Wall
                -Wdeclaration-after-statement \
                -g -DSHA1_HEADER='<openssl/sha.h>' \
                -DGIT_VERSION=\"1.4.1.rc1.g01a1\" Git.c
        Git.xs: In function 'XS_Git_xs__execv_git_cmd':
        Git.xs:57: warning: passing argument 1 of 'free' discards qualifiers from pointer target type
        Git.xs: In function 'XS_Git_xs_hash_object':
        Git.xs:65: warning: ISO C90 forbids mixed declarations and code

I usually compile with -Wdeclaration-after-statement so I
probably get some more warnings than you saw; it probably is
primarily xsubpp's fault, but you could work it around by having
CODE block to be inside an extra set of braces {}.  Constness
reduction of free() is a bit annoying from the point of view of
the coder who has to cast away constness, so I won't be too
strict about that, but it would be nicer if we did not have to
see the warnings.

        rm -f blib/arch/auto/Git/Git.so
        cc  -shared -L/usr/local/lib Git.o  -o blib/arch/auto/Git/Git.so ../libgit.a	\
                   -lz -lcrypto  	\

        /usr/bin/ld: ../libgit.a(exec_cmd.o): relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
        ../libgit.a: could not read symbols: Bad value
        collect2: ld returned 1 exit status
        make[1]: *** [blib/arch/auto/Git/Git.so] Error 1

This is a real killer.  If we compile everything with -fPIC,
this goes away, but I do not think we want -fPIC for the core
level tools.  At least not until we are ready to do libgit.so.

Re: [PATCH] Introduce Git.pm (v3)

[Petr Baudis]

Dear diary, on Fri, Jun 23, 2006 at 02:22:41AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> Eek.  It does not compile for me -- maybe there is more
> >> dependencies that need to be listed in INSTALL file?
> >> ...
> >> Git.xs: In function 'XS_Git_xs_hash_object':
> >> Git.xs:27: warning: ISO C90 forbids mixed declarations and code
> >> Git.xs:44: warning: passing argument 1 of 'Perl_io_close' from incompatible pointer type
> >> Git.xs:44: warning: passing argument 2 of 'Perl_io_close' makes pointer from integer without a cast
> >> Git.xs:44: error: too few arguments to function 'Perl_io_close'
> >
> > Oops, sorry. Apparently, I've digged too deep and unleashed a monster of
> > unstable API. Now I've finally discovered perlapio(1) and know I
> > should've just called PerlIO_close(). Below comes a fixed patch, I
> > didn't bother to bump the version number.
> 
> Thanks; it compiles now with a few more glitches.
> 
>             /usr/bin/perl /usr/share/perl/5.8/ExtUtils/xsubpp  \
>                     -typemap /usr/share/perl/5.8/ExtUtils/typemap  Git.xs > Git.xsc && \
>                     mv Git.xsc Git.c
>             Please specify prototyping behavior for Git.xs (see perlxs manual)
> 
> Says xsubpp.

It's harmless but can be fixed by:

diff --git a/perl/Git.xs b/perl/Git.xs
index 3799ee9..8ab84bb 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -17,6 +17,8 @@ #include "ppport.h"
 
 MODULE = Git		PACKAGE = Git		
 
+PROTOTYPES: DISABLE
+
 # /* TODO: xs_call_gate(). See Git.pm. */
 
 char *

>         cc -c  -I. -I.. -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS \
>                 -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include \
>                 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 \
>                 -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC \
>                 "-I/usr/lib/perl/5.8/CORE" -Wall
>                 -Wdeclaration-after-statement \
>                 -g -DSHA1_HEADER='<openssl/sha.h>' \
>                 -DGIT_VERSION=\"1.4.1.rc1.g01a1\" Git.c
>         Git.xs: In function 'XS_Git_xs__execv_git_cmd':
>         Git.xs:57: warning: passing argument 1 of 'free' discards qualifiers from pointer target type
>         Git.xs: In function 'XS_Git_xs_hash_object':
>         Git.xs:65: warning: ISO C90 forbids mixed declarations and code
> 
> I usually compile with -Wdeclaration-after-statement so I
> probably get some more warnings than you saw; it probably is
> primarily xsubpp's fault, but you could work it around by having
> CODE block to be inside an extra set of braces {}.

Sure. Should I resend all the .xs-related patches with the code blocks
in {}, or will you add it at your side?

> Constness reduction of free() is a bit annoying from the point of view of
> the coder who has to cast away constness, so I won't be too
> strict about that, but it would be nicer if we did not have to
> see the warnings.

Sure, it should be trivial to fix.

>         rm -f blib/arch/auto/Git/Git.so
>         cc  -shared -L/usr/local/lib Git.o  -o blib/arch/auto/Git/Git.so ../libgit.a	\
>                    -lz -lcrypto  	\
> 
>         /usr/bin/ld: ../libgit.a(exec_cmd.o): relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
>         ../libgit.a: could not read symbols: Bad value
>         collect2: ld returned 1 exit status
>         make[1]: *** [blib/arch/auto/Git/Git.so] Error 1
> 
> This is a real killer.  If we compile everything with -fPIC,
> this goes away, but I do not think we want -fPIC for the core
> level tools.  At least not until we are ready to do libgit.so.

Hmm, I didn't get that; I guess that's x86-64 specific. :/ Do you have
any idea what the error actually means? Could the environ be a problem?
(Why aren't we calling just execv()?)

Also, is there any real problem with just using -fPIC?

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

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>>         cc  -shared -L/usr/local/lib Git.o  -o blib/arch/auto/Git/Git.so ../libgit.a	\
>>                    -lz -lcrypto  	\
>> 
>>         /usr/bin/ld: ../libgit.a(exec_cmd.o): relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
>>         ../libgit.a: could not read symbols: Bad value
>>         collect2: ld returned 1 exit status
>>         make[1]: *** [blib/arch/auto/Git/Git.so] Error 1
>> 
>> This is a real killer.  If we compile everything with -fPIC,
>> this goes away, but I do not think we want -fPIC for the core
>> level tools.  At least not until we are ready to do libgit.so.
>
> Hmm, I didn't get that; I guess that's x86-64 specific. :/

So it seems.  Both RH machines I have access at kernel.org and
Debian machines I have locally exhibit that x86-32 is OK and
x86-64 is bad.  They all run libc-2.3.6 except RH x86-32 is at
libc-2.3.4.

Re: [PATCH] Introduce Git.pm (v3)

[Jakub Narebski]

Junio C Hamano wrote:

> Petr Baudis <pasky@suse.cz> writes:
> 
>>>         cc  -shared -L/usr/local/lib Git.o  -o blib/arch/auto/Git/Git.so ../libgit.a        \
>>>                    -lz -lcrypto     \
>>> 
>>>         /usr/bin/ld: ../libgit.a(exec_cmd.o): relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
>>>         ../libgit.a: could not read symbols: Bad value
>>>         collect2: ld returned 1 exit status
>>>         make[1]: *** [blib/arch/auto/Git/Git.so] Error 1
>>> 
>>> This is a real killer.  If we compile everything with -fPIC,
>>> this goes away, but I do not think we want -fPIC for the core
>>> level tools.  At least not until we are ready to do libgit.so.
>>
>> Hmm, I didn't get that; I guess that's x86-64 specific. :/
> 
> So it seems.  Both RH machines I have access at kernel.org and
> Debian machines I have locally exhibit that x86-32 is OK and
> x86-64 is bad.  They all run libc-2.3.6 except RH x86-32 is at
> libc-2.3.4.

Perhaps Git.pm should provide also generic, pure Perl (and slower)
fallback implementation (when for some reason we cannot compile XS).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Introduce Git.pm (v3)

[Nguyễn Thái Ngọc Duy]

On 6/23/06, Jakub Narebski <jnareb@gmail.com> wrote:
> Perhaps Git.pm should provide also generic, pure Perl (and slower)
> fallback implementation (when for some reason we cannot compile XS).
Haven't looked at Git.pm. But I have written receive-pack in perl. It
may have some useful functions for Git.pm

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Also, is there any real problem with just using -fPIC?

Personally, not really, but I consider it a workaround having to
compile with -fPIC (being able to compile with -fPIC is a
feature).

Doesn't it have performance implications to use -fPIC when you
do not have to?

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.

Re: [PATCH] Introduce Git.pm (v3)

[Eric W. Biederman]

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

> Petr Baudis <pasky@suse.cz> writes:
>
>> Also, is there any real problem with just using -fPIC?
>
> Personally, not really, but I consider it a workaround having to
> compile with -fPIC (being able to compile with -fPIC is a
> feature).
>
> Doesn't it have performance implications to use -fPIC when you
> do not have to?
>
> 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.

So what was being compiled was a shared Git.so.

32bit x86 is on of the few architectures that allows you to build
a .so without compiling with -fPIC.  

The question is why are we building with a .so?

Eric

Re: [PATCH] Introduce Git.pm (v3)

[Petr Baudis]

Dear diary, on Fri, Jun 23, 2006 at 10:57:50AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Also, is there any real problem with just using -fPIC?
> 
> Personally, not really, but I consider it a workaround having to
> compile with -fPIC (being able to compile with -fPIC is a
> feature).

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? ;-)

> Doesn't it have performance implications to use -fPIC when you
> do not have to?

No idea here.

> 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.


Dear diary, on Fri, Jun 23, 2006 at 02:04:17PM CEST, I got a letter
where "Eric W. Biederman" <ebiederm@xmission.com> said that...
> The question is why are we building with a .so?

To make use of it in Git.pm - it can call libgit routines directly from
inside of Perl, but for that it needs to dynamically link libgit to the
Perl process on the fly (using dlopen()).

We _can_ avoid the .so, but that involved producing a new perl
executable with libgit statically linked to it, which is quite
impractical, so to say.

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

Re: [PATCH] Introduce Git.pm (v3)

[Petr Baudis]

Dear diary, on Fri, Jun 23, 2006 at 08:03:23AM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> Perhaps Git.pm should provide also generic, pure Perl (and slower)
> fallback implementation (when for some reason we cannot compile XS).

I fiercely want to avoid this if there is any other possible way to go
about it - this is a path to hell of massive code duplication and
additional work, as the number of routines will grow. If it is question
of spending many developer-hours uselessly duplicating code in a way
that'll be much slower than possible anyway OR building with -fPIC... ;-)

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

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

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. 

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

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

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

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

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

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?

Re: [PATCH] Introduce Git.pm (v3)

[Johannes Schindelin]

Hi,

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

> 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?

My original idea: on a machine where you have no accurate diff, you at 
least want to pass the tests, and you want to ensure you can apply a diff 
you generated on that machine.

But I was wrong. This patch is just compile tested, but obviously correct 
(lacking usage() and Documentation updates as usual, since I do not want 
to do that work before I know the patch is applied):

---
[PATCH] apply: add --no-accurate-diff and --accurate-diff options

You can still set the default behaviour in the Makefile, but at least you
can override it.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 builtin-apply.c |   32 +++++++++++++++++++++++---------
 1 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index e113c74..2d26ade 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -125,6 +125,7 @@ #define BINARY_LITERAL_DEFLATED 2
 	unsigned long deflate_origlen;
 	int lines_added, lines_deleted;
 	int score;
+	int no_accurate_diff:1;
 	struct fragment *fragments;
 	char *result;
 	unsigned long resultsize;
@@ -1333,7 +1334,8 @@ static int apply_line(char *output, cons
 	return plen;
 }
 
-static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag)
+static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag,
+	int no_accurate_diff)
 {
 	int match_beginning, match_end;
 	char *buf = desc->buffer;
@@ -1386,13 +1388,11 @@ static int apply_one_fragment(struct buf
 		size -= len;
 	}
 
-#ifdef NO_ACCURATE_DIFF
-	if (oldsize > 0 && old[oldsize - 1] == '\n' &&
+	if (no_accurate_diff && oldsize > 0 && old[oldsize - 1] == '\n' &&
 			newsize > 0 && new[newsize - 1] == '\n') {
 		oldsize--;
 		newsize--;
 	}
-#endif
 
 	oldlines = old;
 	newlines = new;
@@ -1614,7 +1614,7 @@ static int apply_fragments(struct buffer
 		return apply_binary(desc, patch);
 
 	while (frag) {
-		if (apply_one_fragment(desc, frag) < 0)
+		if (apply_one_fragment(desc, frag, patch->no_accurate_diff) < 0)
 			return error("patch failed: %s:%ld",
 				     name, frag->oldpos);
 		frag = frag->next;
@@ -2097,7 +2097,7 @@ static int use_patch(struct patch *p)
 	return 1;
 }
 
-static int apply_patch(int fd, const char *filename)
+static int apply_patch(int fd, const char *filename, int no_accurate_diff)
 {
 	unsigned long offset, size;
 	char *buffer = read_patch_file(fd, &size);
@@ -2113,6 +2113,7 @@ static int apply_patch(int fd, const cha
 		int nr;
 
 		patch = xcalloc(1, sizeof(*patch));
+		patch->no_accurate_diff = no_accurate_diff;
 		nr = parse_chunk(buffer + offset, size, patch);
 		if (nr < 0)
 			break;
@@ -2180,6 +2181,11 @@ int cmd_apply(int argc, const char **arg
 {
 	int i;
 	int read_stdin = 1;
+#ifdef NO_ACCURATE_DIFF
+	int no_accurate_diff = 1;
+#else
+	int no_accurate_diff = 0;
+#endif
 	const char *whitespace_option = NULL;
 
 	for (i = 1; i < argc; i++) {
@@ -2188,7 +2194,7 @@ int cmd_apply(int argc, const char **arg
 		int fd;
 
 		if (!strcmp(arg, "-")) {
-			apply_patch(0, "<stdin>");
+			apply_patch(0, "<stdin>", no_accurate_diff);
 			read_stdin = 0;
 			continue;
 		}
@@ -2265,6 +2271,14 @@ int cmd_apply(int argc, const char **arg
 			parse_whitespace_option(arg + 13);
 			continue;
 		}
+		if (!strcmp(arg, "--no-accurate-diff")) {
+			no_accurate_diff = 1;
+			continue;
+		}
+		if (!strcmp(arg, "--accurate-diff")) {
+			no_accurate_diff = 0;
+			continue;
+		}
 
 		if (check_index && prefix_length < 0) {
 			prefix = setup_git_directory();
@@ -2281,12 +2295,12 @@ int cmd_apply(int argc, const char **arg
 			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		apply_patch(fd, arg);
+		apply_patch(fd, arg, no_accurate_diff);
 		close(fd);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		apply_patch(0, "<stdin>");
+		apply_patch(0, "<stdin>", no_accurate_diff);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {

Re: [PATCH] Introduce Git.pm (v3)

[Junio C Hamano]

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

> My original idea: on a machine where you have no accurate diff, you at 
> least want to pass the tests, and you want to ensure you can apply a diff 
> you generated on that machine.

I remember that, but I think recently we converted t4100 and
t4101 to use pregenerated test vectors so it might not be an
issue anymore?

> This patch is just compile tested, but obviously correct 

Looks sane, thanks.

I would maybe rename the option to --inaccurate-eof and default
it to off (i.e. no --accurate-eof option).  After all we are not
talking about arbitrary inaccuracy but the particular botch of
not having "\No newline at the end of file."

Re: [PATCH] Introduce Git.pm (v3)

[Johannes Schindelin]

Hi,

On Sat, 24 Jun 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > My original idea: on a machine where you have no accurate diff, you at 
> > least want to pass the tests, and you want to ensure you can apply a diff 
> > you generated on that machine.
> 
> I remember that, but I think recently we converted t4100 and
> t4101 to use pregenerated test vectors so it might not be an
> issue anymore?

Exactly.

> I would maybe rename the option to --inaccurate-eof and default
> it to off (i.e. no --accurate-eof option).  After all we are not
> talking about arbitrary inaccuracy but the particular botch of
> not having "\No newline at the end of file."

Sure. Want me to redo the patch?

Ciao,
Dscho

Re: [PATCH] Introduce Git.pm (v3)

[Jakub Narebski]

Dnia piątek 23. czerwca 2006 14:45, napisałeś:
> Dear diary, on Fri, Jun 23, 2006 at 08:03:23AM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...
>> Perhaps Git.pm should provide also generic, pure Perl (and slower)
>> fallback implementation (when for some reason we cannot compile XS).
> 
> I fiercely want to avoid this if there is any other possible way to go
> about it - this is a path to hell of massive code duplication and
> additional work, as the number of routines will grow. If it is question
> of spending many developer-hours uselessly duplicating code in a way
> that'll be much slower than possible anyway OR building with -fPIC... ;-)

I have thought about reimplementing only the parts which are in XS. In C
one would probably write

#if can_asm
 asm (...)
#else
 /* C code equivalent */
#endif

In addition to allowing use of Git.pm for people who cannot compile 
Git.xs, it would be helpful I guess in Git.pm development.

-- 
Jakub Narebski
Poland