Compiling git with makepp patch

Compiling git with makepp patch

[Daniel Pfeiffer]

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

  Hi,

getting ready for the release of makepp version 2.0, I'm testing it for 
building various popular open source software.

Git has been giving our promise of Gnu make compatibility a hard time.  I have 
just checked in a series of small fixes, many which were needed to compile 
Git.  This includes things like:

    * accepting an action-prefix of +
    * implementing the cosmetic --no-print-directory directory option, which
      in your usage is essential
    * smarter MAKEFLAGS handling, because you unset it several times, but we
      have more options, some must reach the submake
    * allow special variables like $@ outside of rules — this used to be an error

There are however two things which I can hardly hope to fix:

GIT-VERSION-FILE: FORCE
      @$(SHELL_PATH) ./GIT-VERSION-GEN
-include GIT-VERSION-FILE
.PHONY: FORCE

I don't know why you depend on a phony that has no rule — I also had to make 
that possible.  The file needs to be built immediately so that it can be 
included, before reading the rest of the makefile.  But the dependency is only 
known to be phony after running the rule.  Here you have a hen-egg problem, 
where I have no clue how Gnu make can cope (this is the one case where it 
requires .PHONY).  Anyway, makepp needs the phony declaration before.

The other thing caused me quite a headache before I understood:

PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'

I suppose you added the comment for Emacs' syntax highlighting, to have an 
even number of unescaped quotes.  The problem is makepp parses this line just 
like Emacs, so it doesn't find the comment, adding in the #' at the point of 
use, which completely screws the sed command.  (You might want to apply my fix 
to a few other makefiles, which have SQ variables, albeit without the syntax 
highlighting workaround, so they are only visually defect.)

coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                     http://lernu.net   /http://ikurso.net


[-- Attachment #2: git-makepp.patch --]
[-- Type: text/x-diff, Size: 2254 bytes --]

diff -wrup git-2010-08-21/git-gui/Makefile git-2010-08-21a/git-gui/Makefile
--- git-2010-08-21/git-gui/Makefile	2010-08-20 21:55:41.000000000 +0200
+++ git-2010-08-21a/git-gui/Makefile	2010-08-21 20:59:10.272785510 +0200
@@ -7,6 +7,7 @@ all::
 # TCL_PATH must be vaild for this to work.
 #
 
+.PHONY: FORCE
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
@@ -340,4 +341,3 @@ ifdef GITGUI_WINDOWS_WRAPPER
 endif
 
 .PHONY: all install uninstall dist-version clean
-.PHONY: FORCE
diff -wrup git-2010-08-21/gitweb/Makefile git-2010-08-21a/gitweb/Makefile
--- git-2010-08-21/gitweb/Makefile	2010-08-20 21:55:41.000000000 +0200
+++ git-2010-08-21a/gitweb/Makefile	2010-08-21 21:09:47.499919056 +0200
@@ -52,12 +52,15 @@ SHELL_PATH ?= $(SHELL)
 PERL_PATH  ?= /usr/bin/perl
 
 # Shell quote;
-bindir_SQ = $(subst ','\'',$(bindir))#'
-gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
-gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
-PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
-DESTDIR_SQ    = $(subst ','\'',$(DESTDIR))#'
+Q='
+# ' This comment is only to appease IDEs like Emacs.
+# The comment is on a new line, else makepp would see it as a quoted hash.
+bindir_SQ = $(subst $Q,'\'',$(bindir))
+gitwebdir_SQ = $(subst $Q,'\'',$(gitwebdir))
+gitwebstaticdir_SQ = $(subst $Q,'\'',$(gitwebdir)/static)
+SHELL_PATH_SQ = $(subst $Q,'\'',$(SHELL_PATH))
+PERL_PATH_SQ  = $(subst $Q,'\'',$(PERL_PATH))
+DESTDIR_SQ    = $(subst $Q,'\'',$(DESTDIR))
 
 # Quiet generation (unless V=1)
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
diff -wrup git-2010-08-21/Makefile git-2010-08-21a/Makefile
--- git-2010-08-21/Makefile	2010-08-20 21:55:41.000000000 +0200
+++ git-2010-08-21a/Makefile	2010-08-21 21:02:07.676932693 +0200
@@ -236,6 +236,7 @@ all::
 #
 # Define NATIVE_CRLF if your platform uses CRLF for line endings.
 
+.PHONY: FORCE
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
@@ -2217,7 +2218,7 @@ endif
 
 .PHONY: all install clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-.PHONY: FORCE TAGS tags cscope
+.PHONY: TAGS tags cscope
 
 ### Check documentation
 #

Re: Compiling git with makepp patch

[Thomas Rast]

Daniel Pfeiffer wrote:
> [Attachment: git-makepp.patch]

Please read Documentation/SubmittingPatches for next time.

> There are however two things which I can hardly hope to fix:
[...]
> PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
> 
> [...] makepp parses this line just like Emacs, so it doesn't find
> the comment, adding in the #' at the point of use, which completely
> screws the sed command.
[...]
>  # Shell quote;
> -bindir_SQ = $(subst ','\'',$(bindir))#'
> -gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
> -gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
> -SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
> -PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
> -DESTDIR_SQ    = $(subst ','\'',$(DESTDIR))#'
> +Q='
> +# ' This comment is only to appease IDEs like Emacs.
> +# The comment is on a new line, else makepp would see it as a quoted hash.
> +bindir_SQ = $(subst $Q,'\'',$(bindir))
> +gitwebdir_SQ = $(subst $Q,'\'',$(gitwebdir))
> +gitwebstaticdir_SQ = $(subst $Q,'\'',$(gitwebdir)/static)
> +SHELL_PATH_SQ = $(subst $Q,'\'',$(SHELL_PATH))
> +PERL_PATH_SQ  = $(subst $Q,'\'',$(PERL_PATH))
> +DESTDIR_SQ    = $(subst $Q,'\'',$(DESTDIR))

Confusingly, you talk about comments above, but the real issue is that
your makepp apparently gives the ' special meaning.  For once "info
make" and "man 1p make" on my system agree on the semantics of ': none
at all.  From the latter:

  Early proposals stated that an "unquoted" number sign  was  treated  as  the
  start of a comment. The make utility does not pay any attention to quotes. A
  number sign starts a comment regardless of its surroundings.

So can you quote chapter and verse to show that there is anything to
fix?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: Compiling git with makepp patch

[Jakub Narebski]

Thomas Rast <trast@student.ethz.ch> writes:
> Daniel Pfeiffer wrote:
> > [Attachment: git-makepp.patch]
> 
> Please read Documentation/SubmittingPatches for next time.
> 
> > There are however two things which I can hardly hope to fix:
> [...]
> > PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
> > 
> > [...] makepp parses this line just like Emacs, so it doesn't find
> > the comment, adding in the #' at the point of use, which completely
> > screws the sed command.
> [...]
> >  # Shell quote;
> > -bindir_SQ = $(subst ','\'',$(bindir))#'
> > -gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
> > -gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
> > -SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
> > -PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
> > -DESTDIR_SQ    = $(subst ','\'',$(DESTDIR))#'
> > +Q='
> > +# ' This comment is only to appease IDEs like Emacs.
> > +# The comment is on a new line, else makepp would see it as a quoted hash.
> > +bindir_SQ = $(subst $Q,'\'',$(bindir))
> > +gitwebdir_SQ = $(subst $Q,'\'',$(gitwebdir))
> > +gitwebstaticdir_SQ = $(subst $Q,'\'',$(gitwebdir)/static)
> > +SHELL_PATH_SQ = $(subst $Q,'\'',$(SHELL_PATH))
> > +PERL_PATH_SQ  = $(subst $Q,'\'',$(PERL_PATH))
> > +DESTDIR_SQ    = $(subst $Q,'\'',$(DESTDIR))
> 
> Confusingly, you talk about comments above, but the real issue is that
> your makepp apparently gives the ' special meaning.  For once "info
> make" and "man 1p make" on my system agree on the semantics of ': none
> at all.  From the latter:
> 
>   Early proposals stated that an "unquoted" number sign  was  treated  as  the
>   start of a comment. The make utility does not pay any attention to quotes. A
>   number sign starts a comment regardless of its surroundings.
> 
> So can you quote chapter and verse to show that there is anything to
> fix?

Nevertheless using

  Q='
  SQ='\''
  bindir_SQ = $(subst $Q,$(SQ),$(bindir))

could make substitution more clear ('make' behavior nothwithstanding).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Compiling git with makepp patch

[Ævar Arnfjörð Bjarmason]

On Sun, Aug 22, 2010 at 22:31, Daniel Pfeiffer <occitan@t-online.de> wrote:

> Git has been giving our promise of Gnu make compatibility a hard
> time.

Aside from our bugs you can't make that promise if projects like Git
need patches to work with makepp :)

> The other thing caused me quite a headache before I understood:
>
> PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
>
> I suppose you added the comment for Emacs' syntax highlighting, to have an
> even number of unescaped quotes.

That was added by John 'Warthog9' Hawley, I wonder if that also came
with a M-x report-emacs-bug, e.g. cperl-mode deals with that case,
sounds like an easy-to-fix bug in makefile-gmake-mode.

> The problem is makepp parses this line
> just like Emacs, so it doesn't find the comment, adding in the #' at the
> point of use, which completely screws the sed command.  (You might want to
> apply my fix to a few other makefiles, which have SQ variables, albeit
> without the syntax highlighting workaround, so they are only visually
> defect.)

The reason Emacs has issues is because it uses an ad-hoc regexp based
parser that favours speed above correctness for syntax
highlighting.

I'm surprised you've gotten this far with makepp if you don't tokenize
comments and throw their contents away.

Re: Compiling git with makepp patch

[Jonathan Nieder]

Hi,

Daniel Pfeiffer wrote:

> .PHONY: FORCE
> 
> I don't know why you depend on a phony that has no rule — I also had
> to make that possible.

Surely the name explains it. :)

> The file needs to be built immediately so
> that it can be included, before reading the rest of the makefile.
> But the dependency is only known to be phony after running the rule.
> Here you have a hen-egg problem, where I have no clue how Gnu make
> can cope (this is the one case where it requires .PHONY).

GNU make, unlike, say, pmake, reads all the rules before it runs
anything iirc.  So you can have

 -include foo

 foo:
	echo bar: >foo
	echo '	echo hi' >>foo

and it will cope okay.

Anyway, the git makefile is very far from topologically sorted; if
you are suggesting we change that, that's fine with me, as long as
the new rule is somehow justified and consistent.

Hope that helps,
Jonathan

Re: Compiling git with makepp patch

[Daniel Pfeiffer]

  la 08/24/2010 06:32 AM Jonathan Nieder skribis:
> Hi,
>
> Daniel Pfeiffer wrote:
>> The file needs to be built immediately so
>> that it can be included, before reading the rest of the makefile.
>> But the dependency is only known to be phony after running the rule.
>> Here you have a hen-egg problem, where I have no clue how Gnu make
>> can cope (this is the one case where it requires .PHONY).
> GNU make, unlike, say, pmake, reads all the rules before it runs
> anything iirc.  So you can have
>
>   -include foo
>
>   foo:
> 	echo bar:>foo
> 	echo '	echo hi'>>foo
>
> and it will cope okay.
While that is not the usual use-case for -include, the file might very well 
define some macros, and the rest of the makefile, indeed the foo-rule itself 
might depend on those macros.  Better to have things in a clear order!

> Anyway, the git makefile is very far from topologically sorted; if
> you are suggesting we change that, that's fine with me, as long as
> the new rule is somehow justified and consistent.

coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                     http://lernu.net  /  http://ikurso.net

Re: Compiling git with makepp patch

[Daniel Pfeiffer]

  la 08/23/2010 10:27 PM Ævar Arnfjörð Bjarmason skribis:
> On Sun, Aug 22, 2010 at 22:31, Daniel Pfeiffer<occitan@t-online.de>  wrote:
>
>> Git has been giving our promise of Gnu make compatibility a hard
>> time.
> Aside from our bugs you can't make that promise if projects like Git
> need patches to work with makepp :)
Just two little things in your big makefiles.  The compatibility is just an 
added bonus, we have many other real strengths in makepp.

>> The other thing caused me quite a headache before I understood:
>>
>> PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
>>
>> I suppose you added the comment for Emacs' syntax highlighting, to have an
>> even number of unescaped quotes.
> That was added by John 'Warthog9' Hawley, I wonder if that also came
> with a M-x report-emacs-bug, e.g. cperl-mode deals with that case,
> sounds like an easy-to-fix bug in makefile-gmake-mode.
Hardly, if you look at my example below!

>> The problem is makepp parses this line
>> just like Emacs, so it doesn't find the comment, adding in the #' at the
>> point of use, which completely screws the sed command.  (You might want to
>> apply my fix to a few other makefiles, which have SQ variables, albeit
>> without the syntax highlighting workaround, so they are only visually
>> defect.)
> The reason Emacs has issues is because it uses an ad-hoc regexp based
> parser that favours speed above correctness for syntax
> highlighting.
Well, gmake rules are very twisted:  a and b don't do the same thing, because 
file functions shall respect quoting (though to my mind that should then be 
only one funny file name, which gmake gets wrong, splitting it up anyway), and 
c causes gmake to choke:

all: a b
a:
     echo : $(dir 'a # b/c/d') :

B = 'a # b/c/d'
b:
     echo : $(dir $B) :

C = $(dir 'a # b/c/d')
c:
     echo : $C :

> I'm surprised you've gotten this far with makepp if you don't tokenize
> comments and throw their contents away.
There are a few subtle differences, which mostly don't hurt.

coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                     http://lernu.net  /  http://ikurso.net

Re: Compiling git with makepp patch

[Daniel Pfeiffer]

  la 08/23/2010 09:47 AM Thomas Rast skribis:
> Daniel Pfeiffer wrote:
>>   # Shell quote;
>> -bindir_SQ = $(subst ','\'',$(bindir))#'
>> -gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
>> -gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
>> -SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
>> -PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
>> -DESTDIR_SQ    = $(subst ','\'',$(DESTDIR))#'
>> +Q='
>> +# ' This comment is only to appease IDEs like Emacs.
>> +# The comment is on a new line, else makepp would see it as a quoted hash.
>> +bindir_SQ = $(subst $Q,'\'',$(bindir))
>> +gitwebdir_SQ = $(subst $Q,'\'',$(gitwebdir))
>> +gitwebstaticdir_SQ = $(subst $Q,'\'',$(gitwebdir)/static)
>> +SHELL_PATH_SQ = $(subst $Q,'\'',$(SHELL_PATH))
>> +PERL_PATH_SQ  = $(subst $Q,'\'',$(PERL_PATH))
>> +DESTDIR_SQ    = $(subst $Q,'\'',$(DESTDIR))
> Confusingly, you talk about comments above, but the real issue is that
> your makepp apparently gives the ' special meaning.  For once "info
> make" and "man 1p make" on my system agree on the semantics of ': none
> at all.  From the latter:
>
>    Early proposals stated that an "unquoted" number sign  was  treated  as  the
>    start of a comment. The make utility does not pay any attention to quotes. A
>    number sign starts a comment regardless of its surroundings.
If you look at the example I just sent to Ævar, that contains examples of how 
crazy the situation really is in gmake.  What you cite here is wrong!

> So can you quote chapter and verse to show that there is anything to
> fix?
Well, I pointed you to makepp, which is subtly different from gmake, which you 
could.  If you choose not to use it, please yourself!

coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                     http://lernu.net  /  http://ikurso.net

Re: Compiling git with makepp patch

[Andreas Schwab]

Daniel Pfeiffer <occitan@t-online.de> writes:

>  la 08/24/2010 06:32 AM Jonathan Nieder skribis:
>> Hi,
>>
>> Daniel Pfeiffer wrote:
>>> The file needs to be built immediately so
>>> that it can be included, before reading the rest of the makefile.
>>> But the dependency is only known to be phony after running the rule.
>>> Here you have a hen-egg problem, where I have no clue how Gnu make
>>> can cope (this is the one case where it requires .PHONY).
>> GNU make, unlike, say, pmake, reads all the rules before it runs
>> anything iirc.  So you can have
>>
>>   -include foo
>>
>>   foo:
>> 	echo bar:>foo
>> 	echo '	echo hi'>>foo
>>
>> and it will cope okay.
> While that is not the usual use-case for -include, the file might very
> well define some macros, and the rest of the makefile, indeed the foo-rule
> itself might depend on those macros.

GNU make will restart reading all makefiles when any of them was remade.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Compiling git with makepp patch

[Junio C Hamano]

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> GNU make, unlike, say, pmake, reads all the rules before it runs
>>> anything iirc.  So you can have
>>>
>>>   -include foo
>>>
>>>   foo:
>>> 	echo bar:>foo
>>> 	echo '	echo hi'>>foo
>>>
>>> and it will cope okay.
>> While that is not the usual use-case for -include, the file might very
>> well define some macros, and the rest of the makefile, indeed the foo-rule
>> itself might depend on those macros.
>
> GNU make will restart reading all makefiles when any of them was remade.

I am not sure about the "reread" part, but shouldn't all POSIX compliant
make be topology driven (i.e. " reads all the rules before it runs
anything")?

Re: Compiling git with makepp patch

[Jonathan Nieder]

Junio C Hamano wrote:
>>> Jonathan Nieder wrote:

>>>> GNU make, unlike, say, pmake, reads all the rules before it runs
>>>> anything iirc.
[...]
> I am not sure about the "reread" part, but shouldn't all POSIX compliant
> make be topology driven (i.e. " reads all the rules before it runs
> anything")?

Yes, you are right.

I was thinking of something else, namely that

-- 8< --
#!/bin/sh

cat <<\EOF >Makefile &&
.foo.bar:
	cp $< $@

.SUFFIXES: .foo .bar
EOF

echo hello >test.foo &&
pmake test.bar
-- >8 --

fails while the corresponding script for GNU make succeeds.