tools/libs: fix build rules to correctly deal with multiple public headers

[PATCH 0/2] tools/libs: fix build rules to correctly deal with multiple public headers

Posted by Jan Beulich 3 years, 6 months ago

1: fix header symlinking rule
2: fix uninstall rule for header files

Jan

Re: [PATCH 0/2] tools/libs: fix build rules to correctly deal with multiple public headers

Posted by Jan Beulich 3 years, 6 months ago

On 19.10.2020 09:19, Jan Beulich wrote:
> 1: fix header symlinking rule
> 2: fix uninstall rule for header files

Actually I've noticed these issues were introduced relatively
recently only, in particular after 4.14. I've added

Fixes: bc44e2fb3199 ("tools: add a copy of library headers in tools/include")

to both of them, albeit with the above they won't need even
just considering of backport.

Jan

[PATCH 1/2] tools/libs: fix header symlinking rule

Posted by Jan Beulich 3 years, 6 months ago

Unlike pattern rules, ordinary rules with multiple targets have their
commands executed once per target. Hence when $(LIBHEADERS) expands to
more than just one item, multiple identical commands would have been
issued, which have been observed to cause build failures relatively
frequently after libx{c,l} code was moved to tools/libs/{ctrl,light}/.
Use a static pattern rule instead.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm aware Jürgen has a series pending to entirely remove the rule in
question, but this being an isolated fix which ought to be easier to
review, I thought I'd still post it. Re-basing his series over this
change should be straightforward.
However, for the above reason I'm not bothering getting right the
theoretical case of headers in subdirs of the respective include/ being
mentioned in $(LIBHEADER).

--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -79,8 +79,8 @@ headers.chk: $(LIBHEADERSGLOB) $(AUTOINC
 libxen$(LIBNAME).map:
 	echo 'VERS_$(MAJOR).$(MINOR) { global: *; };' >$@
 
-$(LIBHEADERSGLOB): $(LIBHEADERS)
-	for i in $(realpath $(LIBHEADERS)); do ln -sf $$i $(XEN_ROOT)/tools/include; done
+$(LIBHEADERSGLOB): $(XEN_ROOT)/tools/include/%.h: include/%.h
+	ln -sf $(CURDIR)/$< $(XEN_ROOT)/tools/include/
 
 lib$(LIB_FILE_NAME).a: $(LIB_OBJS)
 	$(AR) rc $@ $^

[PATCH 2/2] tools/libs: fix uninstall rule for header files

Posted by Jan Beulich 3 years, 6 months ago

This again was working right only as long as $(LIBHEADER) consisted of
just one entry.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
An alternative would be to use $(addprefix ) without any shell loop.

--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -107,7 +107,7 @@ install: build
 .PHONY: uninstall
 uninstall:
 	rm -f $(DESTDIR)$(PKG_INSTALLDIR)/$(LIB_FILE_NAME).pc
-	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$(LIBHEADER); done
+	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$$i; done
 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so
 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR)
 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)

Re: [PATCH 2/2] tools/libs: fix uninstall rule for header files

Posted by Bertrand Marquis 3 years, 6 months ago


> On 19 Oct 2020, at 08:21, Jan Beulich <jbeulich@suse.com> wrote:
> 
> This again was working right only as long as $(LIBHEADER) consisted of
> just one entry.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

The change is obviously fixing a bug :-) and the double $ is required to protect from make.

Cheers
Bertrand


> ---
> An alternative would be to use $(addprefix ) without any shell loop.
> 
> --- a/tools/libs/libs.mk
> +++ b/tools/libs/libs.mk
> @@ -107,7 +107,7 @@ install: build
> .PHONY: uninstall
> uninstall:
> 	rm -f $(DESTDIR)$(PKG_INSTALLDIR)/$(LIB_FILE_NAME).pc
> -	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$(LIBHEADER); done
> +	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$$i; done
> 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so
> 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR)
> 	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)
> 
>

Ping²: [PATCH 2/2] tools/libs: fix uninstall rule for header files

Posted by Jan Beulich 3 years, 5 months ago

On 29.10.2020 16:24, Bertrand Marquis wrote:
>> On 19 Oct 2020, at 08:21, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> This again was working right only as long as $(LIBHEADER) consisted of
>> just one entry.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> The change is obviously fixing a bug :-) and the double $ is required to protect from make.

I'll give it a day or two more to get an ack (or any negative
form of feedback), but I guess I'll go ahead and commit this
with just Bertrand's R-b otherwise.

Jan

Ping: [PATCH 2/2] tools/libs: fix uninstall rule for header files

Posted by Jan Beulich 3 years, 6 months ago

On 19.10.2020 09:21, Jan Beulich wrote:
> This again was working right only as long as $(LIBHEADER) consisted of
> just one entry.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

While patch 1 has become irrelevant with Juergen's more complete
change, this one is still applicable afaict.

Jan

> ---
> An alternative would be to use $(addprefix ) without any shell loop.
> 
> --- a/tools/libs/libs.mk
> +++ b/tools/libs/libs.mk
> @@ -107,7 +107,7 @@ install: build
>  .PHONY: uninstall
>  uninstall:
>  	rm -f $(DESTDIR)$(PKG_INSTALLDIR)/$(LIB_FILE_NAME).pc
> -	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$(LIBHEADER); done
> +	for i in $(LIBHEADER); do rm -f $(DESTDIR)$(includedir)/$$i; done
>  	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so
>  	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR)
>  	rm -f $(DESTDIR)$(libdir)/lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)
> 
>