Quick merge status updates.

I'm planning to do 1.4.1 soonish from what's currently in "next" (fixes already in "master" plus format-patch updates and some more tests). I am waiting for a confirmation that Johannes's cvsimport fix posted earlier on the list does work, or maybe Martin would come up with an alternative. I think then we are good for a release.

Immediately after 1.4.1 happens, I would like to pull in "Git.xs/Git.pm" series by Pasky into "next". After that settles I'd pull in the diff options rewrite by Timo.

For some time, "pu" was left in the state that does not to pass the testsuite, but I've fixed what's minimally needed (the breakage was mostly from the diff options rewrite). People who regularly follow "next" on platforms other than i386 or x86-64 Linux might want to try out tonight's "pu" to make sure "Git.xs/Git.pm" series works on their box before it hits "next". Breakage there would stop your "git pull" working, so this is somewhat important.

Thanks.

On Tue, 2006-06-27 at 17:23 -0700, Junio C Hamano wrote:

PowerPC 32-bit, G3, Fedora Core 5, gcc 4.1.1, perl 5.8.8, pu branch (f5d33e0b4eaa4083b455118a7be473defb61f137)

Warnings:

quote.c: In function 'sq_quote_buf':
quote.c:34: warning: value computed is not used
quote.c:37: warning: value computed is not used
combine-diff.c: In function 'diff_tree_combined':
combine-diff.c:844: warning: assignment makes integer from pointer
without a cast
In file included
from /usr/lib/perl5/5.8.8/ppc-linux-thread-multi/CORE/perl.h:756,
                 from Git.xs:15:
/usr/lib/perl5/5.8.8/ppc-linux-thread-multi/CORE/embed.h:4195:1:
warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition

I'm very concerned about the combine-diff.c warning. The warning seems to be legitimate and I don't see an obvious fix. The offending line comes from 3969cf7db1a13a78f3b7a36d8c1084bbe0a53459 ("Fix some more diff options changes"):

show_log_first = rev->loginfo;

The testsuite passes, but "git-pull" is indeed broken:

$ git-pull
Can't locate Git.pm in @INC (@INC
contains: /usr/lib/perl5/site_perl/5.8.8/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.7/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.6/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.5/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.4/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.3/ppc-linux-thread-multi /usr/lib/perl5/site_perl/5.8.8 /usr/lib/perl5/site_perl/5.8.7 /usr/lib/perl5/site_perl/5.8.6 /usr/lib/perl5/site_perl/5.8.5 /usr/lib/perl5/site_perl/5.8.4 /usr/lib/perl5/site_perl/5.8.3 /usr/lib/perl5/site_perl /usr/lib/perl5/vendor_perl/5.8.8/ppc-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.7/ppc-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.6/ppc-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.5/ppc-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.4/ppc-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.3/ppc-linux-thread-multi /usr/lib/perl5/vend
 or_perl/5.8.8 /usr/lib/perl5/vendor_perl/5.8.7 /usr/lib/perl5/vendor_perl/5.8.6 /usr/lib/perl5/vendor_perl/5.8.5 /usr/lib/perl5/vendor_perl/5.8.4 /usr/lib/perl5/vendor_perl/5.8.3 /usr/lib/perl5/vendor_perl /usr/lib/perl5/5.8.8/ppc-linux-thread-multi /usr/lib/perl5/5.8.8 .) at /home/proski/bin/git-fmt-merge-msg line 10.
BEGIN failed--compilation aborted at /home/proski/bin/git-fmt-merge-msg
line 10.

Git.pm is installed into /home/proski/lib/perl5/site_perl/5.8.8/ppc-linux-thread-multi/

Speaking of x86_64 (also Fedora Core 5, gcc 4.1.1, perl 5.8.8), git doesn't even build:

In file included
from /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE/perl.h:756,
                 from Git.xs:15:
/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE/embed.h:4195:1:
warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition
Running Mkbootstrap for Git ()
chmod 644 Git.bs
rm -f blib/arch/auto/Git/Git.so
gcc  -shared Git.o  -o blib/arch/auto/Git/Git.so ../libgit.a    \
   -lz -lcrypto         \
  
/usr/bin/ld: ../libgit.a(exec_cmd.o): relocation R_X86_64_32 against `a
local symbol' can not be used when making a shared object; recompile
with -fPIC
../libgit.a: could not read symbols: Bad value
collect2: ld returned 1 exit status
make[1]: *** [blib/arch/auto/Git/Git.so] Error 1
make[1]: Leaving directory `/usr/local/src/git/perl'
make: *** [all] Error 2

The build succeeds if I use following: make CFLAGS='-g -O2 -Wall -fPIC'

The testsuite passes. git-pull is also broken with a similar error message. Git.pm is installed into /home/proski/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi/

Pavel Roskin <proski@gnu.org> writes:

Thanks.

Thanks.  I think these are dealt with Jeff King's patches.

Thanks.  The fix is actually simple (two paragraphs below).

I've seen this but haven't looked into it.  Pasky?

This is saying "If rev->loginfo is pointing at some string, we are going to show the log message first and then output from the diff machinery (otherwise rev->loginfo has NULL)". I'll rephrase it to read as:

	show_log_first = !!rev->loginfo;

 ndor_perl/5.8.8 /usr/lib/perl5/vendor_perl/5.8.7 /usr/lib/perl5/vendor_perl/5.8.6 /usr/lib/perl5/vendor_perl/5.8.5 /usr/lib/perl5/vendor_perl/5.8.4 /usr/lib/perl5/vendor_perl/5.8.3 /usr/lib/perl5/vendor_perl /usr/lib/perl5/5.8.8/ppc-linux-thread-multi /usr/lib/perl5/5.8.8 .) at /home/proski/bin/git-fmt-merge-msg line 10.

The same problem with the last paragraph.

	$ git grep -A1 -e 'Define USE_PIC" pu:Makefile

in other words:

	USE_PIC=YesPlease make

I suspect this is either FC perl-dev package is broken (I doubt it) or your installation procedure is pecurilar (much more likely). I pass the same set of prefix, bindir and friends to "make" and "make install" and do not see the problem.

On Wed, 2006-06-28 at 01:49 -0700, Junio C Hamano wrote:

I think my Perl 5.8.8 is "too new".  "man perlfunc" says about "use":

"Imports some semantics into the current package from the named module, generally by aliasing certain subroutine or variable names into your package. It is exactly equivalent to

      BEGIN { require Module; import Module LIST; }

except that Module must be a bareword."

I think the BEGIN block has priority over other statements. My solution was to put the @INC change in the BEGIN block as well.

This patch is working for me:

Pavel Roskin <proski@gnu.org> writes:

Actually I do use 5.8.8 and everything you quoted makes perfect sense, -- in fact now I do not know *why* it did _not_ break for me without the BEGIN {} block, especially I do not have any PERL* environment variable to point at anywhere under $HOME.

Thanks for the fix.

Dear diary, on Wed, Jun 28, 2006 at 12:05:03PM CEST, I got a letter where Pavel Roskin <proski@gnu.org> said that...

I feel that it is time for another stupid question of mine - why can't you just use lib?

	use lib ('@@INSTLIBDIR@@');

Looks a lot better than some @INC unshifting, and it should be equivalent.

Let's pour in to the confusion:

The unshifting was introduced w/o BEGIN{} in

	From: Junio C Hamano <junkio@cox.net>
	Subject: Re: [PATCH 01/12] Introduce Git.pm (v4)
	Date:   Sat, 24 Jun 2006 04:57:31 -0700

but that patch is not in pu anymore while the description of the new patch implicitly refers to it, which made it all a bit confusing.

So the purpose of the original patch was to make it play nicely with $PERLLIB, but unshifting helps nothing, since:

	There's default @INC
	Perl spots PERLLIB and unshifts @INC
	We then unshift @INC too, taking precedence

So didn't the original patch rather want to do push?

	$ PERL5LIB=perl perl -le "BEGIN { unshift @INC, '/home/xpasky/lib/perl5/site_perl/5.8.8/i686-linux'; } use Git; print Git::hash_object('blob','Makefile');"
	17842a3657ae8e5b4fd3ddfeb69268a4b94cb97a
	$ PERL5LIB=perl perl -le "use Git; print Git::hash_object('blob','Makefile');"
	syntax error at perl/Git.pm line 44, near "h>"

(after inserting random junk to perl/Git.pm)

Petr Baudis <pasky@suse.cz> writes:

