[PATCH] doc: Don't install emacs docs when they are not built

classic Classic list List threaded Threaded
7 messages Options
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH] doc: Don't install emacs docs when they are not built

In 40b025 we stopped building the notmuch-emacs documentation if
HAVE_EMACS=0 (i.e. no emacs was detected by configure). Unfortunately
we continued to try to install the (non-existent) documentation, which
causes build/install failures.

As a bonus, we also avoid installing the documentation if the user
configures --without-emacs.

We use '$(if )' instead of 'ifeq' here (and change ':=' to '=') to
make sure that WITH_EMACS and HAVE_EMACS are defined before we expand
INFO_TEXI_FILES.

Thanks to Ralph Seichter for reporting the problem, and testing
previous versions of this fix.
---
 doc/Makefile.local | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/Makefile.local b/doc/Makefile.local
index 719172fe..92c41b57 100644
--- a/doc/Makefile.local
+++ b/doc/Makefile.local
@@ -28,7 +28,9 @@ MAN_GZIP_FILES := $(addsuffix .gz,${MAN_ROFF_FILES})
 MAN1_TEXI := $(patsubst $(srcdir)/doc/man1/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN1_RST))
 MAN5_TEXI := $(patsubst $(srcdir)/doc/man5/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN5_RST))
 MAN7_TEXI := $(patsubst $(srcdir)/doc/man7/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN7_RST))
-INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
+INFO_TEXI_FILES = $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) \
+ $(if $(filter 11,$(HAVE_EMACS)$(WITH_EMACS)), $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi,)
+
 INFO_INFO_FILES := $(INFO_TEXI_FILES:.texi=.info)
 
 .PHONY: sphinx-html sphinx-texinfo sphinx-info
--
2.20.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] doc: Don't install emacs docs when they are not built

On Mon, Jun 10 2019, David Bremner wrote:

> In 40b025 we stopped building the notmuch-emacs documentation if
> HAVE_EMACS=0 (i.e. no emacs was detected by configure). Unfortunately
> we continued to try to install the (non-existent) documentation, which
> causes build/install failures.
>
> As a bonus, we also avoid installing the documentation if the user
> configures --without-emacs.
>
> We use '$(if )' instead of 'ifeq' here (and change ':=' to '=') to
> make sure that WITH_EMACS and HAVE_EMACS are defined before we expand
> INFO_TEXI_FILES.
>
> Thanks to Ralph Seichter for reporting the problem, and testing
> previous versions of this fix.
> ---
>  doc/Makefile.local | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/doc/Makefile.local b/doc/Makefile.local
> index 719172fe..92c41b57 100644
> --- a/doc/Makefile.local
> +++ b/doc/Makefile.local
> @@ -28,7 +28,9 @@ MAN_GZIP_FILES := $(addsuffix .gz,${MAN_ROFF_FILES})
>  MAN1_TEXI := $(patsubst $(srcdir)/doc/man1/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN1_RST))
>  MAN5_TEXI := $(patsubst $(srcdir)/doc/man5/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN5_RST))
>  MAN7_TEXI := $(patsubst $(srcdir)/doc/man7/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN7_RST))
> -INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
> +INFO_TEXI_FILES = $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) \
> + $(if $(filter 11,$(HAVE_EMACS)$(WITH_EMACS)), $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi,)
> +
>  INFO_INFO_FILES := $(INFO_TEXI_FILES:.texi=.info)

Interestingly, in your commit message you mention that changing := to =
makes a difference. I would have thought that assigning INFO_INFO_FILES
with := would have expanded INFO_TEXI_FILES just 2 lines below after
its introduction.

Anyway, it if helps, good (and change LGTM). I'm just a bit curious...

Tomi

>  
>  .PHONY: sphinx-html sphinx-texinfo sphinx-info
> --
> 2.20.1
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> https://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] doc: Don't install emacs docs when they are not built

Tomi Ollila <[hidden email]> writes:
>
> Interestingly, in your commit message you mention that changing := to =
> makes a difference. I would have thought that assigning INFO_INFO_FILES
> with := would have expanded INFO_TEXI_FILES just 2 lines below after
> its introduction.

Ah. Silly me. The non-working version had the following line in it

    $(INFO_TEXI_FILES):= $(INFO_TEXI_FILES) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi

The reason that does have _something_ do with variable expansion, just
not what I claimed ;). I think I'll go back to the ifeq version, it's more
like what we use other places.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] doc: don't build notmuch-emacs.info for configure --without-emacs

In reply to this post by Tomi Ollila-2
Since the docstrings are not built in the case of --without-emacs,
even if emacs is detected, don't let sphinx build the emacs docs. This
avoids a large number of error messages due to missing includes. It's
actually a bit surprising sphinx doesn't generate an error for the
missing include files.
---
 doc/Makefile.local | 2 +-
 doc/conf.py        | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/doc/Makefile.local b/doc/Makefile.local
index 719172fe..a88beb46 100644
--- a/doc/Makefile.local
+++ b/doc/Makefile.local
@@ -4,7 +4,7 @@ dir := doc
 
 # You can set these variables from the command line.
 SPHINXOPTS    := -q
