[PATCH/RFC] "Recursive Make considered harmful"

[PATCH/RFC] "Recursive Make considered harmful"

[Ryan Anderson]

Convert build process from recurse Make to a single Make

The old Makefiles in Documentation/ and tools/ still exist until we feel
confident that I didn't miss anything on this conversion.

Most of this patch is fixing up the main Makefile to avoid overlapping
target names.

Signed-off-by: Ryan Anderson <ryan@michonline.com>
---

 Documentation/Makefile.inc |   50 ++++++++++++++++++++++++++++++++++++++++++++
 Makefile                   |   24 ++++++++++++++-------
 tools/Makefile.inc         |   12 +++++++++++
 3 files changed, 78 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/Makefile.inc
 create mode 100644 tools/Makefile.inc

003afd3ed1f83b4533b628182fa16c9ab0dc0467
diff --git a/Documentation/Makefile.inc b/Documentation/Makefile.inc
new file mode 100644
--- /dev/null
+++ b/Documentation/Makefile.inc
@@ -0,0 +1,50 @@
+MAN1_TXT=$(wildcard Documentation/git-*.txt)
+MAN7_TXT=Documentation/git.txt
+
+DOC_HTML=$(patsubst %.txt,%.html,$(MAN1_TXT) $(MAN7_TXT))
+
+DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT))
+DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
+
+mandir=$(prefix)/man
+man1=$(mandir)/man1
+man7=$(mandir)/man7
+
+#
+# Please note that there is a minor bug in asciidoc.
+# The version after 6.0.3 _will_ include the patch found here:
+#   http://marc.theaimsgroup.com/?l=git&m=111558757202243&w=2
+#
+# Until that version is released you may have to apply the patch
+# yourself - yes, all 6 characters of it!
+#
+
+all-doc: html man
+
+html: $(DOC_HTML)
+
+
+man: man1 man7
+man1: $(DOC_MAN1)
+man7: $(DOC_MAN7)
+
+install-doc:
+	$(INSTALL) -m755 -d $(dest)/$(man1) $(dest)/$(man7)
+	$(INSTALL) $(DOC_MAN1) $(dest)/$(man1)
+	$(INSTALL) $(DOC_MAN7) $(dest)/$(man7)
+
+# 'include' dependencies
+Documentation/git-diff-%.txt: Documentation/diff-format.txt Documentation/diff-options.txt
+	touch $@
+
+clean-doc:
+	rm -f Documentation/*.xml Documentation/*.html Documentation/*.1 Documentation/*.7
+
+%.html : %.txt
+	asciidoc -b xhtml11 -d manpage $<
+
+%.1 %.7 : %.xml
+	xmlto -o Documentation/ man $<
+
+%.xml : %.txt
+	asciidoc -b docbook -d manpage $<
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -54,9 +54,17 @@ PROG=   git-update-cache git-diff-files 
 	git-show-index git-daemon git-var git-peek-remote \
 	git-update-server-info git-show-rev-cache git-build-rev-cache
 
-all: $(PROG)
+include Documentation/Makefile.inc
+include tools/Makefile.inc
 
-install: $(PROG) $(SCRIPTS)
+all: all-bin all-doc
+all-bin: $(PROG)
+#all-tools
+
+install: install-bin install-doc
+#install-tools
+
+install-bin: $(PROG) $(SCRIPTS)
 	$(INSTALL) -m755 -d $(dest)$(bin)
 	$(INSTALL) $(PROG) $(SCRIPTS) $(dest)$(bin)
 
@@ -204,20 +212,20 @@ rpm: dist
 test: all
 	$(MAKE) -C t/ all
 
-doc:
-	$(MAKE) -C Documentation all
+doc: all-doc
+#	$(MAKE) -C Documentation all
 
-install-tools:
+install-toolsxx:
 	$(MAKE) -C tools install
 
-install-doc:
+install-docxx:
 	$(MAKE) -C Documentation install
 
-clean:
+clean: clean-doc clean-tools
 	rm -f *.o mozilla-sha1/*.o ppc/*.o $(PROG) $(LIB_FILE)
 	rm -f git-core-*.tar.gz git-core.spec
 	$(MAKE) -C tools/ clean
-	$(MAKE) -C Documentation/ clean
 
 backup: clean
 	cd .. ; tar czvf dircache.tar.gz dir-cache
+
diff --git a/tools/Makefile.inc b/tools/Makefile.inc
new file mode 100644
--- /dev/null
+++ b/tools/Makefile.inc
@@ -0,0 +1,12 @@
+#
+# Make Linus git-tools
+#
+
+PROG += $(addprefix tools/,git-mailsplit git-mailinfo)
+SCRIPTS += $(addprefix tools/,git-applymbox git-applypatch)
+
+tools/git-%: tools/%.c
+	$(CC) $(CFLAGS) -o $@ $(filter %.c,$^)
+
+clean-tools:
+	rm -f tools/*.o

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH/RFC] "Recursive Make considered harmful"

[A Large Angry SCM]

Ryan Anderson wrote:
> Convert build process from recurse Make to a single Make
> 

Please explain the rational for this.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Kirby C. Bohling]

On Wed, Jul 27, 2005 at 10:25:10AM -0400, A Large Angry SCM wrote:
> Ryan Anderson wrote:
> >Convert build process from recurse Make to a single Make
> >
> 
> Please explain the rational for this.

I'm new to the list, but given the subject, I'm fairly confident
it's this.

http://www.canb.auug.org.au/~millerp/rmch/recu-make-cons-harm.html

I'm a convert.  I converted a fairly large code base at work, and it
was a huge boon for productivity.  Build times dropped dramatically
(from 40 seconds to 2-5 for a single file change).

He used the exact wording just about everyone dones when referring
to it conceptually.  It's easy to google for support and rebuttal:

http://www.google.com/search?hl=en&q=Recursive+Make+considered+harmful&btnG=Google+Search

    Thanks,
        Kirby

Re: [PATCH/RFC] "Recursive Make considered harmful"

[A Large Angry SCM]

Kirby C. Bohling wrote:
> On Wed, Jul 27, 2005 at 10:25:10AM -0400, A Large Angry SCM wrote:
>>Ryan Anderson wrote:
>>>Convert build process from recurse Make to a single Make
>>>
>>Please explain the rational for this.
> 
> I'm new to the list, but given the subject, I'm fairly confident
> it's this.
> 
> http://www.canb.auug.org.au/~millerp/rmch/recu-make-cons-harm.html
> 
...
> 
> He used the exact wording just about everyone dones when referring
> to it conceptually.  It's easy to google for support and rebuttal:
> 
> http://www.google.com/search?hl=en&q=Recursive+Make+considered+harmful&btnG=Google+Search

Thanks for the references.

A quick read of the paper and some of the rebuttals make me think that 
either way (recursive/non-recursive):
	* require about the same amount of makefile/dependency maintenance work 
from developers.
	* allow developers to be lazy in different ways with 
makefiles/dependencies.
	* achieves the same end.

The non-recursive make method may have a small advantage for developers 
using Git for their SCM because the Git operations are also performed at 
the top level due to Git's design.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

> 003afd3ed1f83b4533b628182fa16c9ab0dc0467
> diff --git a/Documentation/Makefile.inc b/Documentation/Makefile.inc
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/Makefile.inc
> @@ -0,0 +1,50 @@
> +MAN1_TXT=$(wildcard Documentation/git-*.txt)
> +MAN7_TXT=Documentation/git.txt
> +
>...

While I do not have strong objections to make the build process
go faster, it is somewhat disturbing that the Makefile pieces
maintained in subdirectories need to name things they touch
using paths that include the subdirectory names.  I do not have
a better alternative to suggest, though...

I'd keep it in the proposed updates branch for now and wait for
a bit until discussions on the list die out.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[A Large Angry SCM]

Junio C Hamano wrote:
> While I do not have strong objections to make the build process
> go faster, it is somewhat disturbing that the Makefile pieces
> maintained in subdirectories need to name things they touch
> using paths that include the subdirectory names.  I do not have
> a better alternative to suggest, though...

For a project the size of Git, is there any real benefit to this change?

Besides pathing issues, you also have to aware that all identifiers in 
the included makefile fragments will be global.

I don't object to the change but I see it as trading one maintenance 
issue for another.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Petr Baudis]

Dear diary, on Thu, Jul 28, 2005 at 12:07:07AM CEST, I got a letter
where A Large Angry SCM <gitzilla@gmail.com> told me that...
> Junio C Hamano wrote:
> >While I do not have strong objections to make the build process
> >go faster, it is somewhat disturbing that the Makefile pieces
> >maintained in subdirectories need to name things they touch
> >using paths that include the subdirectory names.  I do not have
> >a better alternative to suggest, though...
> 
> For a project the size of Git, is there any real benefit to this change?
> 
> Besides pathing issues, you also have to aware that all identifiers in 
> the included makefile fragments will be global.
> 
> I don't object to the change but I see it as trading one maintenance 
> issue for another.

I'd also argue that generally, larger files are inherently harder to
maintain, and having all the targets for all the subdirectories in a
single file sounds nightmarish. (OTOH, by now you probably know that I'm
a keep-it-as-local-as-possible junkie.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Matthias Urlichs]

Hi, Petr Baudis wrote:

> having all the targets for all the subdirectories in a
> single file sounds nightmarish

which is why you'd include Makefile[.inc] snippets from subdirectories
instead.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
The only difference between a fool and a criminal who attacks a system is
that the fool attacks unpredictably and on a broader front.
					-- Tom Gibb

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Junio C Hamano]

Ryan, I am dropping this patch, at least for now, after keeping
it in the "pu" (proposed updates) branch and using it myself.
There are two complaints from me.

I am used to "make bin=$HOME/bin/i386 install install-tools",
which the patch breaks (I do not want to build docs for myself).
This is minor; I could say "install-bin install-toolsxx"
instead.

I do not deal with RPM packages myself, but guessing by reading
git-core.spec.in, I think it relies on the install target not
touching the documentation, in order for it to be able to build
doc-full and/or doc-less binary packages.  The patch makes
install target to also build and install docs.

The Debian build is not affected because it does not produce
separate git-core and doc-git-core packages[*1*]; probably this
was the reason you did not notice this.

I think what is installed from the toplevel and what comes from
tools/ subdirectory are divided mostly for historical reasons
and nothing else[*2*], and I do not mind the install target
depending on install-bin and install-tools, but I suspect that
binary packaging folks would appreciate to have a separate doc
target that is not done by a normal install.

Speeding up the build procedure by defining dependencies
correctly is a worthy goal.  Personally I feel a low hanging
fruit is in the main Makefile, before worrying about the make
recursion.  Many things are in libgit.a and when I touch
something only relevant to small number of things, say
csum-file.c, all "git-%: %.c" programs are recompiled and
relinked, even most of them do not link with csum-file.o (this
particular one is only used by git-pack-objects, by the way).

[Footnote]

*1* Which, BTW, would be the Debian way, if I am not mistaken.

*2* Although one _could_ argue that tools/ is primarily meant
for "project lead" role users who accept and incorporate patches
obtained via e-mails.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Matthias Urlichs]

Hi, Junio C Hamano wrote:

> The Debian build is not affected because it does not produce
> separate git-core and doc-git-core packages[*1*]; probably this
> was the reason you did not notice this.

git-core-doc, actually.

Debian does that only if the documentation is substantial. Even then,
manpages may not be segregated into -doc.

However, I *would* segregate gitk into its own Debian package, because
it requires wish et al., which would pull a large chunk of X11 stuff,
which people may not want on their server.

Patch follows separately -- I'll have to pull it from my other mess
(which includes yet another Debian package for Cogito ;-).

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
A zealot's stones will break my bones, but gods will never hurt me.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Junio C Hamano]

Matthias Urlichs <smurf@smurf.noris.de> writes:

> However, I *would* segregate gitk into its own Debian package, because
> it requires wish et al., which would pull a large chunk of X11 stuff,
> which people may not want on their server.

While I agree gitk should not come as part of git package, this
brings up a different issue.

Ideally, I'd want to see gitk packaged from its repository
kernel.org:/pub/scm/gitk/git.git/ Paul Mackerras maintains, not
from GIT one which _will_ lag behind.

We have a copy of gitk in git repository because Linus "merged"
it as "the coolest merge ever" example.  While I intend to keep
updating from gitk repository from time to time only because I
do not want to ship ancient version of it, I see a big problem
down the road.  What happens if someday Paul wanted to have a
toplevel Makefile of his own, or if somebody sends him a patch
to add debian/rules file to build a separate gitk package from
its own source tree?  Pulling/merging from gitk repo to update
the copy git has suddenly becomes a nightmere.

While I _do_ rely on gitk in my git work, and I _do_ like its
simplicity (just a single file right now), my longer term
preference is to drop the copy we have in git tree and treat it
just like the other repository browser, qgit.  Our documentation
should point people at it as part of the Porcelain suite.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[A Large Angry SCM]

Junio C Hamano wrote:
...
> While I agree gitk should not come as part of git package, this
> brings up a different issue.
> 
> Ideally, I'd want to see gitk packaged from its repository
> kernel.org:/pub/scm/gitk/git.git/ Paul Mackerras maintains, not
> from GIT one which _will_ lag behind.
> 
...
> 
> While I _do_ rely on gitk in my git work, and I _do_ like its
> simplicity (just a single file right now), my longer term
> preference is to drop the copy we have in git tree and treat it
> just like the other repository browser, qgit.  Our documentation
> should point people at it as part of the Porcelain suite.

Then this should happen sooner rather than later.

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Ryan Anderson]

On Wed, Jul 27, 2005 at 02:50:55PM -0700, Junio C Hamano wrote:
> Ryan Anderson <ryan@michonline.com> writes:
> 
> > 003afd3ed1f83b4533b628182fa16c9ab0dc0467
> > diff --git a/Documentation/Makefile.inc b/Documentation/Makefile.inc
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/Makefile.inc
> > @@ -0,0 +1,50 @@
> > +MAN1_TXT=$(wildcard Documentation/git-*.txt)
> > +MAN7_TXT=Documentation/git.txt
> > +
> >...
> 
> While I do not have strong objections to make the build process
> go faster, it is somewhat disturbing that the Makefile pieces
> maintained in subdirectories need to name things they touch
> using paths that include the subdirectory names.  I do not have
> a better alternative to suggest, though...
> 
> I'd keep it in the proposed updates branch for now and wait for
> a bit until discussions on the list die out.

Sorry for taking so long to respond here - I've probably got 2 or 3
general replies to make on this thread, but basically, I truly intended
it as a RFC.

I think the best justification for the end goal of the process I was
thinking of starting is this:

	$ git clone -l git-linus git-example
	defaulting to local storage area
	0 blocks
	$ cd git-example
	$ git checkout
	$ ls | wc -l
	154

I've been spending some time trying to think out what qualifies as a
"tool" and what is "core", etc.  I think it wouldn't be a bad idea to
think about restructuring things a bit so that all the little "helper"
scripts we keep adding don't fill up the top level directory.

I think I'm going to rethink this, a bit more.  I'm unhappy with how I
had to edit the sub-dir Makefiles to include directory names.  Sam, if
you happen to be reading this, feel free to help out!

I'm almost thinking that something like:

	PROGS := 
	SCRIPTS :=
	include x/Makefile.inc
	PROGRAMS += $(addprefix x/,$PROGS)
	ALL_SCRIPTS += $(addprefix x/,$SCRIPTS)

in the top-level Makefile might be the cleanest way to keep the
subdirectory ones simpler - but that's still somewhat distasteful, and
only fixes up one part of the problem.

Anyway, I'll come back to this later when I've got some of the follow-up
issues sorted out, like what to do with the directory structure.

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Sam Ravnborg]

> > While I do not have strong objections to make the build process
> > go faster, it is somewhat disturbing that the Makefile pieces
> > maintained in subdirectories need to name things they touch
> > using paths that include the subdirectory names.  I do not have
> > a better alternative to suggest, though...

If the goal is to speed up the build process the only sane way is to fix
the dependencies. In kbuild fixdep is used to parse the .c file and it
locates all references to .h files (recursive) and also detects any
usage of CONFIG_ symbols.
This part should be relative straightforward to include in git.

> I think I'm going to rethink this, a bit more.  I'm unhappy with how I
> had to edit the sub-dir Makefiles to include directory names.  Sam, if
> you happen to be reading this, feel free to help out!
> 
> I'm almost thinking that something like:
> 
> 	PROGS := 
> 	SCRIPTS :=
> 	include x/Makefile.inc
> 	PROGRAMS += $(addprefix x/,$PROGS)
> 	ALL_SCRIPTS += $(addprefix x/,$SCRIPTS)

That is doable for sure. But it hits you hard when you have to create
some special rules in a subdirectory - then you need to know in what
directory you are placed. You could assing sub := x before including
x/Makefile.inc.

On the other hand. The recursive make considered harmful is IMHO a bit
overaggregated. See the kernel where it is used extensively. And it
works with no hassle. For a small project like git it should be possible
to keep the dependencies in proper shape so there is no cross directory
boundaries to worry about - or at least only a few.

	Sam

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Petr Baudis]

Dear diary, on Fri, Jul 29, 2005 at 09:31:34AM CEST, I got a letter
where Sam Ravnborg <sam@ravnborg.org> told me that...
> > > While I do not have strong objections to make the build process
> > > go faster, it is somewhat disturbing that the Makefile pieces
> > > maintained in subdirectories need to name things they touch
> > > using paths that include the subdirectory names.  I do not have
> > > a better alternative to suggest, though...
> 
> If the goal is to speed up the build process the only sane way is to fix
> the dependencies. In kbuild fixdep is used to parse the .c file and it
> locates all references to .h files (recursive) and also detects any
> usage of CONFIG_ symbols.
> This part should be relative straightforward to include in git.

FWIW, I made tiny "build system" (inspired by kconfig) for smaller
projects I work on:

http://pasky.or.cz/~pasky/dev/tunneler/co/Makefile
http://pasky.or.cz/~pasky/dev/tunneler/co/Makefile.lib
http://pasky.or.cz/~pasky/dev/tunneler/co/client/Makefile

Perhaps someone might find that a nice base for further hacking. It
generally appears to work pretty well in practice, although the
automatic dependency tracking might not be perfect.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH/RFC] "Recursive Make considered harmful"

[Timo Hirvonen]

On Fri, 29 Jul 2005 09:46:14 +0200
Petr Baudis <pasky@suse.cz> wrote:

> FWIW, I made tiny "build system" (inspired by kconfig) for smaller
> projects I work on:

Me too! :)

http://onion.dynserv.net/~timo/index.php?page=Projects/build

It also has configuration system written in bash.

-- 
http://onion.dynserv.net/~timo/