out of tree build fixes

out of tree build fixes

[David Bremner]

Apparently we've neglected the out of tree builds even more than
usual. I originally thought this is what was causing the test failures
on buildbot. I'm less sure now, but I think these fixes don't hurt
anything, and they do unbreak out of tree builds for me.

There is certainly still work to be done with our out of tree builds.
For those of you just joining the party, that doesn't mean we want to
switch to your favourite build system. Or even to Autotools.

An earlier draft of this series (discussed on IRC) defined a builddir
variable, but eventually I realized the current directory of make is
always builddir. So at least for now that doesn't seem to be needed.

[PATCH 1/3] build: compute subdirectories from the presence of Makefile.local

[David Bremner]

This allows the fragile sed command in configure to be replaced with a somewhat
less fragile find command.
---
 Makefile  | 3 +--
 configure | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 061c55a..74b476c 100644
--- a/Makefile
+++ b/Makefile
@@ -5,8 +5,7 @@ all:
 # List all subdirectories here. Each contains its own Makefile.local.
 # Use of '=', without '+=', seems to be required for out-of-tree
 # builds to work.
-subdirs = compat completion doc emacs lib parse-time-string	\
-	performance-test util test test/test-databases
+subdirs = $(dir $(shell find . -mindepth 2 -name Makefile.local))
 
 # We make all targets depend on the Makefiles themselves.
 global_deps = Makefile Makefile.config Makefile.local \
diff --git a/configure b/configure
index fb276f1..b3b1b44 100755
--- a/configure
+++ b/configure
@@ -23,7 +23,7 @@ srcdir=$(dirname "$0")
 # the directory structure and copy Makefiles.
 if [ "$srcdir" != "." ]; then
 
-    for dir in . $(grep "^subdirs *=" "$srcdir"/Makefile | sed -e "s/subdirs *= *//"); do
+    for dir in $(cd $srcdir && find . -name Makefile.local -exec dirname {} \;); do
 	mkdir -p "$dir"
 	cp "$srcdir"/"$dir"/Makefile.local "$dir"
 	cp "$srcdir"/"$dir"/Makefile "$dir"
-- 
1.9.0

Re: [PATCH 1/3] build: compute subdirectories from the presence of Makefile.local

[David Bremner]

David Bremner <david@tethera.net> writes:

> This allows the fragile sed command in configure to be replaced with a somewhat
> less fragile find command.
> -subdirs = compat completion doc emacs lib parse-time-string	\
> -	performance-test util test test/test-databases
> +subdirs = $(dir $(shell find . -mindepth 2 -name Makefile.local))

I discovered a bug with this idea. There is actually an ordering
dependency for the sub Makefiles; in particular the targets of
test/Makefile.local use variables defined in lib/Makefile.local. I'll
have to think if/how this can be salvaged.

d

[PATCH] build: move canonical list of subdirectories to configure script

[David Bremner]

The configure script needs this list for out of tree builds. Grabbing
it from the Makefile via sed was fragile and broken.
---
 Makefile  | 15 +++++----------
 configure |  8 +++++++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 061c55a..4c0e8c6 100644
--- a/Makefile
+++ b/Makefile
@@ -2,16 +2,6 @@
 # given explicitly on the command line) so mention it first.
 all:
 
-# List all subdirectories here. Each contains its own Makefile.local.
-# Use of '=', without '+=', seems to be required for out-of-tree
-# builds to work.
-subdirs = compat completion doc emacs lib parse-time-string	\
-	performance-test util test test/test-databases
-
-# We make all targets depend on the Makefiles themselves.
-global_deps = Makefile Makefile.config Makefile.local \
-	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
-
 # Sub-directory Makefile.local fragments can append to these variables
 # to have directory-specific cflags as necessary.
 
@@ -27,6 +17,11 @@ extra_cxxflags :=
 srcdir ?= .
 
 include Makefile.config
+
+# We make all targets depend on the Makefiles themselves.
+global_deps = Makefile Makefile.config Makefile.local \
+	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
+
 Makefile.config: $(srcdir)/configure
 ifeq ($(configure_options),)
 	@echo ""
diff --git a/configure b/configure
index fb276f1..ab73317 100755
--- a/configure
+++ b/configure
@@ -19,11 +19,14 @@ readonly DEFAULT_IFS="$IFS"
 
 srcdir=$(dirname "$0")
 
+subdirs="util compat lib parse-time-string completion doc emacs"
+subdirs="${subdirs} performance-test test test/test-databases"
+
 # For a non-srcdir configure invocation (such as ../configure), create
 # the directory structure and copy Makefiles.
 if [ "$srcdir" != "." ]; then
 