-SPHINXBUILD   = HAVE_EMACS=${HAVE_EMACS} sphinx-build
+SPHINXBUILD   = HAVE_EMACS=${HAVE_EMACS} WITH_EMACS=${WITH_EMACS} sphinx-build
 DOCBUILDDIR      := $(dir)/_build
 
 # Internal variables.
diff --git a/doc/conf.py b/doc/conf.py
index 5f8c9f1c..8afff929 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -29,9 +29,10 @@ release = version
 # directories to ignore when looking for source files.
 exclude_patterns = ['_build']
 
-# If we don't have emacs, don't build the notmuch-emacs docs, as they need
-# emacs to generate the docstring include files
-if os.environ.get('HAVE_EMACS') != '1':
+# If we don't have emacs (or the user configured --without-emacs),
+# don't build the notmuch-emacs docs, as they need emacs to generate
+# the docstring include files
+if os.environ.get('HAVE_EMACS') != '1' or os.environ.get('WITH_EMACS') != '1':
     exclude_patterns.append('notmuch-emacs.rst')
 
 # The name of the Pygments (syntax highlighting) style to use.
--
2.20.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] doc: Don't install emacs docs when they are not built

In 40b025 we stopped building the notmuch-emacs documentation if
HAVE_EMACS=0 (i.e. no emacs was detected by configure). Unfortunately
we continued to try to install the (non-existent) documentation, which
causes build/install failures.

As a bonus, we also avoid installing the documentation if the user
configures --without-emacs.

Thanks to Ralph Seichter for reporting the problem, and testing
previous versions of this fix.
---
 doc/Makefile.local | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/doc/Makefile.local b/doc/Makefile.local
index a88beb46..d733b51e 100644
--- a/doc/Makefile.local
+++ b/doc/Makefile.local
@@ -28,7 +28,11 @@ MAN_GZIP_FILES := $(addsuffix .gz,${MAN_ROFF_FILES})
 MAN1_TEXI := $(patsubst $(srcdir)/doc/man1/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN1_RST))
 MAN5_TEXI := $(patsubst $(srcdir)/doc/man5/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN5_RST))
 MAN7_TEXI := $(patsubst $(srcdir)/doc/man7/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN7_RST))
-INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
+INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI)
+ifeq ($(HAVE_EMACS)$(WITH_EMACS),11)
+ INFO_TEXI_FILES := $(INFO_TEXI_FILES) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
+endif
+
 INFO_INFO_FILES := $(INFO_TEXI_FILES:.texi=.info)
 
 .PHONY: sphinx-html sphinx-texinfo sphinx-info
--
2.20.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] doc: Don't install emacs docs when they are not built

On Mon, Jun 10 2019, David Bremner wrote:

> In 40b025 we stopped building the notmuch-emacs documentation if
> HAVE_EMACS=0 (i.e. no emacs was detected by configure). Unfortunately
> we continued to try to install the (non-existent) documentation, which
> causes build/install failures.
>
> As a bonus, we also avoid installing the documentation if the user
> configures --without-emacs.
>
> Thanks to Ralph Seichter for reporting the problem, and testing
> previous versions of this fix.
> ---
>  doc/Makefile.local | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/doc/Makefile.local b/doc/Makefile.local
> index a88beb46..d733b51e 100644
> --- a/doc/Makefile.local
> +++ b/doc/Makefile.local
> @@ -28,7 +28,11 @@ MAN_GZIP_FILES := $(addsuffix .gz,${MAN_ROFF_FILES})
>  MAN1_TEXI := $(patsubst $(srcdir)/doc/man1/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN1_RST))
>  MAN5_TEXI := $(patsubst $(srcdir)/doc/man5/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN5_RST))
>  MAN7_TEXI := $(patsubst $(srcdir)/doc/man7/%.rst,$(DOCBUILDDIR)/texinfo/%.texi,$(MAN7_RST))
> -INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
> +INFO_TEXI_FILES := $(MAN1_TEXI) $(MAN5_TEXI) $(MAN7_TEXI)
> +ifeq ($(HAVE_EMACS)$(WITH_EMACS),11)
> + INFO_TEXI_FILES := $(INFO_TEXI_FILES) $(DOCBUILDDIR)/texinfo/notmuch-emacs.texi
> +endif

LGTM. I would have suggested += if this weren't going to be temporary
solution :D

Tomi

> +
>  INFO_INFO_FILES := $(INFO_TEXI_FILES:.texi=.info)
>  
>  .PHONY: sphinx-html sphinx-texinfo sphinx-info
> --
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] doc: Don't install emacs docs when they are not built

In reply to this post by David Bremner-2
David Bremner <[hidden email]> writes:

> In 40b025 we stopped building the notmuch-emacs documentation if
> HAVE_EMACS=0 (i.e. no emacs was detected by configure). Unfortunately
> we continued to try to install the (non-existent) documentation, which
> causes build/install failures.
>
> As a bonus, we also avoid installing the documentation if the user
> configures --without-emacs.

I've pushed this series as a new point release, and merged to master.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch