Re: [PATCH] [RFC] Introduce Git.pm

[Junio C Hamano <junkio@cox.net>]

Petr Baudis <pasky@suse.cz> writes:

> 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. And things will get much more interesting
> when we get to XS-izing libgit.

We would probably need some moderate to major surgery before
doing that depending on which parts.  Worrisome are object layer
that holds onto parsed objects and keeps flag bits from the
previous runs, and lockfile that are cleaned up via atexit() --
there may be others.

> @@ -618,6 +622,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
>  	@FLAGS='$(TRACK_CFLAGS)'; \
>  	    if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
>  		echo 1>&2 "    * new build flags or prefix"; \
> +		echo 1>&2 "    * $$FLAGS != `cat GIT-CFLAGS 2>/dev/null`"; \
>  		echo "$$FLAGS" >GIT-CFLAGS; \
>              fi
>  

Offtopic; do people mind if we drop $GIT_VERSION from
TRACK_CFLAGS?  Every time I switch branches or make a new commit
it ends up rebuilding everything needlessly.

>  sub current_branch {
> -	my ($bra) = qx{git-symbolic-ref HEAD};
> +	my ($bra) = $repo->generic_oneline('symbolic-ref', 'HEAD');
>  	chomp($bra);
>  	$bra =~ s|^refs/heads/||;
>  	if ($bra ne 'master') {

Your *_oneline chomps so chomp($bra) is not needed anymore, I think.

In any case, this is an excellent move to clean things up.
Various implementations of qx{} will be all in one place so
ActiveState folks would have easier time adjusting things to
their environment.

> +require Exporter;

Not "use"?

> +# Methods which can be called as standalone functions as well:
> +@EXPORT_OK = qw(generic generic_oneval generic_vocal);

I am not sure generic_xxx is a good name.  Perhaps command_xxx?

Also as you say in the comment, "vocal" sounds a bit funny.
I wonder if we can use wantarray to detect the case where the
caller wants _no_ value, and do the "not piping" optimization in
that case.