-    for dir in . $(grep "^subdirs *=" "$srcdir"/Makefile | sed -e "s/subdirs *= *//"); do
+    for dir in . ${subdirs}; do
 	mkdir -p "$dir"
 	cp "$srcdir"/"$dir"/Makefile.local "$dir"
 	cp "$srcdir"/"$dir"/Makefile "$dir"
@@ -698,6 +701,9 @@ cat > Makefile.config <<EOF
 # directory (the current directory at the time configure was run).
 srcdir = ${srcdir}
 
+# subdirectories to build
+subdirs = ${subdirs}
+
 configure_options = $@
 
 # We use vpath directives (rather than the VPATH variable) since the
-- 
1.9.0

Re: [PATCH] build: move canonical list of subdirectories to configure script

[Tomi Ollila]

On Tue, Mar 25 2014, David Bremner <david@tethera.net> wrote:

> The configure script needs this list for out of tree builds. Grabbing
> it from the Makefile via sed was fragile and broken.
> ---

This is exactly what I was going to suggest if I had the time.

LGTM.

Tomi


>  Makefile  | 15 +++++----------
>  configure |  8 +++++++-
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 061c55a..4c0e8c6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,16 +2,6 @@
>  # given explicitly on the command line) so mention it first.
>  all:
>  
> -# List all subdirectories here. Each contains its own Makefile.local.
> -# Use of '=', without '+=', seems to be required for out-of-tree
> -# builds to work.
> -subdirs = compat completion doc emacs lib parse-time-string	\
> -	performance-test util test test/test-databases
> -
> -# We make all targets depend on the Makefiles themselves.
> -global_deps = Makefile Makefile.config Makefile.local \
> -	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
> -
>  # Sub-directory Makefile.local fragments can append to these variables
>  # to have directory-specific cflags as necessary.
>  
> @@ -27,6 +17,11 @@ extra_cxxflags :=
>  srcdir ?= .
>  
>  include Makefile.config
> +
> +# We make all targets depend on the Makefiles themselves.
> +global_deps = Makefile Makefile.config Makefile.local \
> +	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
> +
>  Makefile.config: $(srcdir)/configure
>  ifeq ($(configure_options),)
>  	@echo ""
> diff --git a/configure b/configure
> index fb276f1..ab73317 100755
> --- a/configure
> +++ b/configure
> @@ -19,11 +19,14 @@ readonly DEFAULT_IFS="$IFS"
>  
>  srcdir=$(dirname "$0")
>  
> +subdirs="util compat lib parse-time-string completion doc emacs"
> +subdirs="${subdirs} performance-test test test/test-databases"
> +
>  # For a non-srcdir configure invocation (such as ../configure), create
>  # the directory structure and copy Makefiles.
>  if [ "$srcdir" != "." ]; then
>  
> -    for dir in . $(grep "^subdirs *=" "$srcdir"/Makefile | sed -e "s/subdirs *= *//"); do
> +    for dir in . ${subdirs}; do
>  	mkdir -p "$dir"
>  	cp "$srcdir"/"$dir"/Makefile.local "$dir"
>  	cp "$srcdir"/"$dir"/Makefile "$dir"
> @@ -698,6 +701,9 @@ cat > Makefile.config <<EOF
>  # directory (the current directory at the time configure was run).
>  srcdir = ${srcdir}
>  
> +# subdirectories to build
> +subdirs = ${subdirs}
> +
>  configure_options = $@
>  
>  # We use vpath directives (rather than the VPATH variable) since the
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] build: move canonical list of subdirectories to configure script

[Jani Nikula]

On Tue, 25 Mar 2014, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Tue, Mar 25 2014, David Bremner <david@tethera.net> wrote:
>
>> The configure script needs this list for out of tree builds. Grabbing
>> it from the Makefile via sed was fragile and broken.
>> ---
>
> This is exactly what I was going to suggest if I had the time.
>
> LGTM.

Ditto, seems to work too.

