Re: [PATCH v3 3/4] tools: ynl: add install target for generated content

[Donald Hunter <donald.hunter@gmail.com>]

Jan Stancek <jstancek@redhat.com> writes:

> Generate docs using ynl_gen_rst and add install target for
> headers, specs and generates rst files.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  tools/net/ynl/generated/.gitignore |  1 +
>  tools/net/ynl/generated/Makefile   | 40 +++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/tools/net/ynl/generated/.gitignore b/tools/net/ynl/generated/.gitignore
> index ade488626d26..859a6fb446e1 100644
> --- a/tools/net/ynl/generated/.gitignore
> +++ b/tools/net/ynl/generated/.gitignore
> @@ -1,2 +1,3 @@
>  *-user.c
>  *-user.h
> +*.rst
> diff --git a/tools/net/ynl/generated/Makefile b/tools/net/ynl/generated/Makefile
> index 00af721b1571..208f7fead784 100644
> --- a/tools/net/ynl/generated/Makefile
> +++ b/tools/net/ynl/generated/Makefile
> @@ -7,12 +7,19 @@ ifeq ("$(DEBUG)","1")
>    CFLAGS += -g -fsanitize=address -fsanitize=leak -static-libasan
>  endif
>  
> +INSTALL	    ?= install

nit: mix of tabs and spaces here

> +prefix      ?= /usr
> +datarootdir ?= $(prefix)/share
> +docdir      ?= $(datarootdir)/doc
> +includedir  ?= $(prefix)/include
> +
>  include ../Makefile.deps
>  
>  YNL_GEN_ARG_ethtool:=--user-header linux/ethtool_netlink.h \
>  	--exclude-op stats-get
>  
>  TOOL:=../pyynl/ynl_gen_c.py
> +TOOL_RST:=../pyynl/ynl_gen_rst.py
>  
>  GENS_PATHS=$(shell grep -nrI --files-without-match \
>  		'protocol: netlink' \
> @@ -22,7 +29,11 @@ SRCS=$(patsubst %,%-user.c,${GENS})
>  HDRS=$(patsubst %,%-user.h,${GENS})
>  OBJS=$(patsubst %,%-user.o,${GENS})
>  
> -all: protos.a $(HDRS) $(SRCS) $(KHDRS) $(KSRCS) $(UAPI)
> +SPECS_PATHS=$(wildcard ../../../../Documentation/netlink/specs/*.yaml)

You missed Jakub's request to factor out SPECS_DIR:

  Maybe factor out:

  SPECS_DIR := ../../../../Documentation/netlink/specs

  ? It's pretty long and we repeat it all over the place.

> +SPECS=$(patsubst ../../../../Documentation/netlink/specs/%.yaml,%,${SPECS_PATHS})
> +RSTS=$(patsubst %,%.rst,${SPECS})
> +
> +all: protos.a $(HDRS) $(SRCS) $(KHDRS) $(KSRCS) $(UAPI) $(RSTS)
>  
>  protos.a: $(OBJS)
>  	@echo -e "\tAR $@"
> @@ -40,8 +51,12 @@ protos.a: $(OBJS)
>  	@echo -e "\tCC $@"
>  	@$(COMPILE.c) $(CFLAGS_$*) -o $@ $<
>  
> +%.rst: ../../../../Documentation/netlink/specs/%.yaml $(TOOL2)

Did you miss Jakub's review comment: TOOL2 -> TOOL_RST ?

> +	@echo -e "\tGEN_RST $@"
> +	@$(TOOL_RST) -o $@ -i $<
> +
>  clean:
> -	rm -f *.o
> +	rm -f *.o *.rst

Also Jakub's comment:

  No strong preference but I'd count .rst as final artifacts so I'd clean
  them up in distclean target only, not the clean target. The distinction
  itself may be a local custom..

>  distclean: clean
>  	rm -f *.c *.h *.a
> @@ -49,5 +64,24 @@ distclean: clean
>  regen:
>  	@../ynl-regen.sh
>  
> -.PHONY: all clean distclean regen
> +install-headers: $(HDRS)
> +	@echo -e "\tINSTALL generated headers"
> +	@$(INSTALL) -d $(DESTDIR)$(includedir)/ynl
> +	@$(INSTALL) -m 0644 *.h $(DESTDIR)$(includedir)/ynl/
> +
> +install-rsts: $(RSTS)
> +	@echo -e "\tINSTALL generated docs"
> +	@$(INSTALL) -d $(DESTDIR)$(docdir)/ynl
> +	@$(INSTALL) -m 0644 $(RSTS) $(DESTDIR)$(docdir)/ynl/
> +
> +install-specs:
> +	@echo -e "\tINSTALL specs"
> +	@$(INSTALL) -d $(DESTDIR)$(datarootdir)/ynl
> +	@$(INSTALL) -m 0644 ../../../../Documentation/netlink/*.yaml $(DESTDIR)$(datarootdir)/ynl/
> +	@$(INSTALL) -d $(DESTDIR)$(datarootdir)/ynl/specs
> +	@$(INSTALL) -m 0644 ../../../../Documentation/netlink/specs/*.yaml $(DESTDIR)$(datarootdir)/ynl/specs/
> +
> +install: install-headers install-rsts install-specs
> +
> +.PHONY: all clean distclean regen install install-headers install-rsts install-specs
>  .DEFAULT_GOAL: all