[Buildroot] [PATCH v2 0/6] gendoc infra refactoring

[Buildroot] [PATCH v2 0/6] gendoc infra refactoring

[Samuel Martin]

Hi all,

2nd round of this short series about gendoc.
The major change concerns the xsltproc workaround.

Regards,


Samuel Martin (6):
  gendoc infra: cosmetic fixes
  gendoc infra: move manual build location into $(BUILD_DIR)/manual
  gendoc infra: avoid a2x warning
  gendoc infra: disable pdf manual generation
  gendoc infra: Remove the manual* target from .PHONY
  gendoc infra: Move the manual-clean target outside of the infra

 docs/manual/manual.mk | 80 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 66 insertions(+), 14 deletions(-)

--
2.1.0

[Buildroot] [PATCH v2 1/6] gendoc infra: cosmetic fixes

[Samuel Martin]

- wrap lines to 80 characters
- fix space/tab mixup

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- more tabs/space cleanup (ThomasDS)
- fix line mess (ThomasDS)
---
 docs/manual/manual.mk | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 8d401a2..136f725 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -70,8 +70,8 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 	$$(Q)mkdir -p $$(@D)/.build
 	$$(Q)rsync -au docs/$(1)/*.txt $$(@D)/.build
 	$$(Q)a2x $(6) -f $(2) -d book -L -r $$(TOPDIR)/docs/images \
-	        -D $$(@D) $$(@D)/.build/$(1).txt \
-	        --asciidoc-opts="$$(MANUAL_$(2)_ASCIIDOC_OPTS)"
+		--asciidoc-opts="$$(MANUAL_$(2)_ASCIIDOC_OPTS)" \
+		-D $$(@D) $$(@D)/.build/$(1).txt
 	-$$(Q)rm -rf $$(@D)/.build
 endef
 
@@ -84,9 +84,12 @@ endef
 # The variable <DOCUMENT_NAME>_SOURCES defines the dependencies.
 ################################################################################
 define GENDOC
-$(call GENDOC_INNER,$(pkgname),xhtml,html,html,HTML,--xsltproc-opts "--stringparam toc.section.depth 1")
-$(call GENDOC_INNER,$(pkgname),chunked,split-html,chunked,split HTML,--xsltproc-opts "--stringparam toc.section.depth 1")
-$(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,--dblatex-opts "-P latex.output.revhistory=0")
+$(call GENDOC_INNER,$(pkgname),xhtml,html,html,HTML,\
+	--xsltproc-opts "--stringparam toc.section.depth 1")
+$(call GENDOC_INNER,$(pkgname),chunked,split-html,chunked,split HTML,\
+	--xsltproc-opts "--stringparam toc.section.depth 1")
+$(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
+	--dblatex-opts "-P latex.output.revhistory=0")
 $(call GENDOC_INNER,$(pkgname),text,text,text,text)
 $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
 clean: $(pkgname)-clean
-- 
2.1.0

[Buildroot] [PATCH v2 2/6] gendoc infra: move manual build location into $(BUILD_DIR)/manual

[Samuel Martin]

This patch reworks the mannual source preparetion by:
- moving the build directory under $(BUILD_DIR)/, this keeps consistency
  with the other Buildroot infrastructure;
- adding a couple of targets: 'manual-rsync' and 'manual-prepare-sources',
  to deal more efficiently the manual sources and avoid rsync-ing them on
  every single manual-* target.

The 'manual-rsync' target only copy the manual sources under git, while
the 'manual-prepare-sources' also takes care of the generated ones. These
targets are now run only once,  and the manual build is no longer cleaned
after each manual format generation.

Now, the 'manual-clean' target only remove the manual build directory, but
keep the output one $(O)/output/doc/manual unchanged.

Doing so (moving the manual build directory and keeping it between 2
manual format generation) ensures that all generated sources are taking
in account when generating the manual [1].

[1] http://lists.busybox.net/pipermail/buildroot/2014-August/104421.html

Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- remove trailing '\'
---
 docs/manual/manual.mk | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 136f725..044557d 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -1,11 +1,20 @@
+$(BUILD_DIR)/$(pkgname):
+	$(Q)mkdir -p $@
+
+manual-rsync: $(BUILD_DIR)/$(pkgname)
+	$(Q)$(call MESSAGE,"Preparing the manual sources...")
+	$(Q)rsync -au docs/$(pkgname)/ $(BUILD_DIR)/$(pkgname)
+
 # Packages included in BR2_EXTERNAL are not part of buildroot, so they
 # should not be included in the manual.
-manual-update-lists: manual-check-dependencies-lists
+manual-update-lists: manual-check-dependencies-lists $(BUILD_DIR)/$(pkgname)
 	$(Q)$(call MESSAGE,"Updating the manual lists...")
-	$(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \
+	$(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(BUILD_DIR)/$(pkgname) \
 		BR2_EXTERNAL=$(TOPDIR)/support/dummy-external \
 		python -B $(TOPDIR)/support/scripts/gen-manual-lists.py
 
+manual-prepare-sources: manual-rsync manual-update-lists
+
 # we can't use suitable-host-package here because that's not available in
 # the context of 'make release'
 manual-check-dependencies:
@@ -65,14 +74,13 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 			   $$($$(call UPPERCASE,$(1))_SOURCES) \
 			   manual-check-dependencies \
 			   manual-check-dependencies-$(3) \
-			   manual-update-lists
+			   manual-prepare-sources
 	$$(Q)$$(call MESSAGE,"Generating $(5) $(1)...")
-	$$(Q)mkdir -p $$(@D)/.build
-	$$(Q)rsync -au docs/$(1)/*.txt $$(@D)/.build
+	$$(Q)mkdir -p $$(@D)
 	$$(Q)a2x $(6) -f $(2) -d book -L -r $$(TOPDIR)/docs/images \
 		--asciidoc-opts="$$(MANUAL_$(2)_ASCIIDOC_OPTS)" \
-		-D $$(@D) $$(@D)/.build/$(1).txt
-	-$$(Q)rm -rf $$(@D)/.build
+		-D $$(@D) \
+		$$(BUILD_DIR)/$(1)/$(1).txt
 endef
 
 ################################################################################
@@ -94,7 +102,7 @@ $(call GENDOC_INNER,$(pkgname),text,text,text,text)
 $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
 clean: $(pkgname)-clean
 $(pkgname)-clean:
-	$$(Q)$$(RM) -rf $$(O)/docs/$(pkgname)
+	$$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
 .PHONY: $(pkgname) $(pkgname)-clean manual-update-lists
 endef
 
-- 
2.1.0

[Buildroot] [PATCH v2 3/6] gendoc infra: avoid a2x warning

[Samuel Martin]

Though the --destination-dir option works as expected, a2x displays the
following message when generating the pdf and text manual:

  a2x: WARNING: --destination-dir option is only applicable to HTML based outputs

To avoid this warning, we now just build the manual in its build location,
then move the generated files into $(O)/docs/manual.

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- remove remaining '-D $$(@D)' arguments in the a2x command line
---
 docs/manual/manual.mk | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 044557d..42d22be 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -70,6 +70,19 @@ ifneq ($$(wildcard $$(MANUAL_$(2)_ASCIIDOC_CONF)),)
 MANUAL_$(2)_ASCIIDOC_OPTS += -f $$(MANUAL_$(2)_ASCIIDOC_CONF)
 endif
 
+# Handle a2x warning about --destination-dir option only applicable to HTML
+# based outputs. So:
+# - use the --destination-dir option if possible (html and split-html),
+# - otherwise copy the generated manual to the output directory
+MANUAL_$(2)_A2X_OPTS =
+ifneq ($$(filter $(3),html split-html),)
+MANUAL_$(2)_A2X_OPTS += --destination-dir="$$(@D)"
+else
+define MANUAL_$(2)_INSTALL_CMDS
+	$$(Q)cp -f $$(BUILD_DIR)/$(1)/$(1).$(4) $$(@D)
+endef
+endif
+
 $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 			   $$($$(call UPPERCASE,$(1))_SOURCES) \
 			   manual-check-dependencies \
@@ -78,9 +91,11 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 	$$(Q)$$(call MESSAGE,"Generating $(5) $(1)...")
 	$$(Q)mkdir -p $$(@D)
 	$$(Q)a2x $(6) -f $(2) -d book -L -r $$(TOPDIR)/docs/images \
+		$$(MANUAL_$(2)_A2X_OPTS) \
 		--asciidoc-opts="$$(MANUAL_$(2)_ASCIIDOC_OPTS)" \
-		-D $$(@D) \
 		$$(BUILD_DIR)/$(1)/$(1).txt
+# install the generated manual
+	$$(MANUAL_$(2)_INSTALL_CMDS)
 endef
 
 ################################################################################
-- 
2.1.0

[Buildroot] [PATCH v2 4/6] gendoc infra: disable pdf manual generation

[Samuel Martin]

The PDF manual generation reaches the default xsltproc's template
recursion limit when processing the target package list; this makes the
PDF manual generation failed [1-3].

This limit an be raised with the '--maxvars' option. Unfortunately,
this option is not correctly handled in the latest xsltproc/libxslt
release (1.1.28), but this bug is already fixed in the libxslt
repository [4].

This patch disables the PDF manual generation (makes it failed with a
meaningful error message) when the xsltproc program found in the PATH
does not support the --maxvars option.
So, one can still generate the PDF manual if he/she extends PATH with
the location of a working xsltproc, by running:

  $ PATH=/path/to/custom-xsltproc/bin:${PATH} make manual-pdf

[1] http://lists.busybox.net/pipermail/buildroot/2014-August/104390.html
[2] http://lists.busybox.net/pipermail/buildroot/2014-August/104418.html
[3] http://lists.busybox.net/pipermail/buildroot/2014-August/104421.html
[4] https://gitorious.org/libxslt/libxslt/commit/5af7ad745323004984287e48b42712e7305de35c

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- simplify the xsltproc check logic
---
 docs/manual/manual.mk | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 42d22be..016246c 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -83,6 +83,27 @@ define MANUAL_$(2)_INSTALL_CMDS
 endef
 endif
 
+# PDF manual generation is currently broken because of a bug in xsltproc
+# program (provided by libxslt), which does not honor an option we need to set.
+# Fortunately, this bug is already fixed upstream:
+#   https://gitorious.org/libxslt/libxslt/commit/5af7ad745323004984287e48b42712e7305de35c
+#
+# So, bail out when trying to build the pdf manual using a buggy version of the
+# xsltproc program.
+#
+# So, to overcome this issue and being able to build the pdf manual, you can
+# build xsltproc from it source repository, then run:
+#   $ PATH=/path/to/custom-xsltproc/bin:${PATH} make manual
+USE_BUGGY_XSLTPROC = $$(shell xsltproc --maxvars 0 >/dev/null 2>/dev/null || echo y)
+
+ifeq ($(4)-$$(USE_BUGGY_XSLTPROC),pdf-y)
+$$(O)/docs/$(1)/$(1).$(4):
+	$$(error PDF manual generation is disabled because of a bug in \
+		xsltproc program. To be able to generate the PDF manual, you \
+		should build xsltproc from the libxslt sources >=1.1.29 and \
+		passed it to make through the command line: \
+		'PATH=/path/to/custom-xsltproc/bin:$$$${PATH} make manual-pdf')
+else
 $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 			   $$($$(call UPPERCASE,$(1))_SOURCES) \
 			   manual-check-dependencies \
@@ -96,6 +117,7 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
 		$$(BUILD_DIR)/$(1)/$(1).txt
 # install the generated manual
 	$$(MANUAL_$(2)_INSTALL_CMDS)
+endif
 endef
 
 ################################################################################
@@ -111,8 +133,11 @@ $(call GENDOC_INNER,$(pkgname),xhtml,html,html,HTML,\
 	--xsltproc-opts "--stringparam toc.section.depth 1")
 $(call GENDOC_INNER,$(pkgname),chunked,split-html,chunked,split HTML,\
 	--xsltproc-opts "--stringparam toc.section.depth 1")
+# dblatex needs to pass '--maxvars ...' option to xsltproc to prevent it from
+# reaching the template recursion limit when processing the (long) target
+# package table and bailing out.
 $(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
-	--dblatex-opts "-P latex.output.revhistory=0")
+	--dblatex-opts "-P latex.output.revhistory=0 -x '--maxvars 100000'")
 $(call GENDOC_INNER,$(pkgname),text,text,text,text)
 $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
 clean: $(pkgname)-clean
-- 
2.1.0

[Buildroot] [PATCH v2 5/6] gendoc infra: Remove the manual* target from .PHONY

[Samuel Martin]

This is not needed because none of these targets are also the name of a
generated file.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- no change
---
 docs/manual/manual.mk | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 016246c..437eccf 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -143,7 +143,6 @@ $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
 clean: $(pkgname)-clean
 $(pkgname)-clean:
 	$$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
-.PHONY: $(pkgname) $(pkgname)-clean manual-update-lists
 endef
 
 MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.txt) $(wildcard docs/images/*))
-- 
2.1.0

[Buildroot] [PATCH v2 6/6] gendoc infra: Move the manual-clean target outside of the infra

[Samuel Martin]

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1 -> v2:
- no change
---
 docs/manual/manual.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index 437eccf..74e591f 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -140,10 +140,12 @@ $(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
 	--dblatex-opts "-P latex.output.revhistory=0 -x '--maxvars 100000'")
 $(call GENDOC_INNER,$(pkgname),text,text,text,text)
 $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
+endef
+
 clean: $(pkgname)-clean
+
 $(pkgname)-clean:
-	$$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
-endef
+	$(Q)$(RM) -rf $(BUILD_DIR)/$(pkgname)
 
 MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.txt) $(wildcard docs/images/*))
 $(eval $(call GENDOC))
-- 
2.1.0

[Buildroot] [PATCH v2 1/6] gendoc infra: cosmetic fixes

[Thomas De Schampheleire]

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> - wrap lines to 80 characters
> - fix space/tab mixup
>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

[Buildroot] [PATCH v2 2/6] gendoc infra: move manual build location into $(BUILD_DIR)/manual

[Thomas De Schampheleire]

Hi Samuel,

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> This patch reworks the mannual source preparetion by:

manual
preparation

> - moving the build directory under $(BUILD_DIR)/, this keeps consistency
>   with the other Buildroot infrastructure;

infrastructures

> - adding a couple of targets: 'manual-rsync' and 'manual-prepare-sources',
>   to deal more efficiently the manual sources and avoid rsync-ing them on

_with_ the manual sources

>   every single manual-* target.
>
> The 'manual-rsync' target only copy the manual sources under git, while

copies

> the 'manual-prepare-sources' also takes care of the generated ones. These
> targets are now run only once,  and the manual build is no longer cleaned
> after each manual format generation.
>
> Now, the 'manual-clean' target only remove the manual build directory, but
> keep the output one $(O)/output/doc/manual unchanged.

keeps

>
> Doing so (moving the manual build directory and keeping it between 2
> manual format generation) ensures that all generated sources are taking

taken

> in account when generating the manual [1].
>
> [1] http://lists.busybox.net/pipermail/buildroot/2014-August/104421.html
>
> Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v1 -> v2:
> - remove trailing '\'
> ---
>  docs/manual/manual.mk | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> index 136f725..044557d 100644
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -1,11 +1,20 @@
> +$(BUILD_DIR)/$(pkgname):
> +       $(Q)mkdir -p $@
> +
> +manual-rsync: $(BUILD_DIR)/$(pkgname)
> +       $(Q)$(call MESSAGE,"Preparing the manual sources...")
> +       $(Q)rsync -au docs/$(pkgname)/ $(BUILD_DIR)/$(pkgname)

Why are you using 'rsync -au' and not 'rsync -a'?
-u means: "-u, --update : skip files that are newer on the receiver"
and I can't see why we would want that behavior?

I realize that this was already present in the original code, but
still I wonder if it's needed.

> +
>  # Packages included in BR2_EXTERNAL are not part of buildroot, so they
>  # should not be included in the manual.
> -manual-update-lists: manual-check-dependencies-lists
> +manual-update-lists: manual-check-dependencies-lists $(BUILD_DIR)/$(pkgname)
>         $(Q)$(call MESSAGE,"Updating the manual lists...")
> -       $(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \
> +       $(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(BUILD_DIR)/$(pkgname) \
>                 BR2_EXTERNAL=$(TOPDIR)/support/dummy-external \
>                 python -B $(TOPDIR)/support/scripts/gen-manual-lists.py
>
> +manual-prepare-sources: manual-rsync manual-update-lists
> +
>  # we can't use suitable-host-package here because that's not available in
>  # the context of 'make release'
>  manual-check-dependencies:
> @@ -65,14 +74,13 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
>                            $$($$(call UPPERCASE,$(1))_SOURCES) \
>                            manual-check-dependencies \
>                            manual-check-dependencies-$(3) \
> -                          manual-update-lists
> +                          manual-prepare-sources
>         $$(Q)$$(call MESSAGE,"Generating $(5) $(1)...")
> -       $$(Q)mkdir -p $$(@D)/.build
> -       $$(Q)rsync -au docs/$(1)/*.txt $$(@D)/.build
> +       $$(Q)mkdir -p $$(@D)
>         $$(Q)a2x $(6) -f $(2) -d book -L -r $$(TOPDIR)/docs/images \
>                 --asciidoc-opts="$$(MANUAL_$(2)_ASCIIDOC_OPTS)" \
> -               -D $$(@D) $$(@D)/.build/$(1).txt
> -       -$$(Q)rm -rf $$(@D)/.build
> +               -D $$(@D) \
> +               $$(BUILD_DIR)/$(1)/$(1).txt
>  endef
>
>  ################################################################################
> @@ -94,7 +102,7 @@ $(call GENDOC_INNER,$(pkgname),text,text,text,text)
>  $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
>  clean: $(pkgname)-clean
>  $(pkgname)-clean:
> -       $$(Q)$$(RM) -rf $$(O)/docs/$(pkgname)
> +       $$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
>  .PHONY: $(pkgname) $(pkgname)-clean manual-update-lists
>  endef
>

Best regards,
Thomas

[Buildroot] [PATCH v2 3/6] gendoc infra: avoid a2x warning

[Thomas De Schampheleire]

Hi Samuel,

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> Though the --destination-dir option works as expected, a2x displays the
> following message when generating the pdf and text manual:
>
>   a2x: WARNING: --destination-dir option is only applicable to HTML based outputs
>
> To avoid this warning, we now just build the manual in its build location,
> then move the generated files into $(O)/docs/manual.

What you are saying is that even though there is a warning, the option
--destination-dir is taken into account even for non-HTML based
outputs?
In that case, is it not more logical to fix the issue in a2x and
ignore the warning for now?

Besides this general comment, see below if we pursue this patch anyway...

>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v1 -> v2:
> - remove remaining '-D $$(@D)' arguments in the a2x command line
> ---
>  docs/manual/manual.mk | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> index 044557d..42d22be 100644
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -70,6 +70,19 @@ ifneq ($$(wildcard $$(MANUAL_$(2)_ASCIIDOC_CONF)),)
>  MANUAL_$(2)_ASCIIDOC_OPTS += -f $$(MANUAL_$(2)_ASCIIDOC_CONF)
>  endif
>
> +# Handle a2x warning about --destination-dir option only applicable to HTML
> +# based outputs. So:
> +# - use the --destination-dir option if possible (html and split-html),
> +# - otherwise copy the generated manual to the output directory
> +MANUAL_$(2)_A2X_OPTS =

Why do you do this? The variable will be empty by default.

> +ifneq ($$(filter $(3),html split-html),)
> +MANUAL_$(2)_A2X_OPTS += --destination-dir="$$(@D)"
> +else
> +define MANUAL_$(2)_INSTALL_CMDS
> +       $$(Q)cp -f $$(BUILD_DIR)/$(1)/$(1).$(4) $$(@D)
> +endef

Is it really needed to have this dual approach?
The patch would be simpler if we just use the manual install, even for
html manuals.


Best regards,
Thomas

[Buildroot] [PATCH v2 4/6] gendoc infra: disable pdf manual generation

[Thomas De Schampheleire]

Hi Samuel,

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> The PDF manual generation reaches the default xsltproc's template
> recursion limit when processing the target package list; this makes the
> PDF manual generation failed [1-3].

fail

>
> This limit an be raised with the '--maxvars' option. Unfortunately,

can

> this option is not correctly handled in the latest xsltproc/libxslt
> release (1.1.28), but this bug is already fixed in the libxslt
> repository [4].
>
> This patch disables the PDF manual generation (makes it failed with a

fail

> meaningful error message) when the xsltproc program found in the PATH
> does not support the --maxvars option.
> So, one can still generate the PDF manual if he/she extends PATH with
> the location of a working xsltproc, by running:
>
>   $ PATH=/path/to/custom-xsltproc/bin:${PATH} make manual-pdf
>
> [1] http://lists.busybox.net/pipermail/buildroot/2014-August/104390.html
> [2] http://lists.busybox.net/pipermail/buildroot/2014-August/104418.html
> [3] http://lists.busybox.net/pipermail/buildroot/2014-August/104421.html
> [4] https://gitorious.org/libxslt/libxslt/commit/5af7ad745323004984287e48b42712e7305de35c
>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>

I must say that the commit summary 'disable pdf manual generation' was
confusing to me: it is _conditionally_ disabled depending on the
xsltproc check.


> ---
> changes v1 -> v2:
> - simplify the xsltproc check logic
> ---
>  docs/manual/manual.mk | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> index 42d22be..016246c 100644
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -83,6 +83,27 @@ define MANUAL_$(2)_INSTALL_CMDS
>  endef
>  endif
>
> +# PDF manual generation is currently broken because of a bug in xsltproc
> +# program (provided by libxslt), which does not honor an option we need to set.
> +# Fortunately, this bug is already fixed upstream:
> +#   https://gitorious.org/libxslt/libxslt/commit/5af7ad745323004984287e48b42712e7305de35c


This message is not timeless: the 'currently' will remain in the
buildroot sources even in a few releases. I think it would be better
to rewrite this in a way that is accurate even in the future.
Something like: 'in certain versions of xsltproc', for example, or if
you know the version specify that.

> +#
> +# So, bail out when trying to build the pdf manual using a buggy version of the
> +# xsltproc program.
> +#
> +# So, to overcome this issue and being able to build the pdf manual, you can
> +# build xsltproc from it source repository, then run:
> +#   $ PATH=/path/to/custom-xsltproc/bin:${PATH} make manual
> +USE_BUGGY_XSLTPROC = $$(shell xsltproc --maxvars 0 >/dev/null 2>/dev/null || echo y)

The name 'USE_BUGGY_XSLTPROC' is confusing to me: it sounds like it
can be set when you want to build the pdf manual even though xsltproc
is buggy. What about something like: MANUAL_XSLTPROC_IS_BROKEN (I
prefixed with MANUAL to avoid a confusion with the variables of a
possible xsltproc (target) package.

> +
> +ifeq ($(4)-$$(USE_BUGGY_XSLTPROC),pdf-y)
> +$$(O)/docs/$(1)/$(1).$(4):
> +       $$(error PDF manual generation is disabled because of a bug in \
> +               xsltproc program. To be able to generate the PDF manual, you \

I would remove 'program' here.

> +               should build xsltproc from the libxslt sources >=1.1.29 and \
> +               passed it to make through the command line: \

pass

> +               'PATH=/path/to/custom-xsltproc/bin:$$$${PATH} make manual-pdf')
> +else
>  $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
>                            $$($$(call UPPERCASE,$(1))_SOURCES) \
>                            manual-check-dependencies \
> @@ -96,6 +117,7 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \
>                 $$(BUILD_DIR)/$(1)/$(1).txt
>  # install the generated manual
>         $$(MANUAL_$(2)_INSTALL_CMDS)
> +endif
>  endef
>
>  ################################################################################
> @@ -111,8 +133,11 @@ $(call GENDOC_INNER,$(pkgname),xhtml,html,html,HTML,\
>         --xsltproc-opts "--stringparam toc.section.depth 1")
>  $(call GENDOC_INNER,$(pkgname),chunked,split-html,chunked,split HTML,\
>         --xsltproc-opts "--stringparam toc.section.depth 1")
> +# dblatex needs to pass '--maxvars ...' option to xsltproc to prevent it from

the '--maxvars ...' option

> +# reaching the template recursion limit when processing the (long) target
> +# package table and bailing out.
>  $(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
> -       --dblatex-opts "-P latex.output.revhistory=0")
> +       --dblatex-opts "-P latex.output.revhistory=0 -x '--maxvars 100000'")
>  $(call GENDOC_INNER,$(pkgname),text,text,text,text)
>  $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
>  clean: $(pkgname)-clean
> --
> 2.1.0
>

Best regards,
Thomas

[Buildroot] [PATCH v2 5/6] gendoc infra: Remove the manual* target from .PHONY

[Thomas De Schampheleire]

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> This is not needed because none of these targets are also the name of a
> generated file.
>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v1 -> v2:
> - no change
> ---
>  docs/manual/manual.mk | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> index 016246c..437eccf 100644
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -143,7 +143,6 @@ $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
>  clean: $(pkgname)-clean
>  $(pkgname)-clean:
>         $$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
> -.PHONY: $(pkgname) $(pkgname)-clean manual-update-lists

I do not agree with your reasoning that because these targets are not
a file, PHONY is not needed.
The same could be said for the
.PHONY: $(1)-$(3)
line in GENDOC_INNER, and yet PHONY is not removed there.

From the make manual [1]

"There are two reasons to use a phony target: to avoid a conflict with
a file of the same name, and to improve performance.
...
Since it knows that phony targets do not name actual files that could
be remade from other files, make skips the implicit rule search for
phony targets (see Implicit Rules). This is why declaring a target
phony is good for performance, even if you are not worried about the
actual file existing"

So according to me, the PHONY declarations should remain.

In fact, all package infrastructures could use such PHONY
declarations, but they currently do not have them.

Best regards,
Thomas

[1] http://www.gnu.org/software/make/manual/make.html#Phony-Targets

[Buildroot] [PATCH v2 6/6] gendoc infra: Move the manual-clean target outside of the infra

[Thomas De Schampheleire]

On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v1 -> v2:
> - no change
> ---
>  docs/manual/manual.mk | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> index 437eccf..74e591f 100644
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -140,10 +140,12 @@ $(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
>         --dblatex-opts "-P latex.output.revhistory=0 -x '--maxvars 100000'")
>  $(call GENDOC_INNER,$(pkgname),text,text,text,text)
>  $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
> +endef
> +
>  clean: $(pkgname)-clean
> +
>  $(pkgname)-clean:
> -       $$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
> -endef
> +       $(Q)$(RM) -rf $(BUILD_DIR)/$(pkgname)
>
>  MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.txt) $(wildcard docs/images/*))
>  $(eval $(call GENDOC))
> --

This kind of defeats the purpose of having a GENDOC infra: suppose we
add a new type of document next to 'manual'. In this case, we will
have to create a new directory docs/XXXX/ and make sure that the
GENDOC infra moves to another file, say package/pkg-gendoc.mk
Then, we want XXXX-clean to exist too, which is why it should be within GENDOC.

If we don't care about such a case, then we could also simply remove
GENDOC and only keep GENDOC_INNER.
In that case we don't even need $(pkgname).

Best regards,
Thomas

[Buildroot] [PATCH v2 2/6] gendoc infra: move manual build location into $(BUILD_DIR)/manual

[Samuel Martin]

Hi Thomas,

On Fri, Aug 22, 2014 at 2:44 PM, Thomas De Schampheleire
<thomas.de.schampheleire@gmail.com> wrote:
> Hi Samuel,
>
> On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> This patch reworks the mannual source preparetion by:
>
> manual
> preparation
>
>> - moving the build directory under $(BUILD_DIR)/, this keeps consistency
>>   with the other Buildroot infrastructure;
>
> infrastructures
>
>> - adding a couple of targets: 'manual-rsync' and 'manual-prepare-sources',
>>   to deal more efficiently the manual sources and avoid rsync-ing them on
>
> _with_ the manual sources
>
>>   every single manual-* target.
>>
>> The 'manual-rsync' target only copy the manual sources under git, while
>
> copies
>
>> the 'manual-prepare-sources' also takes care of the generated ones. These
>> targets are now run only once,  and the manual build is no longer cleaned
>> after each manual format generation.
>>
>> Now, the 'manual-clean' target only remove the manual build directory, but
>> keep the output one $(O)/output/doc/manual unchanged.
>
> keeps
>
>>
>> Doing so (moving the manual build directory and keeping it between 2
>> manual format generation) ensures that all generated sources are taking
>
> taken
>
>> in account when generating the manual [1].
>>
>> [1] http://lists.busybox.net/pipermail/buildroot/2014-August/104421.html
>>
>> Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v1 -> v2:
>> - remove trailing '\'
>> ---
>>  docs/manual/manual.mk | 24 ++++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> index 136f725..044557d 100644
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -1,11 +1,20 @@
>> +$(BUILD_DIR)/$(pkgname):
>> +       $(Q)mkdir -p $@
>> +
>> +manual-rsync: $(BUILD_DIR)/$(pkgname)
>> +       $(Q)$(call MESSAGE,"Preparing the manual sources...")
>> +       $(Q)rsync -au docs/$(pkgname)/ $(BUILD_DIR)/$(pkgname)
>
> Why are you using 'rsync -au' and not 'rsync -a'?
> -u means: "-u, --update : skip files that are newer on the receiver"
> and I can't see why we would want that behavior?
>
> I realize that this was already present in the original code, but
> still I wonder if it's needed.

As you noticed, I just keep the original code. But that's true, we
certainly don't want the '-u' behavior.
I'll change this in the next submission (+ all the typoes pointed out
above ;-]).

Regards,

-- 
Samuel

[Buildroot] [PATCH v2 3/6] gendoc infra: avoid a2x warning

[Samuel Martin]

Hi Thomas, all,

On Fri, Aug 22, 2014 at 4:24 PM, Thomas De Schampheleire
<thomas.de.schampheleire@gmail.com> wrote:
> Hi Samuel,
>
> On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> Though the --destination-dir option works as expected, a2x displays the
>> following message when generating the pdf and text manual:
>>
>>   a2x: WARNING: --destination-dir option is only applicable to HTML based outputs
>>
>> To avoid this warning, we now just build the manual in its build location,
>> then move the generated files into $(O)/docs/manual.
>
> What you are saying is that even though there is a warning, the option
> --destination-dir is taken into account even for non-HTML based
> outputs?

AFAICS, here the --destination-dir option works as expected, even for
pdf and text outputs (the only difference is the warning).


> In that case, is it not more logical to fix the issue in a2x and
> ignore the warning for now?

Humm... I agree with this, but it is not really what the a2x history shows [1]:
You can see the commit [2] fixing the original issue only triggered
when generating pdf output (using dblatex backend) [3].
Then, the next commit [4] does kind of reverse the previous commit and
add the warning. But in the end, this particular option seems to work
correctly.

>
> Besides this general comment, see below if we pursue this patch anyway...
>
>>
>> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v1 -> v2:
>> - remove remaining '-D $$(@D)' arguments in the a2x command line
>> ---
>>  docs/manual/manual.mk | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> index 044557d..42d22be 100644
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -70,6 +70,19 @@ ifneq ($$(wildcard $$(MANUAL_$(2)_ASCIIDOC_CONF)),)
>>  MANUAL_$(2)_ASCIIDOC_OPTS += -f $$(MANUAL_$(2)_ASCIIDOC_CONF)
>>  endif
>>
>> +# Handle a2x warning about --destination-dir option only applicable to HTML
>> +# based outputs. So:
>> +# - use the --destination-dir option if possible (html and split-html),
>> +# - otherwise copy the generated manual to the output directory
>> +MANUAL_$(2)_A2X_OPTS =
>
> Why do you do this? The variable will be empty by default.

humm... remains... at first, I did: MANUAL_$(2)_A2X_OPTS =$(6)
I'll clean this.

>
>> +ifneq ($$(filter $(3),html split-html),)
>> +MANUAL_$(2)_A2X_OPTS += --destination-dir="$$(@D)"
>> +else
>> +define MANUAL_$(2)_INSTALL_CMDS
>> +       $$(Q)cp -f $$(BUILD_DIR)/$(1)/$(1).$(4) $$(@D)
>> +endef
>
> Is it really needed to have this dual approach?
> The patch would be simpler if we just use the manual install, even for
> html manuals.

The thing that makes me choose this dual approach is that for the html
output, there are the logo.png and a *.css file to copy as well.
Not really a big deal, but in case some images (or others resources)
are added, I'd prefer let a2x handles all the output files.

>
> Best regards,
> Thomas


Regards,

[1] https://code.google.com/r/c0710204-a2x/source/list
[2] https://code.google.com/r/c0710204-a2x/source/detail?r=531e926fd958b45ad99a5a64990df16e8bec37f3
[3] https://groups.google.com/forum/#!topic/asciidoc/le79ORSb1nU
[4] https://code.google.com/r/c0710204-a2x/source/detail?r=7c0bc8eacba98ad4f7f5bad838ef33d5801cc15d


-- 
Samuel

[Buildroot] [PATCH v2 5/6] gendoc infra: Remove the manual* target from .PHONY

[Samuel Martin]

Thomas,

On Sun, Aug 24, 2014 at 3:03 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> This is not needed because none of these targets are also the name of a
>> generated file.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v1 -> v2:
>> - no change
>> ---
>>  docs/manual/manual.mk | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> index 016246c..437eccf 100644
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -143,7 +143,6 @@ $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
>>  clean: $(pkgname)-clean
>>  $(pkgname)-clean:
>>         $$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
>> -.PHONY: $(pkgname) $(pkgname)-clean manual-update-lists
>
> I do not agree with your reasoning that because these targets are not
> a file, PHONY is not needed.
> The same could be said for the
> .PHONY: $(1)-$(3)
> line in GENDOC_INNER, and yet PHONY is not removed there.
>
> From the make manual [1]
>
> "There are two reasons to use a phony target: to avoid a conflict with
> a file of the same name, and to improve performance.
> ...
> Since it knows that phony targets do not name actual files that could
> be remade from other files, make skips the implicit rule search for
> phony targets (see Implicit Rules). This is why declaring a target
> phony is good for performance, even if you are not worried about the
> actual file existing"
>
> So according to me, the PHONY declarations should remain.
>
> In fact, all package infrastructures could use such PHONY
> declarations, but they currently do not have them.
>

Arf! Though I did read this paragraph, I miss the performance point :-s
I drop this patch from the series.

> Best regards,
> Thomas
>
> [1] http://www.gnu.org/software/make/manual/make.html#Phony-Targets

Regards,

-- 
Samuel

[Buildroot] [PATCH v2 6/6] gendoc infra: Move the manual-clean target outside of the infra

[Samuel Martin]

Thomas,

On Sun, Aug 24, 2014 at 3:06 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> On Thu, Aug 21, 2014 at 10:25 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v1 -> v2:
>> - no change
>> ---
>>  docs/manual/manual.mk | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> index 437eccf..74e591f 100644
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -140,10 +140,12 @@ $(call GENDOC_INNER,$(pkgname),pdf,pdf,pdf,PDF,\
>>         --dblatex-opts "-P latex.output.revhistory=0 -x '--maxvars 100000'")
>>  $(call GENDOC_INNER,$(pkgname),text,text,text,text)
>>  $(call GENDOC_INNER,$(pkgname),epub,epub,epub,ePUB)
>> +endef
>> +
>>  clean: $(pkgname)-clean
>> +
>>  $(pkgname)-clean:
>> -       $$(Q)$$(RM) -rf $$(BUILD_DIR)/$(pkgname)
>> -endef
>> +       $(Q)$(RM) -rf $(BUILD_DIR)/$(pkgname)
>>
>>  MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.txt) $(wildcard docs/images/*))
>>  $(eval $(call GENDOC))
>> --
>
> This kind of defeats the purpose of having a GENDOC infra: suppose we
> add a new type of document next to 'manual'. In this case, we will
> have to create a new directory docs/XXXX/ and make sure that the
> GENDOC infra moves to another file, say package/pkg-gendoc.mk
> Then, we want XXXX-clean to exist too, which is why it should be within GENDOC.

Got the point!

>
> If we don't care about such a case, then we could also simply remove
> GENDOC and only keep GENDOC_INNER.
> In that case we don't even need $(pkgname).
>

Well, I drop this patch.

> Best regards,
> Thomas

Regards,

-- 
Samuel