>
> Tomi
>
>
>>  Makefile  | 15 +++++----------
>>  configure |  8 +++++++-
>>  2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 061c55a..4c0e8c6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2,16 +2,6 @@
>>  # given explicitly on the command line) so mention it first.
>>  all:
>>  
>> -# List all subdirectories here. Each contains its own Makefile.local.
>> -# Use of '=', without '+=', seems to be required for out-of-tree
>> -# builds to work.
>> -subdirs = compat completion doc emacs lib parse-time-string	\
>> -	performance-test util test test/test-databases
>> -
>> -# We make all targets depend on the Makefiles themselves.
>> -global_deps = Makefile Makefile.config Makefile.local \
>> -	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
>> -
>>  # Sub-directory Makefile.local fragments can append to these variables
>>  # to have directory-specific cflags as necessary.
>>  
>> @@ -27,6 +17,11 @@ extra_cxxflags :=
>>  srcdir ?= .
>>  
>>  include Makefile.config
>> +
>> +# We make all targets depend on the Makefiles themselves.
>> +global_deps = Makefile Makefile.config Makefile.local \
>> +	$(subdirs:%=%/Makefile) $(subdirs:%=%/Makefile.local)
>> +
>>  Makefile.config: $(srcdir)/configure
>>  ifeq ($(configure_options),)
>>  	@echo ""
>> diff --git a/configure b/configure
>> index fb276f1..ab73317 100755
>> --- a/configure
>> +++ b/configure
>> @@ -19,11 +19,14 @@ readonly DEFAULT_IFS="$IFS"
>>  
>>  srcdir=$(dirname "$0")
>>  
>> +subdirs="util compat lib parse-time-string completion doc emacs"
>> +subdirs="${subdirs} performance-test test test/test-databases"
>> +
>>  # For a non-srcdir configure invocation (such as ../configure), create
>>  # the directory structure and copy Makefiles.
>>  if [ "$srcdir" != "." ]; then
>>  
>> -    for dir in . $(grep "^subdirs *=" "$srcdir"/Makefile | sed -e "s/subdirs *= *//"); do
>> +    for dir in . ${subdirs}; do
>>  	mkdir -p "$dir"
>>  	cp "$srcdir"/"$dir"/Makefile.local "$dir"
>>  	cp "$srcdir"/"$dir"/Makefile "$dir"
>> @@ -698,6 +701,9 @@ cat > Makefile.config <<EOF
>>  # directory (the current directory at the time configure was run).
>>  srcdir = ${srcdir}
>>  
>> +# subdirectories to build
>> +subdirs = ${subdirs}
>> +
>>  configure_options = $@
>>  
>>  # We use vpath directives (rather than the VPATH variable) since the
>> -- 
>> 1.9.0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/3] doc: fix out-of-tree build

[David Bremner]

The subtle part is adding .rst and .py files to vpath so they can be
used as dependencies without prefixing with $(srcdir)

We also change the interface to mkbuildeps.py: rather than getting the
containing directory from the conf file path, we go the other way.
---
 configure          |  3 ++-
 doc/Makefile.local | 12 ++++++------
 doc/mkdocdeps.py   |  7 ++++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index b3b1b44..bc61ad1 100755
--- a/configure
+++ b/configure
@@ -713,8 +713,9 @@ configure_options = $@
 # files, (which is quite ugly).
 vpath %.c \$(srcdir)
 vpath %.cc \$(srcdir)
-vpath %.1 \$(srcdir)
 vpath Makefile.% \$(srcdir)
+vpath %.py \$(srcdir)
+vpath %.rst \$(srcdir)
 
 # The C compiler to use
 CC = ${CC}
diff --git a/doc/Makefile.local b/doc/Makefile.local
index 9c31c24..0980c71 100644
--- a/doc/Makefile.local
+++ b/doc/Makefile.local
@@ -3,15 +3,15 @@
 dir := doc
 
 # You can set these variables from the command line.
-SPHINXOPTS    := -q -c $(dir)
+SPHINXOPTS    := -q
 SPHINXBUILD   = sphinx-build
 DOCBUILDDIR      := $(dir)/_build
 
-prerst2man := python $(dir)/prerst2man.py
-mkdocdeps := python $(dir)/mkdocdeps.py
+prerst2man := python $(srcdir)/$(dir)/prerst2man.py
+mkdocdeps := python $(srcdir)/$(dir)/mkdocdeps.py
 
 # Internal variables.
-ALLSPHINXOPTS   := -d $(DOCBUILDDIR)/doctrees $(SPHINXOPTS) $(dir)
+ALLSPHINXOPTS   := -d $(DOCBUILDDIR)/doctrees $(SPHINXOPTS) $(srcdir)/$(dir)
 
 .PHONY: sphinx-html sphinx-texinfo sphinx-info
 
