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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAASaF6zxdppxDVGCNsd_h8n2-yw7P-z+D62paykBcRQU7-16eA@mail.gmail.com>
Date: Mon, 6 Jan 2025 12:35:50 +0100
From: Jan Stancek <jstancek@...hat.com>
To: Donald Hunter <donald.hunter@...il.com>
Cc: stfomichev@...il.com, kuba@...nel.org, jdamato@...tly.com, 
	pabeni@...hat.com, davem@...emloft.net, edumazet@...gle.com, horms@...nel.org, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] tools: ynl: add install target for generated content

On Mon, Dec 16, 2024 at 3:10 PM Donald Hunter <donald.hunter@...il.com> wrote:
>
> Jan Stancek <jstancek@...hat.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@...hat.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 ?

It seems I did miss his entire email. I'll double check with lore
archives before sending out v4.

Jan


>
> > +     @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
>