* [PATCH 06/12] Add Error.pm to the distribution
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
I have been thinking about how to do the error reporting the best
way and after scraping various overcomplicated concepts, I have
decided that by far the most elegant way is to throw Error exceptions;
the closest sane alternative is to catch the dies in Git.pm by
enclosing the calls in eval{}s and that's really _quite_ ugly.
The only "small" trouble is that Error.pm turns out sadly not to be
part of the standard distribution, and installation from CPAN is
a bother, especially if you can't install it system-wide. But since
it is very small, I've decided to just bundle it.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
perl/Error.pm | 821 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
perl/Makefile.PL | 10 +
2 files changed, 831 insertions(+), 0 deletions(-)
diff --git a/perl/Error.pm b/perl/Error.pm
new file mode 100644
index 0000000..17ad70a
--- /dev/null
+++ b/perl/Error.pm
@@ -0,0 +1,821 @@
+# Error.pm
+#
+# Copyright (c) 1997-8 Graham Barr <gbarr@ti.com>. All rights reserved.
+# This program is free software; you can redistribute it and/or
+# modify it under the same terms as Perl itself.
+#
+# Based on my original Error.pm, and Exceptions.pm by Peter Seibel
+# <peter@weblogic.com> and adapted by Jesse Glick <jglick@sig.bsh.com>.
+#
+# but modified ***significantly***
+
+package Error;
+
+use strict;
+use vars qw($VERSION);
+use 5.004;
+
+$VERSION = "0.15009";
+
+use overload (
+ '""' => 'stringify',
+ '0+' => 'value',
+ 'bool' => sub { return 1; },
+ 'fallback' => 1
+);
+
+$Error::Depth = 0; # Depth to pass to caller()
+$Error::Debug = 0; # Generate verbose stack traces
+@Error::STACK = (); # Clause stack for try
+$Error::THROWN = undef; # last error thrown, a workaround until die $ref works
+
+my $LAST; # Last error created
+my %ERROR; # Last error associated with package
+
+sub throw_Error_Simple
+{
+ my $args = shift;
+ return Error::Simple->new($args->{'text'});
+}
+
+$Error::ObjectifyCallback = \&throw_Error_Simple;
+
+
+# Exported subs are defined in Error::subs
+
+use Scalar::Util ();
+
+sub import {
+ shift;
+ local $Exporter::ExportLevel = $Exporter::ExportLevel + 1;
+ Error::subs->import(@_);
+}
+
+# I really want to use last for the name of this method, but it is a keyword
+# which prevent the syntax last Error
+
+sub prior {
+ shift; # ignore
+
+ return $LAST unless @_;
+
+ my $pkg = shift;
+ return exists $ERROR{$pkg} ? $ERROR{$pkg} : undef
+ unless ref($pkg);
+
+ my $obj = $pkg;
+ my $err = undef;
+ if($obj->isa('HASH')) {
+ $err = $obj->{'__Error__'}
+ if exists $obj->{'__Error__'};
+ }
+ elsif($obj->isa('GLOB')) {
+ $err = ${*$obj}{'__Error__'}
+ if exists ${*$obj}{'__Error__'};
+ }
+
+ $err;
+}
+
+sub flush {
+ shift; #ignore
+
+ unless (@_) {
+ $LAST = undef;
+ return;
+ }
+
+ my $pkg = shift;
+ return unless ref($pkg);
+
+ undef $ERROR{$pkg} if defined $ERROR{$pkg};
+}
+
+# Return as much information as possible about where the error
+# happened. The -stacktrace element only exists if $Error::DEBUG
+# was set when the error was created
+
+sub stacktrace {
+ my $self = shift;
+
+ return $self->{'-stacktrace'}
+ if exists $self->{'-stacktrace'};
+
+ my $text = exists $self->{'-text'} ? $self->{'-text'} : "Died";
+
+ $text .= sprintf(" at %s line %d.\n", $self->file, $self->line)
+ unless($text =~ /\n$/s);
+
+ $text;
+}
+
+# Allow error propagation, ie
+#
+# $ber->encode(...) or
+# return Error->prior($ber)->associate($ldap);
+
+sub associate {
+ my $err = shift;
+ my $obj = shift;
+
+ return unless ref($obj);
+
+ if($obj->isa('HASH')) {
+ $obj->{'__Error__'} = $err;
+ }
+ elsif($obj->isa('GLOB')) {
+ ${*$obj}{'__Error__'} = $err;
+ }
+ $obj = ref($obj);
+ $ERROR{ ref($obj) } = $err;
+
+ return;
+}
+
+sub new {
+ my $self = shift;
+ my($pkg,$file,$line) = caller($Error::Depth);
+
+ my $err = bless {
+ '-package' => $pkg,
+ '-file' => $file,
+ '-line' => $line,
+ @_
+ }, $self;
+
+ $err->associate($err->{'-object'})
+ if(exists $err->{'-object'});
+
+ # To always create a stacktrace would be very inefficient, so
+ # we only do it if $Error::Debug is set
+
+ if($Error::Debug) {
+ require Carp;
+ local $Carp::CarpLevel = $Error::Depth;
+ my $text = defined($err->{'-text'}) ? $err->{'-text'} : "Error";
+ my $trace = Carp::longmess($text);
+ # Remove try calls from the trace
+ $trace =~ s/(\n\s+\S+__ANON__[^\n]+)?\n\s+eval[^\n]+\n\s+Error::subs::try[^\n]+(?=\n)//sog;
+ $trace =~ s/(\n\s+\S+__ANON__[^\n]+)?\n\s+eval[^\n]+\n\s+Error::subs::run_clauses[^\n]+\n\s+Error::subs::try[^\n]+(?=\n)//sog;
+ $err->{'-stacktrace'} = $trace
+ }
+
+ $@ = $LAST = $ERROR{$pkg} = $err;
+}
+
+# Throw an error. this contains some very gory code.
+
+sub throw {
+ my $self = shift;
+ local $Error::Depth = $Error::Depth + 1;
+
+ # if we are not rethrow-ing then create the object to throw
+ $self = $self->new(@_) unless ref($self);
+
+ die $Error::THROWN = $self;
+}
+
+# syntactic sugar for
+#
+# die with Error( ... );
+
+sub with {
+ my $self = shift;
+ local $Error::Depth = $Error::Depth + 1;
+
+ $self->new(@_);
+}
+
+# syntactic sugar for
+#
+# record Error( ... ) and return;
+
+sub record {
+ my $self = shift;
+ local $Error::Depth = $Error::Depth + 1;
+
+ $self->new(@_);
+}
+
+# catch clause for
+#
+# try { ... } catch CLASS with { ... }
+
+sub catch {
+ my $pkg = shift;
+ my $code = shift;
+ my $clauses = shift || {};
+ my $catch = $clauses->{'catch'} ||= [];
+
+ unshift @$catch, $pkg, $code;
+
+ $clauses;
+}
+
+# Object query methods
+
+sub object {
+ my $self = shift;
+ exists $self->{'-object'} ? $self->{'-object'} : undef;
+}
+
+sub file {
+ my $self = shift;
+ exists $self->{'-file'} ? $self->{'-file'} : undef;
+}
+
+sub line {
+ my $self = shift;
+ exists $self->{'-line'} ? $self->{'-line'} : undef;
+}
+
+sub text {
+ my $self = shift;
+ exists $self->{'-text'} ? $self->{'-text'} : undef;
+}
+
+# overload methods
+
+sub stringify {
+ my $self = shift;
+ defined $self->{'-text'} ? $self->{'-text'} : "Died";
+}
+
+sub value {
+ my $self = shift;
+ exists $self->{'-value'} ? $self->{'-value'} : undef;
+}
+
+package Error::Simple;
+
+@Error::Simple::ISA = qw(Error);
+
+sub new {
+ my $self = shift;
+ my $text = "" . shift;
+ my $value = shift;
+ my(@args) = ();
+
+ local $Error::Depth = $Error::Depth + 1;
+
+ @args = ( -file => $1, -line => $2)
+ if($text =~ s/\s+at\s+(\S+)\s+line\s+(\d+)(?:,\s*<[^>]*>\s+line\s+\d+)?\.?\n?$//s);
+ push(@args, '-value', 0 + $value)
+ if defined($value);
+
+ $self->SUPER::new(-text => $text, @args);
+}
+
+sub stringify {
+ my $self = shift;
+ my $text = $self->SUPER::stringify;
+ $text .= sprintf(" at %s line %d.\n", $self->file, $self->line)
+ unless($text =~ /\n$/s);
+ $text;
+}
+
+##########################################################################
+##########################################################################
+
+# Inspired by code from Jesse Glick <jglick@sig.bsh.com> and
+# Peter Seibel <peter@weblogic.com>
+
+package Error::subs;
+
+use Exporter ();
+use vars qw(@EXPORT_OK @ISA %EXPORT_TAGS);
+
+@EXPORT_OK = qw(try with finally except otherwise);
+%EXPORT_TAGS = (try => \@EXPORT_OK);
+
+@ISA = qw(Exporter);
+
+sub run_clauses ($$$\@) {
+ my($clauses,$err,$wantarray,$result) = @_;
+ my $code = undef;
+
+ $err = $Error::ObjectifyCallback->({'text' =>$err}) unless ref($err);
+
+ CATCH: {
+
+ # catch
+ my $catch;
+ if(defined($catch = $clauses->{'catch'})) {
+ my $i = 0;
+
+ CATCHLOOP:
+ for( ; $i < @$catch ; $i += 2) {
+ my $pkg = $catch->[$i];
+ unless(defined $pkg) {
+ #except
+ splice(@$catch,$i,2,$catch->[$i+1]->());
+ $i -= 2;
+ next CATCHLOOP;
+ }
+ elsif(Scalar::Util::blessed($err) && $err->isa($pkg)) {
+ $code = $catch->[$i+1];
+ while(1) {
+ my $more = 0;
+ local($Error::THROWN);
+ my $ok = eval {
+ if($wantarray) {
+ @{$result} = $code->($err,\$more);
+ }
+ elsif(defined($wantarray)) {
+ @{$result} = ();
+ $result->[0] = $code->($err,\$more);
+ }
+ else {
+ $code->($err,\$more);
+ }
+ 1;
+ };
+ if( $ok ) {
+ next CATCHLOOP if $more;
+ undef $err;
+ }
+ else {
+ $err = defined($Error::THROWN)
+ ? $Error::THROWN : $@;
+ $err = $Error::ObjectifyCallback->({'text' =>$err})
+ unless ref($err);
+ }
+ last CATCH;
+ };
+ }
+ }
+ }
+
+ # otherwise
+ my $owise;
+ if(defined($owise = $clauses->{'otherwise'})) {
+ my $code = $clauses->{'otherwise'};
+ my $more = 0;
+ my $ok = eval {
+ if($wantarray) {
+ @{$result} = $code->($err,\$more);
+ }
+ elsif(defined($wantarray)) {
+ @{$result} = ();
+ $result->[0] = $code->($err,\$more);
+ }
+ else {
+ $code->($err,\$more);
+ }
+ 1;
+ };
+ if( $ok ) {
+ undef $err;
+ }
+ else {
+ $err = defined($Error::THROWN)
+ ? $Error::THROWN : $@;
+
+ $err = $Error::ObjectifyCallback->({'text' =>$err})
+ unless ref($err);
+ }
+ }
+ }
+ $err;
+}
+
+sub try (&;$) {
+ my $try = shift;
+ my $clauses = @_ ? shift : {};
+ my $ok = 0;
+ my $err = undef;
+ my @result = ();
+
+ unshift @Error::STACK, $clauses;
+
+ my $wantarray = wantarray();
+
+ do {
+ local $Error::THROWN = undef;
+ local $@ = undef;
+
+ $ok = eval {
+ if($wantarray) {
+ @result = $try->();
+ }
+ elsif(defined $wantarray) {
+ $result[0] = $try->();
+ }
+ else {
+ $try->();
+ }
+ 1;
+ };
+
+ $err = defined($Error::THROWN) ? $Error::THROWN : $@
+ unless $ok;
+ };
+
+ shift @Error::STACK;
+
+ $err = run_clauses($clauses,$err,wantarray,@result)
+ unless($ok);
+
+ $clauses->{'finally'}->()
+ if(defined($clauses->{'finally'}));
+
+ if (defined($err))
+ {
+ if (Scalar::Util::blessed($err) && $err->can('throw'))
+ {
+ throw $err;
+ }
+ else
+ {
+ die $err;
+ }
+ }
+
+ wantarray ? @result : $result[0];
+}
+
+# Each clause adds a sub to the list of clauses. The finally clause is
+# always the last, and the otherwise clause is always added just before
+# the finally clause.
+#
+# All clauses, except the finally clause, add a sub which takes one argument
+# this argument will be the error being thrown. The sub will return a code ref
+# if that clause can handle that error, otherwise undef is returned.
+#
+# The otherwise clause adds a sub which unconditionally returns the users
+# code reference, this is why it is forced to be last.
+#
+# The catch clause is defined in Error.pm, as the syntax causes it to
+# be called as a method
+
+sub with (&;$) {
+ @_
+}
+
+sub finally (&) {
+ my $code = shift;
+ my $clauses = { 'finally' => $code };
+ $clauses;
+}
+
+# The except clause is a block which returns a hashref or a list of
+# key-value pairs, where the keys are the classes and the values are subs.
+
+sub except (&;$) {
+ my $code = shift;
+ my $clauses = shift || {};
+ my $catch = $clauses->{'catch'} ||= [];
+
+ my $sub = sub {
+ my $ref;
+ my(@array) = $code->($_[0]);
+ if(@array == 1 && ref($array[0])) {
+ $ref = $array[0];
+ $ref = [ %$ref ]
+ if(UNIVERSAL::isa($ref,'HASH'));
+ }
+ else {
+ $ref = \@array;
+ }
+ @$ref
+ };
+
+ unshift @{$catch}, undef, $sub;
+
+ $clauses;
+}
+
+sub otherwise (&;$) {
+ my $code = shift;
+ my $clauses = shift || {};
+
+ if(exists $clauses->{'otherwise'}) {
+ require Carp;
+ Carp::croak("Multiple otherwise clauses");
+ }
+
+ $clauses->{'otherwise'} = $code;
+
+ $clauses;
+}
+
+1;
+__END__
+
+=head1 NAME
+
+Error - Error/exception handling in an OO-ish way
+
+=head1 SYNOPSIS
+
+ use Error qw(:try);
+
+ throw Error::Simple( "A simple error");
+
+ sub xyz {
+ ...
+ record Error::Simple("A simple error")
+ and return;
+ }
+
+ unlink($file) or throw Error::Simple("$file: $!",$!);
+
+ try {
+ do_some_stuff();
+ die "error!" if $condition;
+ throw Error::Simple -text => "Oops!" if $other_condition;
+ }
+ catch Error::IO with {
+ my $E = shift;
+ print STDERR "File ", $E->{'-file'}, " had a problem\n";
+ }
+ except {
+ my $E = shift;
+ my $general_handler=sub {send_message $E->{-description}};
+ return {
+ UserException1 => $general_handler,
+ UserException2 => $general_handler
+ };
+ }
+ otherwise {
+ print STDERR "Well I don't know what to say\n";
+ }
+ finally {
+ close_the_garage_door_already(); # Should be reliable
+ }; # Don't forget the trailing ; or you might be surprised
+
+=head1 DESCRIPTION
+
+The C<Error> package provides two interfaces. Firstly C<Error> provides
+a procedural interface to exception handling. Secondly C<Error> is a
+base class for errors/exceptions that can either be thrown, for
+subsequent catch, or can simply be recorded.
+
+Errors in the class C<Error> should not be thrown directly, but the
+user should throw errors from a sub-class of C<Error>.
+
+=head1 PROCEDURAL INTERFACE
+
+C<Error> exports subroutines to perform exception handling. These will
+be exported if the C<:try> tag is used in the C<use> line.
+
+=over 4
+
+=item try BLOCK CLAUSES
+
+C<try> is the main subroutine called by the user. All other subroutines
+exported are clauses to the try subroutine.
+
+The BLOCK will be evaluated and, if no error is throw, try will return
+the result of the block.
+
+C<CLAUSES> are the subroutines below, which describe what to do in the
+event of an error being thrown within BLOCK.
+
+=item catch CLASS with BLOCK
+
+This clauses will cause all errors that satisfy C<$err-E<gt>isa(CLASS)>
+to be caught and handled by evaluating C<BLOCK>.
+
+C<BLOCK> will be passed two arguments. The first will be the error
+being thrown. The second is a reference to a scalar variable. If this
+variable is set by the catch block then, on return from the catch
+block, try will continue processing as if the catch block was never
+found.
+
+To propagate the error the catch block may call C<$err-E<gt>throw>
+
+If the scalar reference by the second argument is not set, and the
+error is not thrown. Then the current try block will return with the
+result from the catch block.
+
+=item except BLOCK
+
+When C<try> is looking for a handler, if an except clause is found
+C<BLOCK> is evaluated. The return value from this block should be a
+HASHREF or a list of key-value pairs, where the keys are class names
+and the values are CODE references for the handler of errors of that
+type.
+
+=item otherwise BLOCK
+
+Catch any error by executing the code in C<BLOCK>
+
+When evaluated C<BLOCK> will be passed one argument, which will be the
+error being processed.
+
+Only one otherwise block may be specified per try block
+
+=item finally BLOCK
+
+Execute the code in C<BLOCK> either after the code in the try block has
+successfully completed, or if the try block throws an error then
+C<BLOCK> will be executed after the handler has completed.
+
+If the handler throws an error then the error will be caught, the
+finally block will be executed and the error will be re-thrown.
+
+Only one finally block may be specified per try block
+
+=back
+
+=head1 CLASS INTERFACE
+
+=head2 CONSTRUCTORS
+
+The C<Error> object is implemented as a HASH. This HASH is initialized
+with the arguments that are passed to it's constructor. The elements
+that are used by, or are retrievable by the C<Error> class are listed
+below, other classes may add to these.
+
+ -file
+ -line
+ -text
+ -value
+ -object
+
+If C<-file> or C<-line> are not specified in the constructor arguments
+then these will be initialized with the file name and line number where
+the constructor was called from.
+
+If the error is associated with an object then the object should be
+passed as the C<-object> argument. This will allow the C<Error> package
+to associate the error with the object.
+
+The C<Error> package remembers the last error created, and also the
+last error associated with a package. This could either be the last
+error created by a sub in that package, or the last error which passed
+an object blessed into that package as the C<-object> argument.
+
+=over 4
+
+=item throw ( [ ARGS ] )
+
+Create a new C<Error> object and throw an error, which will be caught
+by a surrounding C<try> block, if there is one. Otherwise it will cause
+the program to exit.
+
+C<throw> may also be called on an existing error to re-throw it.
+
+=item with ( [ ARGS ] )
+
+Create a new C<Error> object and returns it. This is defined for
+syntactic sugar, eg
+
+ die with Some::Error ( ... );
+
+=item record ( [ ARGS ] )
+
+Create a new C<Error> object and returns it. This is defined for
+syntactic sugar, eg
+
+ record Some::Error ( ... )
+ and return;
+
+=back
+
+=head2 STATIC METHODS
+
+=over 4
+
+=item prior ( [ PACKAGE ] )
+
+Return the last error created, or the last error associated with
+C<PACKAGE>
+
+=item flush ( [ PACKAGE ] )
+
+Flush the last error created, or the last error associated with
+C<PACKAGE>.It is necessary to clear the error stack before exiting the
+package or uncaught errors generated using C<record> will be reported.
+
+ $Error->flush;
+
+=cut
+
+=back
+
+=head2 OBJECT METHODS
+
+=over 4
+
+=item stacktrace
+
+If the variable C<$Error::Debug> was non-zero when the error was
+created, then C<stacktrace> returns a string created by calling
+C<Carp::longmess>. If the variable was zero the C<stacktrace> returns
+the text of the error appended with the filename and line number of
+where the error was created, providing the text does not end with a
+newline.
+
+=item object
+
+The object this error was associated with
+
+=item file
+
+The file where the constructor of this error was called from
+
+=item line
+
+The line where the constructor of this error was called from
+
+=item text
+
+The text of the error
+
+=back
+
+=head2 OVERLOAD METHODS
+
+=over 4
+
+=item stringify
+
+A method that converts the object into a string. This method may simply
+return the same as the C<text> method, or it may append more
+information. For example the file name and line number.
+
+By default this method returns the C<-text> argument that was passed to
+the constructor, or the string C<"Died"> if none was given.
+
+=item value
+
+A method that will return a value that can be associated with the
+error. For example if an error was created due to the failure of a
+system call, then this may return the numeric value of C<$!> at the
+time.
+
+By default this method returns the C<-value> argument that was passed
+to the constructor.
+
+=back
+
+=head1 PRE-DEFINED ERROR CLASSES
+
+=over 4
+
+=item Error::Simple
+
+This class can be used to hold simple error strings and values. It's
+constructor takes two arguments. The first is a text value, the second
+is a numeric value. These values are what will be returned by the
+overload methods.
+
+If the text value ends with C<at file line 1> as $@ strings do, then
+this infomation will be used to set the C<-file> and C<-line> arguments
+of the error object.
+
+This class is used internally if an eval'd block die's with an error
+that is a plain string. (Unless C<$Error::ObjectifyCallback> is modified)
+
+=back
+
+=head1 $Error::ObjectifyCallback
+
+This variable holds a reference to a subroutine that converts errors that
+are plain strings to objects. It is used by Error.pm to convert textual
+errors to objects, and can be overrided by the user.
+
+It accepts a single argument which is a hash reference to named parameters.
+Currently the only named parameter passed is C<'text'> which is the text
+of the error, but others may be available in the future.
+
+For example the following code will cause Error.pm to throw objects of the
+class MyError::Bar by default:
+
+ sub throw_MyError_Bar
+ {
+ my $args = shift;
+ my $err = MyError::Bar->new();
+ $err->{'MyBarText'} = $args->{'text'};
+ return $err;
+ }
+
+ {
+ local $Error::ObjectifyCallback = \&throw_MyError_Bar;
+
+ # Error handling here.
+ }
+
+=head1 KNOWN BUGS
+
+None, but that does not mean there are not any.
+
+=head1 AUTHORS
+
+Graham Barr <gbarr@pobox.com>
+
+The code that inspired me to write this was originally written by
+Peter Seibel <peter@weblogic.com> and adapted by Jesse Glick
+<jglick@sig.bsh.com>.
+
+=head1 MAINTAINER
+
+Shlomi Fish <shlomif@iglu.org.il>
+
+=head1 PAST MAINTAINERS
+
+Arun Kumar U <u_arunkumar@yahoo.com>
+
+=cut
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index dd61056..54e8b20 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -8,9 +8,19 @@ instlibdir:
MAKE_FRAG
}
+my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');
+
+# We come with our own bundled Error.pm. It's not in the set of default
+# Perl modules so install it if it's not available on the system yet.
+eval { require 'Error' };
+if ($@) {
+ $pm{'Error.pm'} = '$(INST_LIBDIR)/Error.pm';
+}
+
WriteMakefile(
NAME => 'Git',
VERSION_FROM => 'Git.pm',
+ PM => \%pm,
MYEXTLIB => '../libgit.a',
INC => '-I. -I..',
);
^ permalink raw reply related
* [PATCH 07/12] Git.pm: Better error handling
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
So far, errors just killed the whole program and in case of an error
inside of libgit it would be totally uncatchable. This patch makes
Git.pm throw standard Perl exceptions instead. In the future we might
subclass Error to Git::Error or something but for now Error::Simple
is more than enough.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
perl/Git.pm | 37 +++++++++++++++++++++----------------
perl/Git.xs | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 16 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index dcd769b..733fec9 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -88,7 +88,8 @@ increate nonwithstanding).
=cut
-use Carp qw(carp croak);
+use Carp qw(carp); # croak is bad - throw instead
+use Error qw(:try);
require XSLoader;
XSLoader::load('Git', $VERSION);
@@ -143,14 +144,14 @@ sub repository {
if (defined $args[0]) {
if ($#args % 2 != 1) {
# Not a hash.
- $#args == 0 or croak "bad usage";
- %opts = (Directory => $args[0]);
+ $#args == 0 or throw Error::Simple("bad usage");
+ %opts = ( Directory => $args[0] );
} else {
%opts = @args;
}
if ($opts{Directory}) {
- -d $opts{Directory} or croak "Directory not found: $!";
+ -d $opts{Directory} or throw Error::Simple("Directory not found: $!");
if (-d $opts{Directory}."/.git") {
# TODO: Might make this more clever
$opts{WorkingCopy} = $opts{Directory};
@@ -242,11 +243,11 @@ read.
sub command_pipe {
my ($self, $cmd, @args) = _maybe_self(@_);
- $cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+ $cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
my $pid = open(my $fh, "-|");
if (not defined $pid) {
- croak "open failed: $!";
+ throw Error::Simple("open failed: $!");
} elsif ($pid == 0) {
_cmd_exec($self, $cmd, @args);
}
@@ -271,16 +272,17 @@ The function returns only after the comm
sub command_noisy {
my ($self, $cmd, @args) = _maybe_self(@_);
- $cmd =~ /^[a-z0-9A-Z_-]+$/ or croak "bad command: $cmd";
+ $cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
my $pid = fork;
if (not defined $pid) {
- croak "fork failed: $!";
+ throw Error::Simple("fork failed: $!");
} elsif ($pid == 0) {
_cmd_exec($self, $cmd, @args);
}
if (waitpid($pid, 0) > 0 and $? != 0) {
- croak "exit status: $?";
+ # This is the best candidate for a custom exception class.
+ throw Error::Simple("exit status: $?");
}
}
@@ -340,10 +342,10 @@ # Implemented in Git.xs.
=back
-=head1 TODO
+=head1 ERROR HANDLING
-This is still fairly crude.
-We need some good way to report errors back except just dying.
+All functions are supposed to throw Perl exceptions in case of errors.
+See L<Error>.
=head1 COPYRIGHT
@@ -372,8 +374,8 @@ sub _cmd_exec {
$self->{opts}->{Repository} and $ENV{'GIT_DIR'} = $self->{opts}->{Repository};
$self->{opts}->{WorkingCopy} and chdir($self->{opts}->{WorkingCopy});
}
- xs__execv_git_cmd(@args);
- croak "exec failed: $!";
+ _execv_git_cmd(@args);
+ die "exec failed: $!";
}
# Execute the given Git command ($_[0]) with arguments ($_[1..])
@@ -388,7 +390,8 @@ sub _cmd_close {
# It's just close, no point in fatalities
carp "error closing pipe: $!";
} elsif ($? >> 8) {
- croak "exit status: ".($? >> 8);
+ # This is the best candidate for a custom exception class.
+ throw Error::Simple("exit status: ".($? >> 8));
}
# else we might e.g. closed a live stream; the command
# dying of SIGPIPE would drive us here.
@@ -415,6 +418,8 @@ sub _call_gate {
#xs_call_gate($self->{opts}->{Repository});
}
+ # Having to call throw from the C code is a sure path to insanity.
+ local $SIG{__DIE__} = sub { throw Error::Simple("@_"); };
&$xsfunc(@args);
}
@@ -422,7 +427,7 @@ sub AUTOLOAD {
my $xsname;
our $AUTOLOAD;
($xsname = $AUTOLOAD) =~ s/.*:://;
- croak "&Git::$xsname not defined" if $xsname =~ /^xs_/;
+ throw Error::Simple("&Git::$xsname not defined") if $xsname =~ /^xs_/;
$xsname = 'xs_'.$xsname;
_call_gate(\&$xsname, @_);
}
diff --git a/perl/Git.xs b/perl/Git.xs
index 9623fdd..ebaac4b 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -8,6 +8,8 @@ #include <ctype.h>
#include "../cache.h"
#include "../exec_cmd.h"
+#define die perlyshadow_die__
+
/* XS and Perl interface */
#include "EXTERN.h"
#include "perl.h"
@@ -15,11 +17,48 @@ #include "XSUB.h"
#include "ppport.h"
+#undef die
+
+
+static char *
+report_xs(const char *prefix, const char *err, va_list params)
+{
+ static char buf[4096];
+ strcpy(buf, prefix);
+ vsnprintf(buf + strlen(prefix), 4096 - strlen(prefix), err, params);
+ return buf;
+}
+
+void
+die_xs(const char *err, va_list params)
+{
+ char *str;
+ str = report_xs("fatal: ", err, params);
+ croak(str);
+}
+
+int
+error_xs(const char *err, va_list params)
+{
+ char *str;
+ str = report_xs("error: ", err, params);
+ warn(str);
+ return -1;
+}
+
MODULE = Git PACKAGE = Git
PROTOTYPES: DISABLE
+
+BOOT:
+{
+ set_error_routine(error_xs);
+ set_die_routine(die_xs);
+}
+
+
# /* TODO: xs_call_gate(). See Git.pm. */
^ permalink raw reply related
* [PATCH 10/12] Git.pm: Implement options for the command interface
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
This gives the user a way to easily pass options to the command routines.
Currently only the STDERR option is implemented and can be used to adjust
what shall be done with error output of the called command (most usefully,
it can be used to silence it).
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
perl/Git.pm | 37 +++++++++++++++++++++++++++++++++++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 11ec62d..e2b66c4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -36,7 +36,8 @@ # Totally unstable API.
my $lastrev = <$fh>; chomp $lastrev;
$repo->command_close_pipe($fh, $c);
- my $lastrev = $repo->command_oneline('rev-list', '--all');
+ my $lastrev = $repo->command_oneline( [ 'rev-list', '--all' ],
+ STDERR => 0 );
=cut
@@ -178,9 +179,21 @@ sub repository {
=item command ( COMMAND [, ARGUMENTS... ] )
+=item command ( [ COMMAND, ARGUMENTS... ], { Opt => Val ... } )
+
Execute the given Git C<COMMAND> (specify it without the 'git-'
prefix), optionally with the specified extra C<ARGUMENTS>.
+The second more elaborate form can be used if you want to further adjust
+the command execution. Currently, only one option is supported:
+
+B<STDERR> - How to deal with the command's error output. By default (C<undef>)
+it is delivered to the caller's C<STDERR>. A false value (0 or '') will cause
+it to be thrown away. If you want to process it, you can get it in a filehandle
+you specify, but you must be extremely careful; if the error output is not
+very short and you want to read it in the same process as where you called
+C<command()>, you are set up for a nice deadlock!
+
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).
@@ -231,6 +244,8 @@ sub command {
=item command_oneline ( COMMAND [, ARGUMENTS... ] )
+=item command_oneline ( [ COMMAND, ARGUMENTS... ], { Opt => Val ... } )
+
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.
@@ -256,6 +271,8 @@ sub command_oneline {
=item command_output_pipe ( COMMAND [, ARGUMENTS... ] )
+=item command_output_pipe ( [ COMMAND, ARGUMENTS... ], { Opt => Val ... } )
+
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.
@@ -272,6 +289,8 @@ sub command_output_pipe {
=item command_input_pipe ( COMMAND [, ARGUMENTS... ] )
+=item command_input_pipe ( [ COMMAND, ARGUMENTS... ], { Opt => Val ... } )
+
Execute the given C<COMMAND> in the same way as command_output_pipe()
does but return an input pipe filehandle instead; the command output
is not captured.
@@ -534,13 +553,27 @@ sub _check_valid_cmd {
# Common backend for the pipe creators.
sub _command_common_pipe {
my $direction = shift;
- my ($self, $cmd, @args) = _maybe_self(@_);
+ my ($self, @p) = _maybe_self(@_);
+ my (%opts, $cmd, @args);
+ if (ref $p[0]) {
+ ($cmd, @args) = @{shift @p};
+ %opts = ref $p[0] ? %{$p[0]} : @p;
+ } else {
+ ($cmd, @args) = @p;
+ }
_check_valid_cmd($cmd);
my $pid = open(my $fh, $direction);
if (not defined $pid) {
throw Error::Simple("open failed: $!");
} elsif ($pid == 0) {
+ if (defined $opts{STDERR}) {
+ close STDERR;
+ }
+ if ($opts{STDERR}) {
+ open (STDERR, '>&', $opts{STDERR})
+ or die "dup failed: $!";
+ }
_cmd_exec($self, $cmd, @args);
}
return wantarray ? ($fh, join(' ', $cmd, @args)) : $fh;
^ permalink raw reply related
* [PATCH 09/12] Git.pm: Enhance the command_pipe() mechanism
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
Rename command_pipe() to command_output_pipe(), outsource
the functionality to _command_common_pipe().
Add command_input_pipe().
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
perl/Git.pm | 76 +++++++++++++++++++++++++++++++++++++++++------------------
1 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 4205ac5..11ec62d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -32,7 +32,7 @@ # Totally unstable API.
my @revs = $repo->command('rev-list', '--since=last monday', '--all');
- my ($fh, $c) = $repo->command_pipe('rev-list', '--since=last monday', '--all');
+ my ($fh, $c) = $repo->command_output_pipe('rev-list', '--since=last monday', '--all');
my $lastrev = <$fh>; chomp $lastrev;
$repo->command_close_pipe($fh, $c);
@@ -48,7 +48,8 @@ require Exporter;
@EXPORT = qw(git_cmd_try);
# Methods which can be called as standalone functions as well:
-@EXPORT_OK = qw(command command_oneline command_pipe command_noisy
+@EXPORT_OK = qw(command command_oneline command_noisy
+ command_output_pipe command_input_pipe command_close_pipe
version exec_path hash_object git_cmd_try);
@@ -194,7 +195,7 @@ In both cases, the command's stdin and s
=cut
sub command {
- my ($fh, $ctx) = command_pipe(@_);
+ my ($fh, $ctx) = command_output_pipe(@_);
if (not defined wantarray) {
# Nothing to pepper the possible exception with.
@@ -237,7 +238,7 @@ of the command's standard output.
=cut
sub command_oneline {
- my ($fh, $ctx) = command_pipe(@_);
+ my ($fh, $ctx) = command_output_pipe(@_);
my $line = <$fh>;
chomp $line;
@@ -253,40 +254,49 @@ sub command_oneline {
}
-=item command_pipe ( COMMAND [, ARGUMENTS... ] )
+=item command_output_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.
+The function can return C<($pipe, $ctx)> in array context.
+See C<command_close_pipe()> for details.
+
=cut
-sub command_pipe {
- my ($self, $cmd, @args) = _maybe_self(@_);
+sub command_output_pipe {
+ _command_common_pipe('-|', @_);
+}
- $cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
- my $pid = open(my $fh, "-|");
- if (not defined $pid) {
- throw Error::Simple("open failed: $!");
- } elsif ($pid == 0) {
- _cmd_exec($self, $cmd, @args);
- }
- return wantarray ? ($fh, join(' ', $cmd, @args)) : $fh;
+=item command_input_pipe ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given C<COMMAND> in the same way as command_output_pipe()
+does but return an input pipe filehandle instead; the command output
+is not captured.
+
+The function can return C<($pipe, $ctx)> in array context.
+See C<command_close_pipe()> for details.
+
+=cut
+
+sub command_input_pipe {
+ _command_common_pipe('|-', @_);
}
=item command_close_pipe ( PIPE [, CTX ] )
-Close the C<PIPE> as returned from C<command_pipe()>, checking
+Close the C<PIPE> as returned from C<command_*_pipe()>, checking
whether the command finished successfuly. The optional C<CTX> argument
is required if you want to see the command name in the error message,
-and it is the second value returned by C<command_pipe()> when
+and it is the second value returned by C<command_*_pipe()> when
called in array context. The call idiom is:
- my ($fh, $ctx) = $r->command_pipe('status');
- while (<$fh>) { ... }
- $r->command_close_pipe($fh, $ctx);
+ my ($fh, $ctx) = $r->command_output_pipe('status');
+ while (<$fh>) { ... }
+ $r->command_close_pipe($fh, $ctx);
Note that you should not rely on whatever actually is in C<CTX>;
currently it is simply the command name but in future the context might
@@ -317,8 +327,7 @@ The function returns only after the comm
sub command_noisy {
my ($self, $cmd, @args) = _maybe_self(@_);
-
- $cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
+ _check_valid_cmd($cmd);
my $pid = fork;
if (not defined $pid) {
@@ -404,7 +413,7 @@ string with the captured command output
call context; C<command_noisy()> returns C<undef>) and $<cmdline> which
returns the command and its arguments (but without proper quoting).
-Note that the C<command_pipe()> function cannot throw this exception since
+Note that the C<command_*_pipe()> functions cannot throw this exception since
it has no idea whether the command failed or not. You will only find out
at the time you C<close> the pipe; if you want to have that automated,
use C<command_close_pipe()>, which can throw the exception.
@@ -516,6 +525,27 @@ sub _maybe_self {
ref $_[0] eq 'Git' ? @_ : (undef, @_);
}
+# Check if the command id is something reasonable.
+sub _check_valid_cmd {
+ my ($cmd) = @_;
+ $cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple("bad command: $cmd");
+}
+
+# Common backend for the pipe creators.
+sub _command_common_pipe {
+ my $direction = shift;
+ my ($self, $cmd, @args) = _maybe_self(@_);
+ _check_valid_cmd($cmd);
+
+ my $pid = open(my $fh, $direction);
+ if (not defined $pid) {
+ throw Error::Simple("open failed: $!");
+ } elsif ($pid == 0) {
+ _cmd_exec($self, $cmd, @args);
+ }
+ return wantarray ? ($fh, join(' ', $cmd, @args)) : $fh;
+}
+
# When already in the subprocess, set up the appropriate state
# for the given repository and execute the git command.
sub _cmd_exec {
^ permalink raw reply related
* [PATCH 08/12] Git.pm: Handle failed commands' output
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
Currently if an external command returns error exit code, a generic exception
is thrown and there is no chance for the caller to retrieve the command's
output.
This patch introduces a Git::Error::Command exception class which is thrown
in this case and contains both the error code and the captured command output.
You can use the new git_cmd_try statement to fatally catch the exception
while producing a user-friendly message.
It also adds command_close_pipe() for easier checking of exit status of
a command we have just a pipe handle of. It has partial forward dependency
on the next patch, but basically only in the area of documentation.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
git-fmt-merge-msg.perl | 13 +++
perl/Git.pm | 192 +++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 183 insertions(+), 22 deletions(-)
diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
index be2a48c..f86231e 100755
--- a/git-fmt-merge-msg.perl
+++ b/git-fmt-merge-msg.perl
@@ -7,6 +7,7 @@ # by grouping branches and tags together
use strict;
use Git;
+use Error qw(:try);
my $repo = Git->repository();
@@ -31,7 +32,17 @@ sub andjoin {
}
sub repoconfig {
- my ($val) = $repo->command_oneline('repo-config', '--get', 'merge.summary');
+ my $val;
+ try {
+ $val = $repo->command_oneline('repo-config', '--get', 'merge.summary');
+ } catch Git::Error::Command with {
+ my ($E) = shift;
+ if ($E->value() == 1) {
+ return undef;
+ } else {
+ throw $E;
+ }
+ };
return $val;
}
diff --git a/perl/Git.pm b/perl/Git.pm
index 733fec9..4205ac5 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -24,16 +24,17 @@ # Totally unstable API.
my $version = Git::command_oneline('version');
- Git::command_noisy('update-server-info');
+ git_cmd_try { Git::command_noisy('update-server-info') }
+ '%s failed w/ code %d';
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 ($fh, $c) = $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
+ $repo->command_close_pipe($fh, $c);
my $lastrev = $repo->command_oneline('rev-list', '--all');
@@ -44,11 +45,11 @@ require Exporter;
@ISA = qw(Exporter);
-@EXPORT = qw();
+@EXPORT = qw(git_cmd_try);
# Methods which can be called as standalone functions as well:
@EXPORT_OK = qw(command command_oneline command_pipe command_noisy
- version exec_path hash_object);
+ version exec_path hash_object git_cmd_try);
=head1 DESCRIPTION
@@ -88,7 +89,7 @@ increate nonwithstanding).
=cut
-use Carp qw(carp); # croak is bad - throw instead
+use Carp qw(carp croak); # but croak is bad - throw instead
use Error qw(:try);
require XSLoader;
@@ -193,21 +194,35 @@ In both cases, the command's stdin and s
=cut
sub command {
- my $fh = command_pipe(@_);
+ my ($fh, $ctx) = command_pipe(@_);
if (not defined wantarray) {
- _cmd_close($fh);
+ # Nothing to pepper the possible exception with.
+ _cmd_close($fh, $ctx);
} elsif (not wantarray) {
local $/;
my $text = <$fh>;
- _cmd_close($fh);
+ try {
+ _cmd_close($fh, $ctx);
+ } catch Git::Error::Command with {
+ # Pepper with the output:
+ my $E = shift;
+ $E->{'-outputref'} = \$text;
+ throw $E;
+ };
return $text;
} else {
my @lines = <$fh>;
- _cmd_close($fh);
chomp @lines;
+ try {
+ _cmd_close($fh, $ctx);
+ } catch Git::Error::Command with {
+ my $E = shift;
+ $E->{'-outputref'} = \@lines;
+ throw $E;
+ };
return @lines;
}
}
@@ -222,12 +237,18 @@ of the command's standard output.
=cut
sub command_oneline {
- my $fh = command_pipe(@_);
+ my ($fh, $ctx) = command_pipe(@_);
my $line = <$fh>;
- _cmd_close($fh);
-
chomp $line;
+ try {
+ _cmd_close($fh, $ctx);
+ } catch Git::Error::Command with {
+ # Pepper with the output:
+ my $E = shift;
+ $E->{'-outputref'} = \$line;
+ throw $E;
+ };
return $line;
}
@@ -251,7 +272,32 @@ sub command_pipe {
} elsif ($pid == 0) {
_cmd_exec($self, $cmd, @args);
}
- return $fh;
+ return wantarray ? ($fh, join(' ', $cmd, @args)) : $fh;
+}
+
+
+=item command_close_pipe ( PIPE [, CTX ] )
+
+Close the C<PIPE> as returned from C<command_pipe()>, checking
+whether the command finished successfuly. The optional C<CTX> argument
+is required if you want to see the command name in the error message,
+and it is the second value returned by C<command_pipe()> when
+called in array context. The call idiom is:
+
+ my ($fh, $ctx) = $r->command_pipe('status');
+ while (<$fh>) { ... }
+ $r->command_close_pipe($fh, $ctx);
+
+Note that you should not rely on whatever actually is in C<CTX>;
+currently it is simply the command name but in future the context might
+have more complicated structure.
+
+=cut
+
+sub command_close_pipe {
+ my ($self, $fh, $ctx) = _maybe_self(@_);
+ $ctx ||= '<unknown>';
+ _cmd_close($fh, $ctx);
}
@@ -280,9 +326,8 @@ sub command_noisy {
} elsif ($pid == 0) {
_cmd_exec($self, $cmd, @args);
}
- if (waitpid($pid, 0) > 0 and $? != 0) {
- # This is the best candidate for a custom exception class.
- throw Error::Simple("exit status: $?");
+ if (waitpid($pid, 0) > 0 and $?>>8 != 0) {
+ throw Git::Error::Command(join(' ', $cmd, @args), $? >> 8);
}
}
@@ -340,12 +385,117 @@ are involved.
# Implemented in Git.xs.
+
=back
=head1 ERROR HANDLING
All functions are supposed to throw Perl exceptions in case of errors.
-See L<Error>.
+See the L<Error> module on how to catch those. Most exceptions are mere
+L<Error::Simple> instances.
+
+However, the C<command()>, C<command_oneline()> and C<command_noisy()>
+functions suite can throw C<Git::Error::Command> exceptions as well: those are
+thrown when the external command returns an error code and contain the error
+code as well as access to the captured command's output. The exception class
+provides the usual C<stringify> and C<value> (command's exit code) methods and
+in addition also a C<cmd_output> method that returns either an array or a
+string with the captured command output (depending on the original function
+call context; C<command_noisy()> returns C<undef>) and $<cmdline> which
+returns the command and its arguments (but without proper quoting).
+
+Note that the C<command_pipe()> function cannot throw this exception since
+it has no idea whether the command failed or not. You will only find out
+at the time you C<close> the pipe; if you want to have that automated,
+use C<command_close_pipe()>, which can throw the exception.
+
+=cut
+
+{
+ package Git::Error::Command;
+
+ @Git::Error::Command::ISA = qw(Error);
+
+ sub new {
+ my $self = shift;
+ my $cmdline = '' . shift;
+ my $value = 0 + shift;
+ my $outputref = shift;
+ my(@args) = ();
+
+ local $Error::Depth = $Error::Depth + 1;
+
+ push(@args, '-cmdline', $cmdline);
+ push(@args, '-value', $value);
+ push(@args, '-outputref', $outputref);
+
+ $self->SUPER::new(-text => 'command returned error', @args);
+ }
+
+ sub stringify {
+ my $self = shift;
+ my $text = $self->SUPER::stringify;
+ $self->cmdline() . ': ' . $text . ': ' . $self->value() . "\n";
+ }
+
+ sub cmdline {
+ my $self = shift;
+ $self->{'-cmdline'};
+ }
+
+ sub cmd_output {
+ my $self = shift;
+ my $ref = $self->{'-outputref'};
+ defined $ref or undef;
+ if (ref $ref eq 'ARRAY') {
+ return @$ref;
+ } else { # SCALAR
+ return $$ref;
+ }
+ }
+}
+
+=over 4
+
+=item git_cmd_try { CODE } ERRMSG
+
+This magical statement will automatically catch any C<Git::Error::Command>
+exceptions thrown by C<CODE> and make your program die with C<ERRMSG>
+on its lips; the message will have %s substituted for the command line
+and %d for the exit status. This statement is useful mostly for producing
+more user-friendly error messages.
+
+In case of no exception caught the statement returns C<CODE>'s return value.
+
+Note that this is the only auto-exported function.
+
+=cut
+
+sub git_cmd_try(&$) {
+ my ($code, $errmsg) = @_;
+ my @result;
+ my $err;
+ my $array = wantarray;
+ try {
+ if ($array) {
+ @result = &$code;
+ } else {
+ $result[0] = &$code;
+ }
+ } catch Git::Error::Command with {
+ my $E = shift;
+ $err = $errmsg;
+ $err =~ s/\%s/$E->cmdline()/ge;
+ $err =~ s/\%d/$E->value()/ge;
+ # We can't croak here since Error.pm would mangle
+ # that to Error::Simple.
+ };
+ $err and croak $err;
+ return $array ? @result : $result[0];
+}
+
+
+=back
=head1 COPYRIGHT
@@ -384,14 +534,14 @@ # _execv_git_cmd(), implemented in Git.x
# Close pipe to a subprocess.
sub _cmd_close {
- my ($fh) = @_;
+ my ($fh, $ctx) = @_;
if (not close $fh) {
if ($!) {
# It's just close, no point in fatalities
carp "error closing pipe: $!";
} elsif ($? >> 8) {
- # This is the best candidate for a custom exception class.
- throw Error::Simple("exit status: ".($? >> 8));
+ # The caller should pepper this.
+ throw Git::Error::Command($ctx, $? >> 8);
}
# else we might e.g. closed a live stream; the command
# dying of SIGPIPE would drive us here.
^ permalink raw reply related
* [PATCH 11/12] Git.pm: Add support for subdirectories inside of working copies
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
This patch adds support for subdirectories inside of working copies;
you can specify them in the constructor either as the Directory
option (it will just get autodetected using rev-parse) or explicitly
using the WorkingSubdir option. This makes Git->repository() do the
exact same path setup and repository lookup as the Git porcelain
does.
This patch also introduces repo_path(), wc_path() and wc_subdir()
accessor methods and wc_chdir() mutator.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
perl/Git.pm | 157 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 129 insertions(+), 28 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index e2b66c4..7bbb5be 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -69,20 +69,18 @@ (In the future, we will also get a new_r
called as methods of the object are then executed in the context of the
repository.
-TODO: In the future, we might also do
+Part of the "repository state" is also information about path to the attached
+working copy (unless you work with a bare repository). You can also navigate
+inside of the working copy using the C<wc_chdir()> method. (Note that
+the repository object is self-contained and will not change working directory
+of your process.)
- my $subdir = $repo->subdir('Documentation');
- # Gets called in the subdirectory context:
- $subdir->command('status');
+TODO: In the future, we might also do
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
@@ -93,6 +91,7 @@ increate nonwithstanding).
use Carp qw(carp croak); # but croak is bad - throw instead
use Error qw(:try);
+use Cwd qw(abs_path);
require XSLoader;
XSLoader::load('Git', $VERSION);
@@ -119,12 +118,17 @@ B<Repository> - Path to the Git reposito
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<WorkingSubdir> - Subdirectory in the working copy to work inside.
+Just left undefined if you do not want to limit the scope of operations.
+
+B<Directory> - Path to the Git working directory in its usual setup.
+The C<.git> directory is searched in the directory and all the parent
+directories; if found, C<WorkingCopy> is set to the directory containing
+it and C<Repository> to the C<.git> directory itself. If no C<.git>
+directory was found, the C<Directory> is assumed to be a bare repository,
+C<Repository> is set to point at it and C<WorkingCopy> is left undefined.
+If the C<$GIT_DIR> environment variable is set, things behave as expected
+as well.
You should not use both C<Directory> and either of C<Repository> and
C<WorkingCopy> - the results of that are undefined.
@@ -134,7 +138,10 @@ to the constructor; it is equivalent to
field.
Calling the constructor with no options whatsoever is equivalent to
-calling it with C<< Directory => '.' >>.
+calling it with C<< Directory => '.' >>. In general, if you are building
+a standard porcelain command, simply doing C<< Git->repository() >> should
+do the right thing and setup the object to reflect exactly where the user
+is right now.
=cut
@@ -152,18 +159,59 @@ sub repository {
} else {
%opts = @args;
}
+ }
+
+ if (not defined $opts{Repository} and not defined $opts{WorkingCopy}) {
+ $opts{Directory} ||= '.';
+ }
+
+ if ($opts{Directory}) {
+ -d $opts{Directory} or throw Error::Simple("Directory not found: $!");
+
+ my $search = Git->repository(WorkingCopy => $opts{Directory});
+ my $dir;
+ try {
+ $dir = $search->command_oneline(['rev-parse', '--git-dir'],
+ STDERR => 0);
+ } catch Git::Error::Command with {
+ $dir = undef;
+ };
- if ($opts{Directory}) {
- -d $opts{Directory} or throw Error::Simple("Directory not found: $!");
- if (-d $opts{Directory}."/.git") {
- # TODO: Might make this more clever
- $opts{WorkingCopy} = $opts{Directory};
- $opts{Repository} = $opts{Directory}."/.git";
- } else {
- $opts{Repository} = $opts{Directory};
+ if ($dir) {
+ $opts{Repository} = abs_path($dir);
+
+ # If --git-dir went ok, this shouldn't die either.
+ my $prefix = $search->command_oneline('rev-parse', '--show-prefix');
+ $dir = abs_path($opts{Directory}) . '/';
+ if ($prefix) {
+ if (substr($dir, -length($prefix)) ne $prefix) {
+ throw Error::Simple("rev-parse confused me - $dir does not have trailing $prefix");
+ }
+ substr($dir, -length($prefix)) = '';
}
- delete $opts{Directory};
+ $opts{WorkingCopy} = $dir;
+ $opts{WorkingSubdir} = $prefix;
+
+ } else {
+ # A bare repository? Let's see...
+ $dir = $opts{Directory};
+
+ unless (-d "$dir/refs" and -d "$dir/objects" and -e "$dir/HEAD") {
+ # Mimick git-rev-parse --git-dir error message:
+ throw Error::Simple('fatal: Not a git repository');
+ }
+ my $search = Git->repository(Repository => $dir);
+ try {
+ $search->command('symbolic-ref', 'HEAD');
+ } catch Git::Error::Command with {
+ # Mimick git-rev-parse --git-dir error message:
+ throw Error::Simple('fatal: Not a git repository');
+ }
+
+ $opts{Repository} = abs_path($dir);
}
+
+ delete $opts{Directory};
}
$self = { opts => \%opts };
@@ -256,7 +304,7 @@ sub command_oneline {
my ($fh, $ctx) = command_output_pipe(@_);
my $line = <$fh>;
- chomp $line;
+ defined $line and chomp $line;
try {
_cmd_close($fh, $ctx);
} catch Git::Error::Command with {
@@ -374,7 +422,7 @@ # Implemented in Git.xs.
=item exec_path ()
-Return path to the git sub-command executables (the same as
+Return path to the Git sub-command executables (the same as
C<git --exec-path>). Useful mostly only internally.
Implementation of this function is very fast; no external command calls
@@ -385,6 +433,58 @@ are involved.
# Implemented in Git.xs.
+=item repo_path ()
+
+Return path to the git repository. Must be called on a repository instance.
+
+=cut
+
+sub repo_path { $_[0]->{opts}->{Repository} }
+
+
+=item wc_path ()
+
+Return path to the working copy. Must be called on a repository instance.
+
+=cut
+
+sub wc_path { $_[0]->{opts}->{WorkingCopy} }
+
+
+=item wc_subdir ()
+
+Return path to the subdirectory inside of a working copy. Must be called
+on a repository instance.
+
+=cut
+
+sub wc_subdir { $_[0]->{opts}->{WorkingSubdir} ||= '' }
+
+
+=item wc_chdir ( SUBDIR )
+
+Change the working copy subdirectory to work within. The C<SUBDIR> is
+relative to the working copy root directory (not the current subdirectory).
+Must be called on a repository instance attached to a working copy
+and the directory must exist.
+
+=cut
+
+sub wc_chdir {
+ my ($self, $subdir) = @_;
+
+ $self->wc_path()
+ or throw Error::Simple("bare repository");
+
+ -d $self->wc_path().'/'.$subdir
+ or throw Error::Simple("subdir not found: $!");
+ # Of course we will not "hold" the subdirectory so anyone
+ # can delete it now and we will never know. But at least we tried.
+
+ $self->{opts}->{WorkingSubdir} = $subdir;
+}
+
+
=item hash_object ( FILENAME [, TYPE ] )
=item hash_object ( FILEHANDLE [, TYPE ] )
@@ -584,8 +684,9 @@ # for the given repository and execute t
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});
+ $self->repo_path() and $ENV{'GIT_DIR'} = $self->repo_path();
+ $self->wc_path() and chdir($self->wc_path());
+ $self->wc_subdir() and chdir($self->wc_subdir());
}
_execv_git_cmd(@args);
die "exec failed: $!";
^ permalink raw reply related
* [PATCH 12/12] Convert git-mv to use Git.pm
From: Petr Baudis @ 2006-06-24 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
Fairly straightforward.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
git-mv.perl | 46 ++++++++++++++++++++++------------------------
1 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/git-mv.perl b/git-mv.perl
index 75aa8fe..b9e3537 100755
--- a/git-mv.perl
+++ b/git-mv.perl
@@ -10,6 +10,8 @@ # at the discretion of Linus Torvalds.
use warnings;
use strict;
use Getopt::Std;
+use Git;
+use Error qw(:try);
sub usage() {
print <<EOT;
@@ -24,9 +26,7 @@ getopts("hnfkv") || usage;
usage() if $opt_h;
@ARGV >= 1 or usage;
-my $GIT_DIR = `git rev-parse --git-dir`;
-exit 1 if $?; # rev-parse would have given "not a git dir" message.
-chomp($GIT_DIR);
+my $repo = Git->repository();
my (@srcArgs, @dstArgs, @srcs, @dsts);
my ($src, $dst, $base, $dstDir);
@@ -62,11 +62,11 @@ else {
$dstDir = "";
}
-my $subdir_prefix = `git rev-parse --show-prefix`;
-chomp($subdir_prefix);
+my $subdir_prefix = $repo->wc_subdir();
# run in git base directory, so that git-ls-files lists all revisioned files
-chdir "$GIT_DIR/..";
+chdir $repo->wc_path();
+$repo->wc_chdir('');
# normalize paths, needed to compare against versioned files and update-index
# also, this is nicer to end-users by doing ".//a/./b/.//./c" ==> "a/b/c"
@@ -84,12 +84,10 @@ my (@allfiles,@srcfiles,@dstfiles);
my $safesrc;
my (%overwritten, %srcForDst);
-$/ = "\0";
-open(F, 'git-ls-files -z |')
- or die "Failed to open pipe from git-ls-files: " . $!;
-
-@allfiles = map { chomp; $_; } <F>;
-close(F);
+{
+ local $/ = "\0";
+ @allfiles = $repo->command('ls-files', '-z');
+}
my ($i, $bad);
@@ -219,28 +217,28 @@ if ($opt_n) {
}
else {
if (@changedfiles) {
- open(H, "| git-update-index -z --stdin")
- or die "git-update-index failed to update changed files with code $!\n";
+ my ($fd, $ctx) = $repo->command_input_pipe('update-index', '-z', '--stdin');
foreach my $fileName (@changedfiles) {
- print H "$fileName\0";
+ print $fd "$fileName\0";
}
- close(H);
+ git_cmd_try { $repo->command_close_pipe($fd, $ctx); }
+ 'git-update-index failed to update changed files with code %d';
}
if (@addedfiles) {
- open(H, "| git-update-index --add -z --stdin")
- or die "git-update-index failed to add new names with code $!\n";
+ my ($fd, $ctx) = $repo->command_input_pipe('update-index', '--add', '-z', '--stdin');
foreach my $fileName (@addedfiles) {
- print H "$fileName\0";
+ print $fd "$fileName\0";
}
- close(H);
+ git_cmd_try { $repo->command_close_pipe($fd, $ctx); }
+ 'git-update-index failed to add new files with code %d';
}
if (@deletedfiles) {
- open(H, "| git-update-index --remove -z --stdin")
- or die "git-update-index failed to remove old names with code $!\n";
+ my ($fd, $ctx) = $repo->command_input_pipe('update-index', '--remove', '-z', '--stdin');
foreach my $fileName (@deletedfiles) {
- print H "$fileName\0";
+ print $fd "$fileName\0";
}
- close(H);
+ git_cmd_try { $repo->command_close_pipe($fd, $ctx); }
+ 'git-update-index failed to remove old files with code %d';
}
}
^ permalink raw reply related
* Re: [PATCH 12/12] Convert git-mv to use Git.pm
From: Petr Baudis @ 2006-06-24 2:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624023453.32751.73126.stgit@machine.or.cz>
Dear diary, on Sat, Jun 24, 2006 at 04:34:53AM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> +use Error qw(:try);
Duh, no matter how long I stare at the patches before submitting them,
something always slips through. This isn't actually necessary anymore
since we have git_cmd_try now. (The try { } catch { } blocks turned
quite fat and ugly.)
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.
^ permalink raw reply
* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Junio C Hamano @ 2006-06-24 2:46 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>
Just to let you know, I already have v3 in my tree which is
merged in "pu". With the two fixes I sent you earlier I think
it is in a good shape to be cooked in "next".
I do not think I'd have trouble applying this new series (I
would probably start from "master" to apply it and perhaps merge
or --squash merge it onto pb/gitpm topic branch that has v3 with
the two fixes I sent you separately) but we will see soon
enough.
^ permalink raw reply
* Re: [Patch] trap: exit: invalid signal specification
From: Junio C Hamano @ 2006-06-24 2:50 UTC (permalink / raw)
To: caglar; +Cc: git
In-Reply-To: <200606240410.18334.caglar@pardus.org.tr>
> diff -ur git-1.4.0.orig/git-clone.sh git-1.4.0/git-clone.sh
> --- git-1.4.0.orig/git-clone.sh 2006-06-10 22:41:54.000000000 +0300
> +++ git-1.4.0/git-clone.sh 2006-06-24 03:54:49.000000000 +0300
> @@ -205,7 +205,7 @@
> [ -e "$dir" ] && echo "$dir already exists." && usage
> mkdir -p "$dir" &&
> D=$(cd "$dir" && pwd) &&
> -trap 'err=$?; cd ..; rm -r "$D"; exit $err' 0
> +trap 'err=$?; cd ..; rm -r "$D"; EXIT $err' 0
> case "$bare" in
> yes)
I am not quite sure what to make out this... Do you mean your
shell does not like the command "exit" spelled in lowercase
under Turkic locale?
^ permalink raw reply
* Re: [PATCH 0/5] Rework diff options
From: Junio C Hamano @ 2006-06-24 2:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Timo Hirvonen, git
In-Reply-To: <Pine.LNX.4.63.0606240201580.29667@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Fri, 23 Jun 2006, Junio C Hamano wrote:
>
>> I personally feel that the benefit of being able to make sure you
>> covered everything outweighs the size of initial diff.
>
> IMHO the difficulty of finding bugs is proportional to the square of the
> diff size, while the number of people willing to review it is proportional
> to its square root. So, if it is not difficult (which it is not at all in
> this case), I politely ask to cut the patch size down.
Yeah, your "renaming in private while you are making sure but
rename them back" suggestion actually would work in this case.
^ permalink raw reply
* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24 3:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr71f5kzs.fsf@assigned-by-dhcp.cox.net>
Dear diary, on Sat, Jun 24, 2006 at 04:46:15AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Just to let you know, I already have v3 in my tree which is
> merged in "pu". With the two fixes I sent you earlier I think
> it is in a good shape to be cooked in "next".
Ah, yes, could you please apply your fixes as well? :-)
> I do not think I'd have trouble applying this new series (I
> would probably start from "master" to apply it and perhaps merge
> or --squash merge it onto pb/gitpm topic branch that has v3 with
> the two fixes I sent you separately) but we will see soon
> enough.
I noticed v3 in pu but thought that it should be trivial to just throw
that away from pu and take v4 instead - I'm sorry if that turns out to
be an issue.
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.
^ permalink raw reply
* commit triggers.
From: Dave Jones @ 2006-06-24 3:29 UTC (permalink / raw)
To: git
I've grepped around, and come up with nothing, so hopefully
I'm not overlooking some already-implemented feature
(though it'd be great if I am). How much work is involved in
creating a mechanism where some scripts living in say .git/triggers/
get executed on commits ?
The idea behind this stems from some scripts I've been running
periodically against an mbox of the daily kernel commits, which
greps for common bugs; kmalloc(GFP, size) instead of kmalloc(size,GFP),
memsets with reversed 2nd/3rd args etc etc.
It'd be useful I think to have a way to hook up such a script
to git's commit process that aborts a commit if a script returns
a hit, forcing the user to fix up the mistake before committing
it to the world.
thoughts ?
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply
* [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Martin Langhoff @ 2006-06-24 5:09 UTC (permalink / raw)
To: git, junkio, Johannes.Schindelin; +Cc: Martin Langhoff
In-Reply-To: <Pine.LNX.4.63.0606231811200.29667@wbgn013.biozentrum.uni-wuerzburg.de>
On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> It seems that git-cvsimport makes a temporary file of size 0, which cannot
> get mmap()ed, because it has size 0.
This switch to tmpnam() avoids creating the tmpfile in the first place and
streamlines the code. This handling of tmpfiles is slightly safer, but there
is an inherent race condition.
---
NOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,
if trivial.
However, this switch to tmpnam() avoids creating the tmpfile in the first place
and streamlines the code. This usage of tempfiles is open to a race condition
if someone could guess the name returned by tmpnam, but even this is safer than
what we did before, which was creating a file, closing the fh and then
clobbering it from git-read-tree.
And if someone can guess the name that tmpnam() returns their magic is strong
enough that they'll go for more interesting targets.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-cvsimport.perl | 20 +++++---------------
1 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index f3daa6c..d961b7b 100644
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -17,7 +17,7 @@ use strict;
use warnings;
use Getopt::Std;
use File::Spec;
-use File::Temp qw(tempfile);
+use File::Temp qw(tempfile tmpnam);
use File::Path qw(mkpath);
use File::Basename qw(basename dirname);
use Time::Local;
@@ -467,12 +467,8 @@ my $orig_git_index;
$orig_git_index = $ENV{GIT_INDEX_FILE} if exists $ENV{GIT_INDEX_FILE};
my %index; # holds filenames of one index per branch
-{ # init with an index for origin
- my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
- DIR => File::Spec->tmpdir());
- close ($fh);
- $index{$opt_o} = $fn;
-}
+$index{$opt_o} = tmpnam();
+
$ENV{GIT_INDEX_FILE} = $index{$opt_o};
unless(-d $git_dir) {
system("git-init-db");
@@ -502,10 +498,7 @@ unless(-d $git_dir) {
# populate index
unless ($index{$last_branch}) {
- my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
- DIR => File::Spec->tmpdir());
- close ($fh);
- $index{$last_branch} = $fn;
+ $index{$last_branch} = tmpnam();
}
$ENV{GIT_INDEX_FILE} = $index{$last_branch};
system('git-read-tree', $last_branch);
@@ -818,10 +811,7 @@ while(<CVS>) {
if(($ancestor || $branch) ne $last_branch) {
print "Switching from $last_branch to $branch\n" if $opt_v;
unless ($index{$branch}) {
- my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
- DIR => File::Spec->tmpdir());
- close ($fh);
- $index{$branch} = $fn;
+ $index{$branch} = tmpnam();
$ENV{GIT_INDEX_FILE} = $index{$branch};
system("git-read-tree", $branch);
die "read-tree failed: $?\n" if $?;
--
1.4.0.gcda2
^ permalink raw reply related
* Re: commit triggers.
From: Junio C Hamano @ 2006-06-24 6:43 UTC (permalink / raw)
To: Dave Jones; +Cc: git
In-Reply-To: <20060624032908.GH19461@redhat.com>
Dave Jones <davej@redhat.com> writes:
> I've grepped around, and come up with nothing, so hopefully
> I'm not overlooking some already-implemented feature
> (though it'd be great if I am). How much work is involved in
> creating a mechanism where some scripts living in say .git/triggers/
> get executed on commits ?
Perhaps "grep hooks/ git-commit.sh"?
The hooks we have right now are more interested in preventing
you from making crappy commits, so we might not have a
post-commit hook you can use for notification purposes, but if
that is the case I am open to suggestions to add one. I think
post-commit hook is already there, though.
^ permalink raw reply
* Re: x86 asm SHA1 (draft)
From: linux @ 2006-06-24 1:22 UTC (permalink / raw)
To: git, junkio; +Cc: linux
In-Reply-To: <7vzmg376ee.fsf@assigned-by-dhcp.cox.net>
> The series to revamp SHA1 is good but judging the merit of each
> is outside my expertise, so I'd appreciate help to evaluate
> these changes. For example, I cannot choose between competing
> three implementations for ppc without having access to a variety
> of ppc machines, and even if I did, ppc is not what I normally
> use, so incentive to try picking the best one for everybody is
> relatively low on my part.
Well, I'm not sure it's worth this much trouble. Both of my PPC
implementations are smaller and faster than the current one,
so that's a pretty easy decision. The difference between them
is 2-3%, which is, I think, not enough to be worth the maintenance
burden of a run-time decision infrastructure. Just pick either one
and call it a day.
> The only external interface for the set of SHA1 implementation
> alternatives to the outside world is a well established SHA_CTX
> type, and three functions SHA1_Init(), SHA1_Update() and
> SHA1_Final(), and the alternative implementations are supposed
> to be drop-in replaceable.
I'd prefer it it was an *opaque* SHA_CTX type. Sometimes you can achieve
some useful benefits by rearranging the fields. For example, keeping the
64-bit length field as a native-order 64-bit number when appropriate.
And sometimes it's useful to have the full 80-word W[] array, while
other implementations don't want it.
> We probably would want to collect the benchmark results from
> popular platforms, have a summary to help people to choose a
> sensible one in the toplevel INSTALL file, and include the raw
> numbers in Documentation/technical/sha1-implementations.txt.
Not that numbers are bad, but I think that until there's a real
need for more than a single good-enough version per instruction set,
this is excessive. Does hashing even show up on a profile?
^ permalink raw reply
* Re: [PATCH 2/5] Rework diff options
From: Junio C Hamano @ 2006-06-24 6:55 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: git
In-Reply-To: <20060624005252.c694e421.tihirvon@gmail.com>
Timo Hirvonen <tihirvon@gmail.com> writes:
> diff --git a/builtin-diff.c b/builtin-diff.c
> index 99a2f76..372894a 100644
> --- a/builtin-diff.c
> +++ b/builtin-diff.c
> @@ -56,15 +56,15 @@ static int builtin_diff_files(struct rev
> 3 < revs->max_count)
> usage(builtin_diff_usage);
> if (revs->max_count < 0 &&
> - (revs->diffopt.output_format == DIFF_FORMAT_PATCH))
> + (revs->diffopt.output_fmt & OUTPUT_FMT_PATCH))
> revs->combine_merges = revs->dense_combined_merges = 1;
> /*
> * Backward compatibility wart - "diff-files -s" used to
> * defeat the common diff option "-s" which asked for
> - * DIFF_FORMAT_NO_OUTPUT.
> + * OUTPUT_FMT_NONE
> */
> - if (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
> - revs->diffopt.output_format = DIFF_FORMAT_RAW;
> + if (revs->diffopt.output_fmt & OUTPUT_FMT_NONE)
> + revs->diffopt.output_fmt = OUTPUT_FMT_RAW;
> return run_diff_files(revs, silent);
> }
We do not have to remove this now, but I think we can remove
this "backward compatibility wart" sometime in the next round;
Cogito has been fixed not to use this.
> @@ -110,7 +110,7 @@ static int builtin_diff_b_f(struct rev_i
> while (1 < argc) {
> const char *arg = argv[1];
> if (!strcmp(arg, "--raw"))
> - revs->diffopt.output_format = DIFF_FORMAT_RAW;
> + revs->diffopt.output_fmt |= OUTPUT_FMT_RAW;
> else
> usage(builtin_diff_usage);
> argv++; argc--;
I see later in the series you taught low-level diff_opt_parse
about --raw switch, which is good.
> diff --git a/builtin-log.c b/builtin-log.c
> index 5a8a50b..e4a6385 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -26,8 +26,8 @@ static int cmd_log_wc(int argc, const ch
> if (rev->always_show_header) {
> if (rev->diffopt.pickaxe || rev->diffopt.filter) {
> rev->always_show_header = 0;
> - if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
> - rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
> + if (rev->diffopt.output_fmt & OUTPUT_FMT_RAW)
> + rev->diffopt.output_fmt |= OUTPUT_FMT_NONE;
> }
> }
The original code is saying "For git-log command (i.e. when
always-show-header is on), if the command line did not override
but ended up asking for diff only because it wanted to do -S or
--diff-filter, do not show any diff" which is quite an opaque
logic.
> diff --git a/combine-diff.c b/combine-diff.c
> index 64b20cc..d0d8d01 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c
> num_paths++;
> }
> if (num_paths) {
> - if (opt->with_raw) {
> - int saved_format = opt->output_format;
> - opt->output_format = DIFF_FORMAT_RAW;
> + if (opt->output_fmt & OUTPUT_FMT_RAW) {
> + int saved_fmt = opt->output_fmt;
> + opt->output_fmt |= OUTPUT_FMT_RAW;
> for (p = paths; p; p = p->next) {
> show_combined_diff(p, num_parent, dense, rev);
> }
> - opt->output_format = saved_format;
> + opt->output_fmt = saved_fmt;
> putchar(opt->line_termination);
> }
> for (p = paths; p; p = p->next) {
The code needs to run show_combined_diff twice, once for raw
output and another for normal output, since the processing
needed to be done in show_combined_diff are quite different
between the case where you emit raw diff (in which case you do
not combine at the hunk level) and normal diff (in which case
you do). Since your modified show_combined_diff checks FMT_RAW
first, I suspect it would not make any difference if it is ORing
OUTPUT_FMT_RAW into output_fmt or assignment. In fact, since
the bit is already on when the if () statement is taken, I think
you can lose the whole saved_fmt business, and just say
something like this here:
if (opt->output_fmt & OUTPUT_FMT_RAW) {
for (p = paths; p; p = p->next) {
show_combined_diff(p, num_parent, dense, rev);
}
putchar(opt->line_termination);
}
But I think the other side needs to drop OUTPUT_FMT_RAW bit,
since around ll. 817 in combine-diff.c you show_raw_diff() if
OUTPUT_FMT_RAW bit is on regardless of the OUTPUT_FMT_PATCH bit.
> diff --git a/diff.c b/diff.c
> index 1db0285..6eb7db0 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -203,7 +203,7 @@ static void emit_rewrite_diff(const char
> static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
> {
> if (!DIFF_FILE_VALID(one)) {
> - mf->ptr = ""; /* does not matter */
> + mf->ptr = (char *)""; /* does not matter */
> mf->size = 0;
> return 0;
> }
> @@ -395,7 +395,7 @@ static void show_stats(struct diffstat_t
> }
>
> for (i = 0; i < data->nr; i++) {
> - char *prefix = "";
> + const char *prefix = "";
> char *name = data->files[i]->name;
> int added = data->files[i]->added;
> int deleted = data->files[i]->deleted;
> @@ -917,7 +917,7 @@ int diff_populate_filespec(struct diff_f
> err_empty:
> err = -1;
> empty:
> - s->data = "";
> + s->data = (char *)"";
> s->size = 0;
> return err;
> }
> @@ -1408,7 +1411,7 @@ int diff_setup_done(struct diff_options
> return 0;
> }
>
> -int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
> +static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
> {
> char c, *eq;
> int len;
> @@ -1720,16 +1722,12 @@ static void diff_flush_raw(struct diff_f
> free((void*)path_two);
> }
>
> -static void diff_flush_name(struct diff_filepair *p,
> - int inter_name_termination,
> - int line_termination)
> +static void diff_flush_name(struct diff_filepair *p, int line_termination)
> {
> char *path = p->two->path;
>
> if (line_termination)
> path = quote_one(p->two->path);
> - else
> - path = p->two->path;
> printf("%s%c", path, line_termination);
> if (p->two->path != path)
> free(path);
These are all good changes but I would have liked to see them as
a separate series.
> @@ -1371,23 +1371,26 @@ int diff_setup_done(struct diff_options
> (0 <= options->rename_limit && !options->detect_rename))
> return -1;
>
> + if (options->output_fmt & OUTPUT_FMT_NONE)
> + options->output_fmt = 0;
> +
> + if (options->output_fmt & (OUTPUT_FMT_NAME |
> + OUTPUT_FMT_CHECKDIFF |
> + OUTPUT_FMT_NONE))
> + options->output_fmt &= ~(OUTPUT_FMT_RAW |
> + OUTPUT_FMT_DIFFSTAT |
> + OUTPUT_FMT_SUMMARY |
> + OUTPUT_FMT_PATCH);
> +
Maybe doing the same for --name-status? I wonder if the --name,
--name-status and --check should be mutually exclusive. What
happens when you specify more than one of them?
> +/* Same as output_fmt = 0 but we know that -s flag was given
> + * and we should not give default value to output_fmt.
> + */
> +#define OUTPUT_FMT_NONE 0x0080
This is a very good comment to tell the reader what is going on.
I appreciate it.
> diff --git a/revision.c b/revision.c
> index b963f2a..4ad2272 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char
> if (revs->combine_merges) {
> revs->ignore_merges = 0;
> if (revs->dense_combined_merges &&
> - (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))
> - revs->diffopt.output_format = DIFF_FORMAT_PATCH;
> + !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))
> + revs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;
> }
> revs->diffopt.abbrev = revs->abbrev;
> diff_setup_done(&revs->diffopt);
This tells it to default to patch format unless we are asked to
do diffstat only, in which case we just show stat without patch.
The new logic seems to be fishy.
Whew, now it's quite a lot of changes. How to proceed from
here? My rather selfish preference is:
- "git-merge not add -p when asking for --summary --stat" is an
obvious fix that is independently applicable to "master", so
I took it already.
- could I ask you to redo a patch to do only the clean-up part
first, so that I can accept it for either "next" or "master".
- Then after I take the clean-up, could you rebase four
remainder patches ("Rework diff options" to "Add --patch
option for diff-*") on the result? The patches this round
are already split quite well in that the first one does the
enum to bit conversion and the latter three cleans things up
(all of which I like a lot). As Johannes suggested, it might
be easier to review if they reused the same preprocessor
symbols instead of renaming them. I'd take them for "next".
^ permalink raw reply
* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Junio C Hamano @ 2006-06-24 6:59 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git, Johannes.Schindelin
In-Reply-To: <11511257501323-git-send-email-martin@catalyst.net.nz>
Martin Langhoff <martin@catalyst.net.nz> writes:
> On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> It seems that git-cvsimport makes a temporary file of size 0, which cannot
>> get mmap()ed, because it has size 0.
>
> This switch to tmpnam() avoids creating the tmpfile in the first place and
> streamlines the code. This handling of tmpfiles is slightly safer, but there
> is an inherent race condition.
>
> ---
> NOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,
> if trivial.
Thanks both.
I'd take this to "next" after I hear from somebody, most likely
Johannes, who had trouble with the code earlier that the problem
is fixed.
^ permalink raw reply
* Re: x86 asm SHA1 (draft)
From: Junio C Hamano @ 2006-06-24 7:03 UTC (permalink / raw)
To: linux; +Cc: git
In-Reply-To: <20060624012202.4822.qmail@science.horizon.com>
linux@horizon.com writes:
> Well, I'm not sure it's worth this much trouble. Both of my PPC
> implementations are smaller and faster than the current one,
> so that's a pretty easy decision. The difference between them
> is 2-3%, which is, I think, not enough to be worth the maintenance
> burden of a run-time decision infrastructure. Just pick either one
> and call it a day.
>...
> Not that numbers are bad, but I think that until there's a real
> need for more than a single good-enough version per instruction set,
> this is excessive.
OK. I somehow got an impression that your two versions had
quite different performance characteristics on G4 and G5 and
there was a real choice. If they are between a few per-cent,
then I agree it is not worth doing at all.
^ permalink raw reply
* Re: [PATCH] rebase --merge: fix for rebasing more than 7 commits.
From: Junio C Hamano @ 2006-06-24 7:09 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> Junio C Hamano <junkio@cox.net> wrote:
>> Junio C Hamano <junkio@cox.net> writes:
>>
>> > * I wanted to raise my confidence level in the new rebase --merge
>> > code, so I did a little exercise which resulted in finding this
>> > buglet.
>> >...
>> > So the exercise went like this:
>> >...
>> > With this fix, the above works beautifully. I am reasonably
>> > happy with this shiny new toy. Good job, Eric! and thanks.
>
> :) Thanks for the extra QA and fix.
Another thing I noticed is while rebasing onto the mainline that
has accepted a few of the patches from the topic. The original
rebase with "git am -3" logic notices that the patch has already
been applied and drops that commit, which is rather nice, but
the new "rebase --merge" logic barfs when git-commit notices
there is nothing to commit. I think you could before calling
git-commit check if the git-merge-$strategy gave you the tree
identical to the HEAD tree, and simply skip it (maybe after
giving the user "patch already applied"message).
^ permalink raw reply
* From b65bc21e7d8dc8cafc70dfa6354cb66b8874b2d9 Mon Sep 17 00:00:00 2001, [PATCH] Makefile: add framework to verify and bench sha1 implementations.
From: Junio C Hamano, Junio C Hamano @ 2006-06-24 7:59 UTC (permalink / raw)
To: git; +Cc: linux
In-Reply-To: <7vfyhv11ej.fsf@assigned-by-dhcp.cox.net>
With this, you can say
MOZILLA_SHA1=YesPlease make check-sha1
to see how fast your favorite SHA-1 implementation is and if it
fails with small to huge inputs.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* As the maintainer, I do need people to see breakage before it
happens, without having access to all the platforms, hence
this.
Makefile | 6 ++++
test-sha1.c | 24 +++++++++++++++++
test-sha1.sh | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 113 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index e29e3fa..ea0044d 100644
--- a/Makefile
+++ b/Makefile
@@ -636,6 +636,12 @@ test-delta$X: test-delta.c diff-delta.o
test-dump-cache-tree$X: dump-cache-tree.o $(GITLIBS)
$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
+test-sha1$X: test-sha1.o $(GITLIBS)
+ $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
+
+check-sha1:: test-sha1$X
+ ./test-sha1.sh
+
check:
for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
diff --git a/test-sha1.c b/test-sha1.c
new file mode 100644
index 0000000..2efc315
--- /dev/null
+++ b/test-sha1.c
@@ -0,0 +1,24 @@
+#include "cache.h"
+
+int main(int ac, char **av)
+{
+ SHA_CTX ctx;
+ unsigned char sha1[20];
+
+ SHA1_Init(&ctx);
+
+ while (1) {
+ ssize_t sz;
+ char buffer[8192];
+ sz = xread(0, buffer, sizeof(buffer));
+ if (sz == 0)
+ break;
+ if (sz < 0)
+ die("test-sha1: %s", strerror(errno));
+ SHA1_Update(&ctx, buffer, sz);
+ }
+ SHA1_Final(sha1, &ctx);
+ puts(sha1_to_hex(sha1));
+ exit(0);
+}
+
diff --git a/test-sha1.sh b/test-sha1.sh
new file mode 100755
index 0000000..01bbb57
--- /dev/null
+++ b/test-sha1.sh
@@ -0,0 +1,83 @@
+#!/bin/sh
+
+dd if=/dev/zero bs=1048576 count=100 2>/dev/null |
+/usr/bin/time ./test-sha1 >/dev/null
+
+while read expect cnt pfx
+do
+ case "$expect" in '#'*) continue ;; esac
+ actual=`
+ {
+ test -z "$pfx" || echo "$pfx"
+ dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
+ tr '[\0]' '[g]'
+ } | ./test-sha1
+ `
+ if test "$expect" = "$actual"
+ then
+ echo "OK: $expect $cnt $pfx"
+ else
+ echo >&2 "OOPS: $cnt"
+ echo >&2 "expect: $expect"
+ echo >&2 "actual: $actual"
+ exit 1
+ fi
+done <<EOF
+da39a3ee5e6b4b0d3255bfef95601890afd80709 0
+3f786850e387550fdab836ed7e6dc881de23001b 0 a
+5277cbb45a15902137d332d97e89cf8136545485 0 ab
+03cfd743661f07975fa2f1220c5194cbaff48451 0 abc
+3330b4373640f9e4604991e73c7e86bfd8da2dc3 0 abcd
+ec11312386ad561674f724b8cca7cf1796e26d1d 0 abcde
+bdc37c074ec4ee6050d68bc133c6b912f36474df 0 abcdef
+69bca99b923859f2dc486b55b87f49689b7358c7 0 abcdefg
+e414af7161c9554089f4106d6f1797ef14a73666 0 abcdefgh
+0707f2970043f9f7c22029482db27733deaec029 0 abcdefghi
+a4dd8aa74a5636728fe52451636e2e17726033aa 1
+9986b45e2f4d7086372533bb6953a8652fa3644a 1 frotz
+23d8d4f788e8526b4877548a32577543cbaaf51f 10
+8cd23f822ab44c7f481b8c92d591f6d1fcad431c 10 frotz
+f3b5604a4e604899c1233edb3bf1cc0ede4d8c32 512
+b095bd837a371593048136e429e9ac4b476e1bb3 512 frotz
+08fa81d6190948de5ccca3966340cc48c10cceac 1200 xyzzy
+e33a291f42c30a159733dd98b8b3e4ff34158ca0 4090 4G
+#a3bf783bc20caa958f6cb24dd140a7b21984838d 9999 nitfol
+EOF
+
+exit
+
+# generating test vectors
+# inputs are number of megabytes followed by some random string to prefix.
+
+while read cnt pfx
+do
+ actual=`
+ {
+ test -z "$pfx" || echo "$pfx"
+ dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
+ tr '[\0]' '[g]'
+ } | sha1sum |
+ sed -e 's/ .*//'
+ `
+ echo "$actual $cnt $pfx"
+done <<EOF
+0
+0 a
+0 ab
+0 abc
+0 abcd
+0 abcde
+0 abcdef
+0 abcdefg
+0 abcdefgh
+0 abcdefghi
+1
+1 frotz
+10
+10 frotz
+512
+512 frotz
+1200 xyzzy
+4090 4G
+9999 nitfol
+EOF
--
1.4.1.rc1.g0fca
^ permalink raw reply related
* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Junio C Hamano @ 2006-06-24 8:33 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <7vr71f5kzs.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Just to let you know, I already have v3 in my tree which is
> merged in "pu". With the two fixes I sent you earlier I think
> it is in a good shape to be cooked in "next".
>
> I do not think I'd have trouble applying this new series (I
> would probably start from "master" to apply it and perhaps merge
> or --squash merge it onto pb/gitpm topic branch that has v3 with
> the two fixes I sent you separately) but we will see soon
> enough.
EEEEEEEEEK.
git-fmt-merge-msg failed and git-pull did not notice it and went
ahead to call git-merge with an empty log message. This screwed
up my tree rather big way.
The reason it failed? Well, it could not find Git.pm because
the changes to fmt-merge-msg was done for distros not for people
who install under their home directories.
Now, I am quite unhappy about the situation (and it is not your
fault). "git pull" is something almost everybody uses, and
having the series means they would need to make sure whereever
Git.pm is installed is on their PERL5LIB as things currently
stand.
In the case of git-merge-recursive.py, Fredrik does
sys.path.append('''@@GIT_PYTHON_PATH@@''')
and while running test this invalid path is overridden by having
PYTHONPATH environment variable. Since it is "append", the test
picks up the freshly built version, not the version from the
previous install (i.e. PYTHONPATH environment variable wins).
So you would probably want to have something like
use lib '@@GIT_PERL_PATH@@';
inside git-fmt-merge-msg.perl, which will be turned into
use lib '/home/junio/some/where/you/install/pm';
in git-fmt-merge-msg and things might start working.
... gitster mumbles something in his mouth, and in a puff of
smoke Merlyn appears and solves all our Perl problems ;-) ...
^ permalink raw reply
* Re: [PATCH 07/12] Git.pm: Better error handling
From: Junio C Hamano @ 2006-06-24 8:37 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20060624023442.32751.28342.stgit@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> +int
> +error_xs(const char *err, va_list params)
> +{
You said in git-compat-util.h that set_error_routine takes a
function that returns void, so this gives unnecessary type
clash.
--------------------------------
In file included from /usr/lib/perl/5.8/CORE/perl.h:756,
from Git.xs:15:
/usr/lib/perl/5.8/CORE/embed.h:4193:1: warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition
Git.xs: In function 'boot_Git':
Git.xs:57: warning: passing argument 1 of 'set_error_routine' from incompatible pointer type
Git.xs:58: warning: passing argument 1 of 'set_die_routine' makes qualified function pointer from unqualified
--------------------------------
Other troubles I saw with the v4 series while compiling:
--------------------------------
usage.c:35: warning: initialization makes qualified function pointer from unqualified
usage.c:36: warning: initialization makes qualified function pointer from unqualified
I'd fix it with this
diff --git a/usage.c b/usage.c
index b781b00..52c2e96 100644
--- a/usage.c
+++ b/usage.c
@@ -12,19 +12,19 @@ static void report(const char *prefix, c
fputs("\n", stderr);
}
-void usage_builtin(const char *err)
+static NORETURN void usage_builtin(const char *err)
{
fprintf(stderr, "usage: %s\n", err);
exit(129);
}
-void die_builtin(const char *err, va_list params)
+static NORETURN void die_builtin(const char *err, va_list params)
{
report("fatal: ", err, params);
exit(128);
}
-void error_builtin(const char *err, va_list params)
+static void error_builtin(const char *err, va_list params)
{
report("error: ", err, params);
}
--------------------------------
(cd perl && /usr/bin/perl Makefile.PL \
PREFIX="/home/junio/git-test" \
DEFINE="-O2 -Wall -Wdeclaration-after-statement
-g -DSHA1_HEADER='<openssl/sha.h>'
-DGIT_VERSION=\\\"1.4.1.rc1.gab0df\\\"" \
LIBS="libgit.a xdiff/lib.a -lz -lcrypto")
Unrecognized argument in LIBS ignored: 'libgit.a'
Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
Do you need to pass LIBS, and if so maybe this is not a way
Makefile.PL expects it to be passed perhaps?
--------------------------------
Makefile out-of-date with respect to Makefile.PL
Cleaning current config before rebuilding Makefile...
make -f Makefile.old clean > /dev/null 2>&1
/usr/bin/perl Makefile.PL "PREFIX=/home/junio/git-test" "DEFINE=-O2 -Wall -Wdeclaration-after-statement -g -DSHA1_HEADER='<openssl/sha.h>' -DGIT_VERSION=\"1.4.1.rc1.gab0df\"" "LIBS=libgit.a xdiff/lib.a -lz -lcrypto"
Unrecognized argument in LIBS ignored: 'libgit.a'
Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
Writing Makefile for Git
==> Your Makefile has been rebuilt. <==
==> Please rerun the make command. <==
false
make[1]: *** [Makefile] Error 1
--------------------------------
The latter is what Perl's build mechanism does so it is not
strictly your fault, but it nevertheless is irritating that we
have to say make clean twice.
^ permalink raw reply related
* Re: A series file for git?
From: Junio C Hamano @ 2006-06-24 9:01 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <m1odwkyuf5.fsf_-_@ebiederm.dsl.xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> I was using:
> git-rev-list $revargs | tac > list
> for sha1 in $(cat list); do git-cherry-pick -r $sha1 ; done
>...
> - Keeping patches in git and just remembering their sha1 is nice
> but it has the downside that it can be really easy to loose
> the sha1, and thus the patch. So some sort of history mechanism
> so you can get back to where you were would be nice.
Actually, the $revargs above is composed of your branch names
(e.g. "^master this-topic that-topic"), so as long as you do not
lose these branches they are protected.
>
> - This is a similar technique to topic branches. However in some
> of the more interesting cases a topic branch can't be used because
> you have a whole series of little changes, that allow depend on
> each other. So topic branches need not apply.
Sorry I fail to see why. A series of little changes that depend
on each other would be a string of pearls on a topic branch in
the simplest case, or a handful of topic branches that
occasionally merge with each other if you want to get fancier.
Cherry-picking from a DAG of the latter kind with your rev-list
piped to tac is no different than cherry-picking from a simple
straight line of the former kind, isn't it?
> - One of the places where we currently uses series files
> (patch-scripts && quilt), and heavy cherry picking is for patch
> aging. That is letting patches sit in a testing branch for
> a while so people have a chance to test and look at them.
I agree that patch aging and updating does not mesh well with
how git inherently works, as git regards commits immutable
objects. Even then, I use my "pu" branch (and topics that
hasn't merged to "next" but in "pu") pretty much as patch aging
area and I regularly do "git commit --amend" to update them.
This however is cumbersome with core git tools alone, and I
suspect is better done with StGIT.
> If we create a meta data branch with just the series file
> we can remove the risk of loosing things, as we always
> have a path back to the old history if we want it.
I am not sure about that. What does the series file contain,
and what other things the meta data branch contain? If you are
listing commit SHA1 in the series file, you _do_ have the risk
of losing things -- git-fsck-objects needs to look at such blob
object and consider that as the root of reachability chain; to
me that seems too specialized hack.
I have a feeling that I really should study how well StGIT does
things before talking about this further. It may suit your
needs perfectly. What do you feel is lacking in StGIT that you
need?
^ permalink raw reply
* [PATCH] git-repack -- respect -q and be quiet
From: Martin Langhoff @ 2006-06-24 9:41 UTC (permalink / raw)
To: git, junkio; +Cc: Martin Langhoff
git-repack was passing the -q along to pack-objects but ignoring it
itself. Correct the oversight.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-repack.sh | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index 4fb3f26..eb75c8c 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -49,8 +49,9 @@ name=$(git-rev-list --objects --all $rev
if [ -z "$name" ]; then
echo Nothing new to pack.
else
- echo "Pack pack-$name created."
-
+ if test "$quiet" != '-q'; then
+ echo "Pack pack-$name created."
+ fi
mkdir -p "$PACKDIR" || exit
mv .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
--
1.4.1.rc1.g59c8
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox