[v4,1/5] build/doc: Ensure destination dir exists despite VPATH

Submitted by Daniel Stone on Aug. 28, 2018, 10:19 p.m.

Details

Message ID 20180828221919.18208-1-daniels@collabora.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Daniel Stone Aug. 28, 2018, 10:19 p.m.
Make considers a variable called VPATH when trying to satisfy
dependencies, e.g. for a target 'foo', it will consider the target
extant if VPATH is '../../bar' and '../../bar/foo' exists.

Part of the doc build, the '$(alldirs)' target, exists to create the
target directories if they do not exist. For example, before generating
xml/wayland-architecture.png, it will ensure the 'xml' target is
considered up-to-date thanks to the target dependency.

Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency
will be satisfied, so we'll never create the output directory, and the
doc build will fail.

Change the alldirs target list to be absolute paths, so VPATH will not
be consulted and defeat the entire point of what we're trying to do.
This fixes the Meson build, where we later create
doc/doxygen/xml/meson.build.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 doc/doxygen/Makefile.am | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
index f8b0b3aa..31d953c0 100644
--- a/doc/doxygen/Makefile.am
+++ b/doc/doxygen/Makefile.am
@@ -53,20 +53,21 @@  diagram_maps := $(patsubst $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map))
 dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3" -printf "man/man3/%P\n")
 
 # Listing various directories that might need to be created.
-alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server
+alldirsrel := xml xml/Client xml/Server man/man3 html/Client html/Server
+alldirs := $(patsubst %,$(CURDIR)/%,$(alldirsrel))
 
 $(diagrams): $(diagramssrc)
 
 $(diagram_maps):  $(diagramssrc)
 
-xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | xml/%
+xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | $(CURDIR)/xml/%
 	$(AM_V_GEN)(cat wayland.doxygen; \
           echo "GENERATE_XML=YES"; \
           echo "XML_OUTPUT=xml/$*"; \
           echo "INPUT= $(scanned_src_files_$*)"; \
           ) | $(DOXYGEN) -
 
-html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | html/%
+html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | $(CURDIR)/html/%
 	$(AM_V_GEN)(cat wayland.doxygen; \
           echo "PROJECT_NAME=\"Wayland $* API\""; \
           echo "GENERATE_HTML=YES"; \
@@ -74,7 +75,7 @@  html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_
           echo "INPUT= $(scanned_src_files_$*) $(extra_doxygen_$*)"; \
           ) | $(DOXYGEN) -
 
-man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayland.doxygen | man/man3
+man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayland.doxygen | $(CURDIR)/man/man3
 	$(AM_V_GEN)(cat wayland.doxygen; \
           echo "GENERATE_MAN=YES"; \
           echo "MAN_OUTPUT=man"; \
@@ -82,10 +83,10 @@  man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayl
           echo "INPUT= $(scanned_src_files_man)"; \
           ) | $(DOXYGEN) -
 
-xml/%.png: $(diagramsdir)/%.gv | xml
+xml/%.png: $(diagramsdir)/%.gv | $(CURDIR)/xml
 	$(AM_V_GEN)$(DOT) -Tpng -o$@ $<
 
-xml/%.map: $(diagramsdir)/%.gv | xml
+xml/%.map: $(diagramsdir)/%.gv | $(CURDIR)/xml
 	$(AM_V_GEN)$(DOT) -Tcmapx_np -o$@ $<
 
 # general rule to create one of the listed directories.

Comments

Pekka Paalanen Aug. 29, 2018, 8:57 a.m.
On Tue, 28 Aug 2018 23:19:15 +0100
Daniel Stone <daniels@collabora.com> wrote:

> Make considers a variable called VPATH when trying to satisfy
> dependencies, e.g. for a target 'foo', it will consider the target
> extant if VPATH is '../../bar' and '../../bar/foo' exists.
> 
> Part of the doc build, the '$(alldirs)' target, exists to create the
> target directories if they do not exist. For example, before generating
> xml/wayland-architecture.png, it will ensure the 'xml' target is
> considered up-to-date thanks to the target dependency.
> 
> Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency
> will be satisfied, so we'll never create the output directory, and the
> doc build will fail.
> 
> Change the alldirs target list to be absolute paths, so VPATH will not
> be consulted and defeat the entire point of what we're trying to do.
> This fixes the Meson build, where we later create
> doc/doxygen/xml/meson.build.
> 
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  doc/doxygen/Makefile.am | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

Hi Daniel,

this makes sense to me, so even though I didn't test:

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>


Thanks,
pq

