diff mbox series

[XEN,for-4.17,v6,5/5] tools: Rework $(xenlibs-ldlibs, ) to provide library flags only.

Message ID 20230120194431.55922-6-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show
Series Toolstack build system improvement, toward non-recursive makefiles | expand

Commit Message

Anthony PERARD Jan. 20, 2023, 7:44 p.m. UTC
LDLIBS is usually only the library flags (that is the `-l` flags), as
proposed in GNU make manual, while LDFLAGS would be extra flags such
as `-L`.

Rework the make macro $(xenlibs-ldlibs, ) to only provide the library
flags. $(xenlibs-ldflags, ) already only provide the extra flags like
the -rpath-link flags.

Also fix "test_%" recipe in libs/light as "libxenlight.so" in
$(LDLIBS_libxenlight) is been replaced by "-lxenlight". Instead of
just changing the filter, we will start using the $(xenlibs-*,) macro.
For LDFLAGS, we only needs the one for libxenlight, as the one for
toolcore and toollog are already in $(LDFLAGS), they are dependencies
to build libxenlight.so.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v6:
    - new patch

 tools/console/client/Makefile |  1 +
 tools/console/daemon/Makefile |  1 +
 tools/helpers/Makefile        |  3 +++
 tools/libs/light/Makefile     |  2 +-
 tools/Rules.mk                | 16 +++++++---------
 tools/libs/libs.mk            |  1 +
 6 files changed, 14 insertions(+), 10 deletions(-)

Comments

Jürgen Groß Feb. 8, 2023, 7:48 a.m. UTC | #1
On 20.01.23 20:44, Anthony PERARD wrote:
> LDLIBS is usually only the library flags (that is the `-l` flags), as
> proposed in GNU make manual, while LDFLAGS would be extra flags such
> as `-L`.
> 
> Rework the make macro $(xenlibs-ldlibs, ) to only provide the library
> flags. $(xenlibs-ldflags, ) already only provide the extra flags like
> the -rpath-link flags.
> 
> Also fix "test_%" recipe in libs/light as "libxenlight.so" in
> $(LDLIBS_libxenlight) is been replaced by "-lxenlight". Instead of
> just changing the filter, we will start using the $(xenlibs-*,) macro.
> For LDFLAGS, we only needs the one for libxenlight, as the one for

nit: s/needs/need/

> toolcore and toollog are already in $(LDFLAGS), they are dependencies
> to build libxenlight.so.

I don't like this very much. toolcore and toollog are used directly by
the test_% programs, so I don't think we should rely on libxenlight
depending on them.


Juergen
Anthony PERARD Feb. 8, 2023, 3:10 p.m. UTC | #2
On Wed, Feb 08, 2023 at 08:48:33AM +0100, Juergen Gross wrote:
> On 20.01.23 20:44, Anthony PERARD wrote:
> > LDLIBS is usually only the library flags (that is the `-l` flags), as
> > proposed in GNU make manual, while LDFLAGS would be extra flags such
> > as `-L`.
> > 
> > Rework the make macro $(xenlibs-ldlibs, ) to only provide the library
> > flags. $(xenlibs-ldflags, ) already only provide the extra flags like
> > the -rpath-link flags.
> > 
> > Also fix "test_%" recipe in libs/light as "libxenlight.so" in
> > $(LDLIBS_libxenlight) is been replaced by "-lxenlight". Instead of
> > just changing the filter, we will start using the $(xenlibs-*,) macro.
> > For LDFLAGS, we only needs the one for libxenlight, as the one for
> 
> nit: s/needs/need/
> 
> > toolcore and toollog are already in $(LDFLAGS), they are dependencies
> > to build libxenlight.so.
> 
> I don't like this very much. toolcore and toollog are used directly by
> the test_% programs, so I don't think we should rely on libxenlight
> depending on them.

Ok, I can add them back, it isn't much of an issue to have duplicated
flags on the command line.

Cheers,
Jürgen Groß Feb. 8, 2023, 3:20 p.m. UTC | #3
On 08.02.23 16:10, Anthony PERARD wrote:
> On Wed, Feb 08, 2023 at 08:48:33AM +0100, Juergen Gross wrote:
>> On 20.01.23 20:44, Anthony PERARD wrote:
>>> LDLIBS is usually only the library flags (that is the `-l` flags), as
>>> proposed in GNU make manual, while LDFLAGS would be extra flags such
>>> as `-L`.
>>>
>>> Rework the make macro $(xenlibs-ldlibs, ) to only provide the library
>>> flags. $(xenlibs-ldflags, ) already only provide the extra flags like
>>> the -rpath-link flags.
>>>
>>> Also fix "test_%" recipe in libs/light as "libxenlight.so" in
>>> $(LDLIBS_libxenlight) is been replaced by "-lxenlight". Instead of
>>> just changing the filter, we will start using the $(xenlibs-*,) macro.
>>> For LDFLAGS, we only needs the one for libxenlight, as the one for
>>
>> nit: s/needs/need/
>>
>>> toolcore and toollog are already in $(LDFLAGS), they are dependencies
>>> to build libxenlight.so.
>>
>> I don't like this very much. toolcore and toollog are used directly by
>> the test_% programs, so I don't think we should rely on libxenlight
>> depending on them.
> 
> Ok, I can add them back, it isn't much of an issue to have duplicated
> flags on the command line.

In that case you can add my

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen
diff mbox series

Patch

diff --git a/tools/console/client/Makefile b/tools/console/client/Makefile
index 071262c9ae..ea7819c03e 100644
--- a/tools/console/client/Makefile
+++ b/tools/console/client/Makefile
@@ -6,6 +6,7 @@  USELIBS := ctrl store
 CFLAGS += $(call xenlibs-cflags,$(USELIBS))
 CFLAGS += -include $(XEN_ROOT)/tools/config.h
 
+LDFLAGS += $(call xenlibs-ldflags,$(USELIBS))
 LDLIBS += $(call xenlibs-ldlibs,$(USELIBS))
 LDLIBS += $(SOCKET_LIBS)
 
diff --git a/tools/console/daemon/Makefile b/tools/console/daemon/Makefile
index e53c874eee..400611fc2d 100644
--- a/tools/console/daemon/Makefile
+++ b/tools/console/daemon/Makefile
@@ -7,6 +7,7 @@  CFLAGS += $(call xenlibs-cflags,$(USELIBS))
 CFLAGS-$(CONFIG_ARM) += -DCONFIG_ARM
 CFLAGS += -include $(XEN_ROOT)/tools/config.h
 
+LDFLAGS += $(call xenlibs-ldflags,$(USELIBS))
 LDLIBS += $(call xenlibs-ldlibs,$(USELIBS))
 LDLIBS += $(SOCKET_LIBS)
 LDLIBS += $(UTIL_LIBS)
diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index 0d4df01365..5db88dc81b 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -18,17 +18,20 @@  endif
 XEN_INIT_DOM0_USELIBS := ctrl toollog store light
 XEN_INIT_DOM0_OBJS = xen-init-dom0.o init-dom-json.o
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(call xenlibs-cflags,$(XEN_INIT_DOM0_USELIBS))
+xen-init-dom0: LDFLAGS += $(call xenlibs-ldflags,$(XEN_INIT_DOM0_USELIBS))
 xen-init-dom0: LDLIBS += $(call xenlibs-ldlibs,$(XEN_INIT_DOM0_USELIBS))
 
 INIT_XENSTORE_DOMAIN_USELIBS := toollog store ctrl guest light
 INIT_XENSTORE_DOMAIN_OBJS = init-xenstore-domain.o init-dom-json.o
 $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += $(call xenlibs-cflags,$(INIT_XENSTORE_DOMAIN_USELIBS))
 $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h
+init-xenstore-domain: LDFLAGS += $(call xenlibs-ldflags,$(INIT_XENSTORE_DOMAIN_USELIBS))
 init-xenstore-domain: LDLIBS += $(call xenlibs-ldlibs,$(INIT_XENSTORE_DOMAIN_USELIBS))
 
 INIT_DOM0LESS_USELIBS := ctrl evtchn toollog store light guest foreignmemory
 INIT_DOM0LESS_OBJS = init-dom0less.o init-dom-json.o
 $(INIT_DOM0LESS_OBJS): CFLAGS += $(call xenlibs-cflags,$(INIT_DOM0LESS_USELIBS))
+init-dom0less: LDFLAGS += $(call xenlibs-ldflags,$(INIT_DOM0LESS_USELIBS))
 init-dom0less: LDLIBS += $(call xenlibs-ldlibs,$(INIT_DOM0LESS_USELIBS))
 
 .PHONY: all
diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
index 96daeabc47..273f3d0864 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -244,7 +244,7 @@  libxenlight_test.so: $(PIC_OBJS) $(LIBXL_TEST_OBJS)
 	$(CC) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenlight.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS) $(APPEND_LDFLAGS)
 
 test_%: test_%.o test_common.o libxenlight_test.so
-	$(CC) $(LDFLAGS) -o $@ $^ $(filter-out %libxenlight.so, $(LDLIBS_libxenlight)) $(LDLIBS_libxentoollog) $(LDLIBS_libxentoolcore) -lyajl $(APPEND_LDFLAGS)
+	$(CC) $(LDFLAGS) $(call xenlibs-ldflags,light) -o $@ $^ $(call xenlibs-ldlibs,toollog toolcore) -lyajl $(APPEND_LDFLAGS)
 
 libxl-save-helper: $(SAVE_HELPER_OBJS) libxenlight.so
 	$(CC) $(LDFLAGS) -o $@ $(SAVE_HELPER_OBJS) $(LDLIBS_libxentoollog) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxentoolcore) $(APPEND_LDFLAGS)
diff --git a/tools/Rules.mk b/tools/Rules.mk
index d7c1f61cdf..007a64f2f5 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -105,12 +105,6 @@  define xenlibs-libs
         $(XEN_ROOT)/tools/libs/$(lib)/lib$(FILENAME_$(lib))$(libextension))
 endef
 
-# Flags for linking against all Xen libraries listed in $(1)
-define xenlibs-ldlibs
-    $(call xenlibs-rpath,$(1)) $(call xenlibs-libs,$(1)) \
-    $(foreach lib,$(1),$(xenlibs-ldlibs-$(lib)))
-endef
-
 # Provide needed flags for linking an in-tree Xen library by an external
 # project (or when it is necessary to link with "-lxen$(1)" instead of using
 # the full path to the library).
@@ -119,12 +113,16 @@  define xenlibs-ldflags
     $(foreach lib,$(1),-L$(XEN_ROOT)/tools/libs/$(lib))
 endef
 
+# Flags for linking against all Xen libraries listed in $(1)
+define xenlibs-ldlibs
+    $(foreach lib,$(1),-l$(FILENAME_$(lib)) $(xenlibs-ldlibs-$(lib)))
+endef
+
 # Flags for linking against all Xen libraries listed in $(1) but by making use
 # of -L and -l instead of providing a path to the shared library.
 define xenlibs-ldflags-ldlibs
     $(call xenlibs-ldflags,$(1)) \
-    $(foreach lib,$(1), -l$(FILENAME_$(lib))) \
-    $(foreach lib,$(1),$(xenlibs-ldlibs-$(lib)))
+    $(call xenlibs-ldlibs,$(1))
 endef
 
 define LIB_defs
@@ -132,7 +130,7 @@  define LIB_defs
  XEN_libxen$(1) = $$(XEN_ROOT)/tools/libs/$(1)
  CFLAGS_libxen$(1) = $$(CFLAGS_xeninclude)
  SHLIB_libxen$(1) = $$(call xenlibs-rpath,$(1)) -Wl,-rpath-link=$$(XEN_libxen$(1))
- LDLIBS_libxen$(1) = $$(call xenlibs-ldlibs,$(1))
+ LDLIBS_libxen$(1) = $$(call xenlibs-ldflags,$(1)) $$(call xenlibs-ldlibs,$(1))
 endef
 
 $(foreach lib,$(LIBS_LIBS),$(eval $(call LIB_defs,$(lib))))
diff --git a/tools/libs/libs.mk b/tools/libs/libs.mk
index 0e4b5e0bd0..fc6aa7ede9 100644
--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -17,6 +17,7 @@  CFLAGS   += -Wmissing-prototypes
 CFLAGS   += $(CFLAGS_xeninclude)
 CFLAGS   += $(foreach lib, $(USELIBS_$(LIBNAME)), $(CFLAGS_libxen$(lib)))
 
+LDFLAGS += $(call xenlibs-ldflags,$(USELIBS_$(LIBNAME)))
 LDLIBS += $(call xenlibs-ldlibs,$(USELIBS_$(LIBNAME)))
 
 PIC_OBJS := $(OBJS-y:.o=.opic)