[PATCH] perl bindings fix compilation errors

[PATCH] perl bindings fix compilation errors

[Andy Whitcroft]

perl bindings: fix compilation errors

With the introduction of Makefile.PL to the perl bindings we no
longer seem to pass in either the definition of SHA1_HEADER or
GIT_VERSION.  It seems we no longer pass over the BASIC_FLAGS into
the compilation.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/Makefile b/Makefile
index d705e06..d32f4c4 100644
--- a/Makefile
+++ b/Makefile
@@ -603,7 +603,7 @@ all: $(ALL_PROGRAMS) $(BUILT_INS) git$X 
 	git-merge-recur$X
 
 all: perl/Makefile
-	$(MAKE) -C perl
+	$(MAKE) -C perl GIT_VERSION='$(GIT_VERSION)' BASIC_CFLAGS="$(BASIC_CFLAGS)"
 	$(MAKE) -C templates
 
 strip: $(PROGRAMS) git$X
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index de73235..00fc779 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -5,6 +5,8 @@ sub MY::postamble {
 instlibdir:
 	@echo '$(INSTALLSITELIB)'
 
+CCFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' $(BASIC_CFLAGS)
+
 MAKE_FRAG
 }
 

Re: [PATCH] perl bindings fix compilation errors

[Johannes Schindelin]

Hi,

On Mon, 25 Sep 2006, Andy Whitcroft wrote:

> With the introduction of Makefile.PL to the perl bindings we no
> longer seem to pass in either the definition of SHA1_HEADER or
> GIT_VERSION.  It seems we no longer pass over the BASIC_FLAGS into
> the compilation.

You probably got bitten by the fact that earlier runs left Git.c in the 
perl/ directory. Go to perl/, "make distclean", and make git again.

Hth,
Dscho

Re: [PATCH] perl bindings fix compilation errors

[Andy Whitcroft]

Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 25 Sep 2006, Andy Whitcroft wrote:
> 
>> With the introduction of Makefile.PL to the perl bindings we no
>> longer seem to pass in either the definition of SHA1_HEADER or
>> GIT_VERSION.  It seems we no longer pass over the BASIC_FLAGS into
>> the compilation.
> 
> You probably got bitten by the fact that earlier runs left Git.c in the 
> perl/ directory. Go to perl/, "make distclean", and make git again.

Hmmm that sucks.  Yes, I had to make distclean more than once, but it
seems to fix things.

I am somewhat unhappy that a make clean at the top level and remake was
not sufficient to get a working tree.

-apw

Re: [PATCH] perl bindings fix compilation errors

[Johannes Schindelin]

Hi,

On Mon, 25 Sep 2006, Andy Whitcroft wrote:

> Johannes Schindelin wrote:
> > Hi,
> > 
> > On Mon, 25 Sep 2006, Andy Whitcroft wrote:
> > 
> >> With the introduction of Makefile.PL to the perl bindings we no
> >> longer seem to pass in either the definition of SHA1_HEADER or
> >> GIT_VERSION.  It seems we no longer pass over the BASIC_FLAGS into
> >> the compilation.
> > 
> > You probably got bitten by the fact that earlier runs left Git.c in the 
> > perl/ directory. Go to perl/, "make distclean", and make git again.
> 
> Hmmm that sucks.  Yes, I had to make distclean more than once, but it
> seems to fix things.
> 
> I am somewhat unhappy that a make clean at the top level and remake was
> not sufficient to get a working tree.

Yes. I almost submitted a patch which checks for perl/Git.c, and makes 
distclean in perl/ if exists.

Ciao,
Dscho

Re: [PATCH] perl bindings fix compilation errors

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> I am somewhat unhappy that a make clean at the top level and remake was
> not sufficient to get a working tree.

Sympathetic I am (since I was bitten by it too), but I am not
sure what the right thing to do.

It is certainly incorrect to say that it is a driver error to
git-pull (or git-checkout to switch branches) without running
"make clean" first to get rid of perl/Git.c while your Makefile
still knows it is a generated file.  Our Makefile should be more
helpful.

But the question is Makefile in which branch?

Ideally, if Makefile of each and every branch head currently
alive knew that perl/Git.c is problematic and removed it, then
we would not suffer from this problem.  But it's obviously not
practical.

Re: [PATCH] perl bindings fix compilation errors

[Johannes Schindelin]

Hi,

On Mon, 25 Sep 2006, Junio C Hamano wrote:

> It is certainly incorrect to say that it is a driver error to
> git-pull (or git-checkout to switch branches) without running
> "make clean" first to get rid of perl/Git.c while your Makefile
> still knows it is a generated file.  Our Makefile should be more
> helpful.

How about this:

diff --git a/Makefile b/Makefile
index d3310ae..a722c5a 100644
--- a/Makefile
+++ b/Makefile
@@ -612,7 +612,14 @@ ### Build rules
 all: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk gitweb/gitweb.cgi \
 	git-merge-recur$X
 
-all: perl/Makefile
+# work around for left-over files
+.PHONY: perl/check-left-over
+
+perl/check-left-over:
+	(test -f perl/Git.c && test ! -f perl/Git.xs \
+	 && rm perl/{Git.c,Git.bs,Makefile}) || true
+
+all: perl/check-left-over perl/Makefile
 	$(MAKE) -C perl
 	$(MAKE) -C templates
 

Re: [PATCH] perl bindings fix compilation errors

[Junio C Hamano]

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

> Hi,
>
> On Mon, 25 Sep 2006, Junio C Hamano wrote:
>
>> It is certainly incorrect to say that it is a driver error to
>> git-pull (or git-checkout to switch branches) without running
>> "make clean" first to get rid of perl/Git.c while your Makefile
>> still knows it is a generated file.  Our Makefile should be more
>> helpful.
>
> How about this:

The problem is on which branch we would apply this and how long
we will carry it.  I suspect you would have to have it forever
and everywhere.  After all your bisect can hit a version with
Git.xs and then next build suggested by bisect may be a version
without it.  And the user is doing that bisect long after we
release version 47.

The worst part is that the version bisect suggests that does not
have Git.xs may be a version in the past -- which we obviously
cannot apply your patch to.

Re: [PATCH] perl bindings fix compilation errors

[Johannes Schindelin]

Hi,

On Tue, 26 Sep 2006, Junio C Hamano wrote:

> The worst part is that the version bisect suggests that does not
> have Git.xs may be a version in the past -- which we obviously
> cannot apply your patch to.

Okay, just forget about my patch.

Ciao,
Dscho