I tried this:

 0. check out the branch that has the Perly git ("pu").  build
    and install normally to have a perfectly working version
    accessible on your usual $PATH.

 1. apply the patch [1] below to make it use "use lib" instead of
    "unshift".

 2. break perl/Git.pm being built to pretend we introduced a bug
    in the work in progress by applying the patch [2] below.

 3. without installing the broken Git.pm, run "make test", and
    see a test that uses "git pull" and needs to create a true
    merge succeed.  It tells me that everything including
    perl/Git.pm is GOOD, and I'd find the breakage only after
    installing and running the test again.

If you make the freshly built script to say "use lib" to use Git.pm and its friends from the installed location, it defeats the attempt by test-lib.sh to override it to test what we freshly built -- the use of "unshift @INC" is to work that around and is parallel to the way Python stuff works around the same problem.

[Footnotes] *1* "use lib" patch.

Dear diary, on Sun, Jul 02, 2006 at 11:33:47PM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

So, just to clarify and make sure we understand each other perfectly, you claim that when skipping (1), (3) _does_ FAIL for you? Because it really doesn't for me and I can't see how could it ever fail without installing the broken version first.

Petr Baudis <pasky@suse.cz> writes:

Gaah.  You are right.

PERL5LIB does not seem to just do a push (and that was I thought why unshift was a way to defeat it) but do something more evil.

With this:

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

Yuck; that means we would need to have something evil like this.

-- >8 -- Perly Git: make sure we do test the freshly built one.

We could BEGIN { push @INC, '@@INSTLIBDIR@@'; } but that is not a good idea for normal execution. The would prevent a workaround for a user who is trying to override an old, faulty Git.pm installed on the system path with a newer version installed under $HOME/.

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

---

On Sun, 2 Jul 2006, Junio C Hamano wrote:

That's just disgusting.

How about a _much_ simpler approach.

Just make it do

	BEGIN { push ENV{'GIT_PERL_EXEC_DIR'}; }

And then simply _require_ that the setup code sets up GIT_PERL_EXEC_DIR.

Which is usually simple enough to do. For git.c, something like this will do it, other places you can reach things through can have something similar.

		Linus

---

Linus Torvalds <torvalds@osdl.org> writes:

No, we are not mucking with /usr/bin but are talking about a directory for platform specific Perl libraries, which we need to ask Perl at build-time.

And the standard way to say that is to do:

	use lib '@@INSTLIBDIR@@';

but that always prepends the directory to the search path, and the ugly stuff is to allow test scripts to override it when needed.

On Sun, 2006-07-02 at 22:49 +0200, Petr Baudis wrote:

I honestly have no idea. I think whatever gets you closer to the satisfaction of the "three conditions" should be used.

Hi,

On Tue, 27 Jun 2006, Junio C Hamano wrote:

Earlier, you said that this is primarily for gitweb, not so much for the core of git. But...

May I respectfully offer my objection? This is the _heart_ of git. You do not want to muck around with this.

I am not opposed to a sane interface between Perl and git, but please please PLEASE do not make such an important part of git dependent on Git.pm. (Somehow I have the impressions there are echoes here.)

Ciao, Dscho

Hi,

on my iBook, make in pu outputs:

GIT_VERSION = 1.4.1.rc1.gf5d3
    * new build flags or prefix
(cd perl && /usr/bin/perl Makefile.PL \
     PREFIX='/Users/gene099' \
     DEFINE=' -I/sw/include -DSHA1_HEADER='\''<openssl/sha.h>'\'' 
-DNO_STRCASESTR -DNO_STRLCPY -DGIT_VERSION='\''"1.4.1.rc1.gf5d3"'\''' \
     LIBS=' -L/sw/lib -lz  -liconv  -lcrypto -lssl')
Can't locate Devel/PPPort.pm in @INC (@INC contains: 
/System/Library/Perl/darwin /System/Library/Perl /Library/Perl/darwin 
/Library/Perl /Library/Perl /Network/Library/Perl/darwin 
/Network/Library/Perl /Network/Library/Perl .) at Makefile.PL line 29.
BEGIN failed--compilation aborted at Makefile.PL line 29.
make: *** [perl/Makefile] Error 2

FWIW all Perl scripts in git, except git-send-email, work here.

Ciao, Dscho

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

I suspect git-fmt-merge-msg doesn't, and perhaps git-mv.

Hi,

On Wed, 28 Jun 2006, Junio C Hamano wrote:

Last time I checked, git-mv worked (couple of weeks ago). And if I am not mistaken, git-fmt-merge-msg is needed when git-pull does not fast forward? I definitely successfully merged on my iBook, IIRC last time was last week.