@@ -43,7 +43,7 @@ ifeq ($(HAVE_SPHINX),1)
 	    mv $(DOCBUILDDIR)/man/*.$${section} $(DOCBUILDDIR)/man/man$${section}; \
 	done
 else ifeq ($(HAVE_RST2MAN),1)
-	$(prerst2man) $(DOCBUILDDIR)/.. $(DOCBUILDDIR)/man
+	$(prerst2man) $(srcdir)/doc $(DOCBUILDDIR)/man
 else
 	@echo "Fatal: build dependency fail."
 	@false
@@ -68,7 +68,7 @@ install-man: ${MAN_GZIP_FILES}
 endif
 
 $(dir)/docdeps.mk: $(dir)/conf.py $(dir)/mkdocdeps.py
-	$(mkdocdeps) $< $(DOCBUILDDIR) $@
+	$(mkdocdeps) $(srcdir)/doc $(DOCBUILDDIR) $@
 
 CLEAN := $(CLEAN) $(DOCBUILDDIR) $(dir)/docdeps.mk $(dir)/man.stamp
 CLEAN := $(CLEAN) $(MAN_GZIP_FILES) $(MAN_ROFF_FILES)
diff --git a/doc/mkdocdeps.py b/doc/mkdocdeps.py
index 3effdd8..71bd135 100644
--- a/doc/mkdocdeps.py
+++ b/doc/mkdocdeps.py
@@ -1,15 +1,16 @@
 from sys import argv
-conffile = argv[1]
+srcdir = argv[1]
 builddir = argv[2]
 outfile = argv[3]
 
-execfile(conffile)
+execfile(srcdir + '/conf.py')
+
 
 roff_files = []
 rst_files = []
 out=open(outfile,'w')
 for page in man_pages:
-    rst_files = rst_files + ["doc/{0:s}.rst".format(page[0])]
+    rst_files = rst_files + ["{0:s}/{1:s}.rst".format(srcdir,page[0])]
     roff_files = roff_files + ["{0:s}/man/{1:s}.{2:d}".format(builddir,page[0],page[4])]
 
 out.write ('MAN_ROFF_FILES := ' + ' \\\n\t'.join(roff_files)+'\n')
-- 
1.9.0

[PATCH 3/3] test: use $(srcdir) instead of . as include path

[David Bremner]

This is needed for out of tree builds. The functional change is the
modification of extra_cflags; the other changes are cosmetic.
---
 test/Makefile.local | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/Makefile.local b/test/Makefile.local
index 36b1c1b..ae9db80 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -4,7 +4,7 @@ dir := test
 
 # save against changes in $(dir)
 test_src_dir := $(dir)
-extra_cflags += -I.
+extra_cflags += -I$(srcdir)
 
 smtp_dummy_srcs =		\
 	$(notmuch_compat_srcs)	\
@@ -13,10 +13,10 @@ smtp_dummy_srcs =		\
 smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 
 $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a
-	$(call quiet,CC) -I. $^ -o $@
+	$(call quiet,CC) $^ -o $@
 
 $(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
-	$(call quiet,CC) -I. $^ -o $@ -ltalloc
+	$(call quiet,CC) $^ -o $@ -ltalloc
 
 random_corpus_deps =  $(dir)/random-corpus.o  $(dir)/database-test.o \
 			notmuch-config.o command-line-arguments.o \
-- 
1.9.0

Re: out of tree build fixes

[Tomi Ollila]

On Thu, Mar 20 2014, David Bremner <david@tethera.net> wrote:

> Apparently we've neglected the out of tree builds even more than
> usual. I originally thought this is what was causing the test failures
> on buildbot. I'm less sure now, but I think these fixes don't hurt
> anything, and they do unbreak out of tree builds for me.
>
> There is certainly still work to be done with our out of tree builds.
> For those of you just joining the party, that doesn't mean we want to
> switch to your favourite build system. Or even to Autotools.
>
> An earlier draft of this series (discussed on IRC) defined a builddir
> variable, but eventually I realized the current directory of make is
> always builddir. So at least for now that doesn't seem to be needed.

LGTM. Too bad we cannot expand $(dir) in recipes (or I don't know how),
and using other variables don't bring any benefit (naming would contain
the 'doc' part and we even cannot check whether the variable is defined
or not...).

>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: out of tree build fixes

[Jani Nikula]

On Thu, 20 Mar 2014, David Bremner <david@tethera.net> wrote:
> Apparently we've neglected the out of tree builds even more than
> usual. I originally thought this is what was causing the test failures
> on buildbot. I'm less sure now, but I think these fixes don't hurt
> anything, and they do unbreak out of tree builds for me.

Did not have the time for reviewing this one, but it seems to work for
me.

Jani.

Re: out of tree build fixes

[David Bremner]

David Bremner <david@tethera.net> writes:

> Apparently we've neglected the out of tree builds even more than
> usual. I originally thought this is what was causing the test failures
> on buildbot. I'm less sure now, but I think these fixes don't hurt
> anything, and they do unbreak out of tree builds for me.

Yeah, turns out the buildbot failures are bad prereq checking in the
tests.

Pushed this (revised) series anyway.

d