threads / patch / 4625

patchIntroduce Git.pm (v3)

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

## tl;dr

20 messages between Jun 22, 2006 and Jun 26, 2006. Diffs are folded; open one to read it.

replies: 19people: 6as markdown or json

Petr Baudis· Jun 22, 2006, 22:02 UTC · lore

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(-)
Show changes to 6 files +504 −7

Makefile, git-fmt-merge-msg.perl, perl/.gitignore, perl/Git.pm, perl/Git.xs, perl/Makefile.PL

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();
Junio C Hamano· Jun 22, 2006, 23:18 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 11 quoted lines
> 	* 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

Petr Baudis· Jun 22, 2006, 23:50 UTC · re: Junio C Hamano · lore

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

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

Show 25 quoted lines
> 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(-)
Show changes to 6 files +504 −7

Makefile, git-fmt-merge-msg.perl, perl/.gitignore, perl/Git.pm, perl/Git.xs, perl/Makefile.PL

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.
Junio C Hamano· Jun 23, 2006, 00:22 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 13 quoted lines
>> 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.

Petr Baudis· Jun 23, 2006, 01:12 UTC · re: Junio C Hamano · lore

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

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

Show 24 quoted lines
> 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:
Show changes to perl/Git.xs +2 −0
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.
Junio C Hamano· Jun 23, 2006, 04:41 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 13 quoted lines
>>         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.

Jakub Narebski· Jun 23, 2006, 06:03 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 20 quoted lines
> 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
Nguyễn Thái Ngọc Duy· Jun 23, 2006, 06:34 UTC · re: Jakub Narebski · lore

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

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

Petr Baudis· Jun 23, 2006, 12:45 UTC · re: Jakub Narebski · lore

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

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.
Jakub Narebski· Jun 26, 2006, 23:20 UTC · re: Petr Baudis · lore

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

Dnia piątek 23. czerwca 2006 14:45, napisałeś:
Show 10 quoted lines
> 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
Junio C Hamano· Jun 23, 2006, 08:57 UTC · re: Petr Baudis · lore

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

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.

Eric W. Biederman· Jun 23, 2006, 12:04 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <junkio@cox.net> writes:
Show 15 quoted lines
> 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
Petr Baudis· Jun 23, 2006, 12:39 UTC · re: Eric W. Biederman · lore

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

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

Show 7 quoted lines
> 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.
Junio C Hamano· Jun 24, 2006, 00:39 UTC · re: Petr Baudis · lore

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

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.

Junio C Hamano· Jun 24, 2006, 01:01 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 6 quoted lines
>> 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(-)
Show changes to Makefile +13 −7
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
Junio C Hamano· Jun 24, 2006, 01:07 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <junkio@cox.net> writes:
Show 9 quoted lines
>> 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:
Show 5 quoted lines
>> 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?
Johannes Schindelin· Jun 24, 2006, 20:10 UTC · re: Junio C Hamano · lore

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

Hi,
On Fri, 23 Jun 2006, Junio C Hamano wrote:
Show 9 quoted lines
> 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(-)
Show changes to builtin-apply.c +23 −9
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) {
Junio C Hamano· Jun 25, 2006, 01:20 UTC · re: Johannes Schindelin · lore

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

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

Johannes Schindelin· Jun 25, 2006, 01:54 UTC · re: Junio C Hamano · lore

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

Hi,
On Sat, 24 Jun 2006, Junio C Hamano wrote:
Show 9 quoted lines
> 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

Junio C Hamano· Jun 24, 2006, 01:04 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 6 quoted lines
>> 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(-)
Show changes to t/test-lib.sh +2 −0
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

← back to recent threads