BTW: Devel/PPPort.pm is introduced by this line in perl/Git.xs:

Ciao, Dscho

  Hi,

Dear diary, on Wed, Jun 28, 2006 at 09:39:46AM CEST, I got a letter where Johannes Schindelin <Johannes.Schindelin@gmx.de> said that...

  ow, Devel::PPPort might not be around all the way back to 5.6.0. What
is your Perl version, BTW?

->8-

This makes us not include ppport.h which seems not to give us anything real anyway; it is useful for checking for portability warts but since Devel::PPPort is a portability wart itself, we shouldn't require it for build. You can check for portability problems by calling make check in perl/.

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

 perl/Git.xs      |    2 --
 perl/Makefile.PL |   10 +++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

Hi,

On Sun, 2 Jul 2006, Petr Baudis wrote:

This is perl, v5.6.0 built for darwin

Why do people introduce a "portability enhancer" like that? This is soo dumb.

Well, your patch helped. Now the error is somewhere else:

mkdir blib
mkdir blib/lib
mkdir blib/arch
mkdir blib/arch/auto
mkdir blib/arch/auto/Git
mkdir blib/lib/auto
mkdir blib/lib/auto/Git
mkdir blib/man3
cp private-Error.pm blib/lib/Error.pm
cp Git.pm blib/lib/Git.pm
/usr/bin/perl -I/System/Library/Perl/darwin -I/System/Library/Perl 
/System/Library/Perl/ExtUtils/xsubpp  -typemap 
/System/Library/Perl/ExtUtils/typemap Git.xs > Git.xsc && mv Git.xsc Git.c
Error: 'const char *' not in typemap in Git.xs, line 69
Error: 'const char *' not in typemap in Git.xs, line 79
make: *** [Git.c] Error 1

It seems like my typemap starts like this:

-- snip -- # $Header: /cvs/Darwin/src/live/perl/perl/lib/ExtUtils/typemap,v 1.2 2002/03/14 08:58:22 zarzycki Exp $ # basic C types int T_IV unsigned T_UV unsigned int T_UV long T_IV unsigned long T_UV short T_IV unsigned short T_UV char T_CHAR unsigned char T_U_CHAR char * T_PV unsigned char * T_PV caddr_t T_PV wchar_t * T_PV wchar_t T_IV bool_t T_IV size_t T_IV ssize_t T_IV time_t T_NV -- snap --

So, no "const char *". See next mail for a minimal patch. The warning (IIRC that was mentioned already on the list) still persists:

cc -c -I. -I.. -g -pipe -pipe -fno-common -no-cpp-precomp -flat_namespace 
-DHAS_TELLDIR_PROTOTYPE -fno-strict-aliasing -Os     -DVERSION=\"0.01\" 
-DXS_VERSION=\"0.01\"  -I/System/Library/Perl/darwin/CORE -I/sw/include 
-DSHA1_HEADER='<openssl/sha.h>' -DNO_STRCASESTR -DNO_STRLCPY 
-DGIT_VERSION='"1.4.1.g3b26"' Git.c
In file included from /System/Library/Perl/darwin/CORE/perl.h:500,
                 from Git.xs:15:
/System/Library/Perl/darwin/CORE/embed.h:156:1: warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition

Ciao, Dscho

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

I think that statement is a bit too harsh.

My understanding is that it's more for use by developers working with later version to produce portability headers, so used that way it is very sane. From its manual page:

       How to use ppport.h

       Don't direct the users of your module to download "Devel::PPPort".
       They are most probably no XS writers. Also, don't make ppport.h
       optional. Rather, just take the most recent copy of ppport.h that you
       can find (e.g. by generating it with the latest "Devel::PPPort" release
       from CPAN), copy it into your project, adjust your project to use it,
       and distribute the header along with your module.

But in the case of source distribution, and if the source distribution wants to be compatible with older versions, the above advice does not apply.

An alternative would be to carry our own typemap but I think your fix is less intrusive and fine. Pasky?

I see the same here.

Dear diary, on Sun, Jul 02, 2006 at 09:05:33PM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Yes, it should be fine.

-8<-

Back in the old days, we called Git's die() from the .xs code, but we had to hijack Perl's die() for that. Now we don't call Git's die() so no need to do the hijacking and it silences a compiler warning.

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

 perl/Git.xs |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)