> 
> diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
> index f8b0b3aa..31d953c0 100644
> --- a/doc/doxygen/Makefile.am
> +++ b/doc/doxygen/Makefile.am
> @@ -53,20 +53,21 @@ diagram_maps := $(patsubst $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map))
>  dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3" -printf "man/man3/%P\n")
>  
>  # Listing various directories that might need to be created.
> -alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server
> +alldirsrel := xml xml/Client xml/Server man/man3 html/Client html/Server
> +alldirs := $(patsubst %,$(CURDIR)/%,$(alldirsrel))
>  
>  $(diagrams): $(diagramssrc)
>  
>  $(diagram_maps):  $(diagramssrc)
>  
> -xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | xml/%
> +xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | $(CURDIR)/xml/%
>  	$(AM_V_GEN)(cat wayland.doxygen; \
>            echo "GENERATE_XML=YES"; \
>            echo "XML_OUTPUT=xml/$*"; \
>            echo "INPUT= $(scanned_src_files_$*)"; \
>            ) | $(DOXYGEN) -
>  
> -html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | html/%
> +html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_maps) | $(CURDIR)/html/%
>  	$(AM_V_GEN)(cat wayland.doxygen; \
>            echo "PROJECT_NAME=\"Wayland $* API\""; \
>            echo "GENERATE_HTML=YES"; \
> @@ -74,7 +75,7 @@ html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) $(diagram_
>            echo "INPUT= $(scanned_src_files_$*) $(extra_doxygen_$*)"; \
>            ) | $(DOXYGEN) -
>  
> -man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayland.doxygen | man/man3
> +man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayland.doxygen | $(CURDIR)/man/man3
>  	$(AM_V_GEN)(cat wayland.doxygen; \
>            echo "GENERATE_MAN=YES"; \
>            echo "MAN_OUTPUT=man"; \
> @@ -82,10 +83,10 @@ man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) wayl
>            echo "INPUT= $(scanned_src_files_man)"; \
>            ) | $(DOXYGEN) -
>  
> -xml/%.png: $(diagramsdir)/%.gv | xml
> +xml/%.png: $(diagramsdir)/%.gv | $(CURDIR)/xml
>  	$(AM_V_GEN)$(DOT) -Tpng -o$@ $<
>  
> -xml/%.map: $(diagramsdir)/%.gv | xml
> +xml/%.map: $(diagramsdir)/%.gv | $(CURDIR)/xml
>  	$(AM_V_GEN)$(DOT) -Tcmapx_np -o$@ $<
>  
>  # general rule to create one of the listed directories.
Emil Velikov Aug. 29, 2018, 4:46 p.m.
Hi Dan,

On 28 August 2018 at 23:19, Daniel Stone <daniels@collabora.com> wrote:
> Make considers a variable called VPATH when trying to satisfy
> dependencies, e.g. for a target 'foo', it will consider the target
> extant if VPATH is '../../bar' and '../../bar/foo' exists.
>
> Part of the doc build, the '$(alldirs)' target, exists to create the
> target directories if they do not exist. For example, before generating
> xml/wayland-architecture.png, it will ensure the 'xml' target is
> considered up-to-date thanks to the target dependency.
>
> Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency
> will be satisfied, so we'll never create the output directory, and the
> doc build will fail.
>
> Change the alldirs target list to be absolute paths, so VPATH will not
> be consulted and defeat the entire point of what we're trying to do.
> This fixes the Meson build, where we later create
> doc/doxygen/xml/meson.build.
>
Have you tried something as trivial as the below sed?
s/$(AM_V_GEN)/$(AM_V_GEN)$(MKDIR_P) $@/g

It will allow you to remove the, dare I say it, bonkers "let's make a
target that only creates a folder".
Plus avoid all the complexity that you've proposing.

-Emil
Daniel Stone Aug. 30, 2018, 1:02 p.m.
On Wed, 29 Aug 2018 at 17:46, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 28 August 2018 at 23:19, Daniel Stone <daniels@collabora.com> wrote:
> > Change the alldirs target list to be absolute paths, so VPATH will not
> > be consulted and defeat the entire point of what we're trying to do.
> > This fixes the Meson build, where we later create
> > doc/doxygen/xml/meson.build.
> >
> Have you tried something as trivial as the below sed?
> s/$(AM_V_GEN)/$(AM_V_GEN)$(MKDIR_P) $@/g

I assume you mean '$(dir $@)' here.

> It will allow you to remove the, dare I say it, bonkers "let's make a
> target that only creates a folder".
> Plus avoid all the complexity that you've proposing.

Sure. I was trying to work within what was already there rather than
rewrite the Makefile, given how fragile the entire thing is, but I've
now rewritten the commit.

Cheers,
Daniel