| Message ID | 20211125134006.1076646-31-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 25.11.2021 14:39, Anthony PERARD wrote: > @@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH)) > > all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk > > -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y)) > +public-hdrs-path := $(srcdir)/public > > -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h > -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS)) > +public-list-headers = $(wildcard $1/*.h $1/*/*.h) > +public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1)) > + > +public-c99-headers := io/9pfs.h io/pvcalls.h > +public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y) > +public-ansi-headers := $(public-headers) Personally I find it odd for public-c99-headers to be first in this group. Further down, in the upper-case counterparts, you have it at the end (still in context below). > +public-headers-filter := dom0_ops.h arch-% What is the criteria to be listed here vs ... > +public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers) ... outside of that macro's expansion here? There's no other use of the macro that I can spot, so its presence is puzzling me. > + > +PUBLIC_HEADERS := $(call public-filter-headers,public-headers) > +PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers) > +PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers)) While benign right now, wouldn't it be more consistent if public-filter-headers was also applied in this last case? Or is there a reason not to? Jan
On Tue, Dec 21, 2021 at 03:07:52PM +0100, Jan Beulich wrote: > On 25.11.2021 14:39, Anthony PERARD wrote: > > @@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH)) > > > > all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk > > > > -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y)) > > +public-hdrs-path := $(srcdir)/public > > > > -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h > > -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS)) > > +public-list-headers = $(wildcard $1/*.h $1/*/*.h) > > +public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1)) > > + > > +public-c99-headers := io/9pfs.h io/pvcalls.h > > +public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y) > > +public-ansi-headers := $(public-headers) > > Personally I find it odd for public-c99-headers to be first in this group. > Further down, in the upper-case counterparts, you have it at the end (still > in context below). > > > +public-headers-filter := dom0_ops.h arch-% > > What is the criteria to be listed here vs ... > > > +public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers) > > ... outside of that macro's expansion here? There's no other use of the > macro that I can spot, so its presence is puzzling me. The "%-filter" macro are used by the macro "public-filter-headers", which takes "$(%)" and filter-out "$(%-filter)". Currently, PUBLIC_ANSI_HEADERS is PUBLIC_HEADERS with headers filtered-out. And PUBLIC_HEADERS is public-y with headers filtered-out. In my patch, I derive PUBLIC_ANSI_HEADERS from public-y, so I also need to use the filter used to generate PUBLIC_HEADERS. So $(public-headers-filter) is both used on its own and in the ansi-filter. > > +PUBLIC_HEADERS := $(call public-filter-headers,public-headers) > > +PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers) > > +PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers)) > > While benign right now, wouldn't it be more consistent if > public-filter-headers was also applied in this last case? Or is > there a reason not to? It wouldn't work at the moment because $(public-c99-headers) aren't prefix with "$(public-hdrs-path)" like $(public-headers). This is because I'm using $(public-c99-headers) in $(public-ansi-headers-filter). Maybe I should just had the prefix to the c99-headers and removing the prefix when using it in the filter, something like that: public-c99-headers := $(addprefix $(public-hdrs-path)/,io/9pfs.h io/pvcalls.h) public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) \ $(patsubst $(public-hdrs-path)/%,%,$(public-c99-headers)) then I thin that would work: PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers) I could put an empty "public-c99-headers-filter :=" somewhere, just in case one is looking for it later. Thanks,
diff --git a/xen/include/Makefile b/xen/include/Makefile index 5a2b4c9f65fa..6e80ef276fd9 100644 --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -39,8 +39,8 @@ cppflags-$(CONFIG_X86) += -m32 endif -public-$(CONFIG_X86) := $(wildcard $(src)/public/arch-x86/*.h $(src)/public/arch-x86/*/*.h) -public-$(CONFIG_ARM) := $(wildcard $(src)/public/arch-arm/*.h $(src)/public/arch-arm/*/*.h) +public-$(CONFIG_X86) := $(wildcard $(srcdir)/public/arch-x86/*.h $(srcdir)/public/arch-x86/*/*.h) +public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/public/arch-arm/*/*.h) .PHONY: all all: $(addprefix $(obj)/,$(headers-y)) @@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH)) all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y)) +public-hdrs-path := $(srcdir)/public -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS)) +public-list-headers = $(wildcard $1/*.h $1/*/*.h) +public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1)) + +public-c99-headers := io/9pfs.h io/pvcalls.h +public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y) +public-ansi-headers := $(public-headers) + +public-headers-filter := dom0_ops.h arch-% +public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers) + +PUBLIC_HEADERS := $(call public-filter-headers,public-headers) +PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers) +PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers)) $(src)/public/io/9pfs.h-prereq := string $(src)/public/io/pvcalls.h-prereq := string
Listing public headers when out-of-tree build are involved becomes more annoying where every path to every headers needs to start with "$(srctree)/$(src)", or $(wildcard ) will not work. This means more repetition. ( "$(srcdir)" is a shortcut for "$(srctree)/$(src)" ) This patch attempt to reduce the amount of duplication and make better use of make's meta programming capability. The filters are now listed in a variable and don't have to repeat the path to the headers files as this is added later as needed. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v8: - add prefix "public-" to newly introduced macros. - make use of the new "$(srcdir)" shortcut. xen/include/Makefile | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)