I m good with your version. Thanks !

On Fri, Jan 26, 2024 at 5:35 PM Willy Tarreau <w...@1wt.eu> wrote:

> On Fri, Jan 26, 2024 at 04:41:36PM +0000, David Carlier wrote:
> > Hi,
> >
> > Please find the revised patch.
>
> OK thanks, it looks good and addresses the build issue.
>
> I noticed that when building with the dummy lib, we continue to link
> with -lstdc++ even if it's not used (unless DEVICEATLAS_NOCACHE=1)
> while nothing was using c++. At first I thought it was useless, but
> I spotted that there was this for the dummy lib:
>
>  OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/dacache.cpp
>
> instead of:
>
>  OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/dacache.o
>
> So I fixed it and it went fine.
>
> I'm still seeing another issue, the last patch adds a check on
> $(uname -s) to figure dependencies, which should never be used in a
> makefile since it breaks cross-compilation. From what I'm seeing, the
> offending part is:
>
> OS              := $(shell uname -s)
> ...
> ifeq ($(OS), Linux)
> LDFLAGS         += -lrt
> endif
> ifeq ($(OS), SunOS)
> LDFLAGS         += -lrt
> endif
>
> We already have this in the main makefile:
>
> ifneq ($(USE_RT),)
>   RT_LDFLAGS = -lrt
> endif
>
> And USE_RT is automatically set on Linux and Solaris targets, so normally
> you should be able to build this new check.
>
> Another point, I'm seeing that the c++ output is printed not "prettified",
> the whole c++ command line is always printed. With a small change I have
> it clean like other ones:
>
>   CXX     addons/deviceatlas/dummy/dacache.o
>   CC      addons/deviceatlas/dummy/dac.o
>   CC      addons/deviceatlas/dummy/json.o
>   CC      addons/deviceatlas/dummy/dasch.o
>   CC      addons/deviceatlas/dummy/dadwarc.o
>   CC      addons/deviceatlas/dummy/dadwcom.o
>   CC      addons/deviceatlas/dummy/dadwcurl.o
>   CC      src/version.o
>   LD      haproxy
>
> The whole diff is below, if you're OK, I'll directly apply the changes
> there instead of doing another round trip, otherwise feel free to suggest
> a different approach (I'll merge the verbose.mk change separately).
>
> Willy
>
> ---
>
> diff --git a/addons/deviceatlas/Makefile.inc
> b/addons/deviceatlas/Makefile.inc
> index 6ca2be6b3..63719bf50 100644
> --- a/addons/deviceatlas/Makefile.inc
> +++ b/addons/deviceatlas/Makefile.inc
> @@ -1,7 +1,6 @@
>  # DEVICEATLAS_SRC     : DeviceAtlas API source root path
>
>
> -OS              := $(shell uname -s)
>  CXX             := c++
>  CXXLIB          := -lstdc++
>
> @@ -21,7 +20,7 @@ OPTIONS_CFLAGS  += -DDATLAS_CURL -DDATLAS_CURLSSET
> -DDATLAS_GZ -DDATLAS_ZIP
>  OPTIONS_CFLAGS  += -I$(DEVICEATLAS_INC) -I$(CURL_INC)
>  ifeq ($(DEVICEATLAS_NOCACHE),)
>  CXXFLAGS        := $(OPTIONS_CFLAGS) -std=gnu++11
> -OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/dacache.cpp
> +OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/dacache.o
>  OPTIONS_LDFLAGS += $(CXXLIB)
>  else
>  OPTIONS_CFLAGS  += -DAPINOCACHE
> @@ -35,9 +34,5 @@ OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/dadwcurl.o
>  OPTIONS_OBJS    += $(DEVICEATLAS_SRC)/Os/daunix.o
>  endif
>
> -ifeq ($(OS), Linux)
> -LDFLAGS         += -lrt
> -endif
> -ifeq ($(OS), SunOS)
> -LDFLAGS         += -lrt
> -endif
> +addons/deviceatlas/dummy/%.o:    addons/deviceatlas/dummy/%.cpp
> +       $(cmd_CXX) $(CXXFLAGS) -c -o $@ $<
> diff --git a/include/make/verbose.mk b/include/make/verbose.mk
> index c37d513c4..9a546312d 100644
> --- a/include/make/verbose.mk
> +++ b/include/make/verbose.mk
> @@ -10,6 +10,7 @@ endif
>  # or to themselves depending on the verbosity level.
>  ifeq ($V,1)
>  cmd_CC = $(CC)
> +cmd_CXX = $(CXX)
>  cmd_LD = $(LD)
>  cmd_AR = $(AR)
>  cmd_MAKE = +$(MAKE)
> @@ -17,12 +18,14 @@ else
>  ifeq (3.81,$(firstword $(sort $(MAKE_VERSION) 3.81)))
>  # 3.81 or above
>  cmd_CC = $(info $   CC      $@) $(Q)$(CC)
> +cmd_CXX = $(info $   CXX     $@) $(Q)$(CC)
>  cmd_LD = $(info $   LD      $@) $(Q)$(LD)
>  cmd_AR = $(info $   AR      $@) $(Q)$(AR)
>  cmd_MAKE = $(info $   MAKE    $@) $(Q)+$(MAKE)
>  else
>  # 3.80 or older
>  cmd_CC = $(Q)echo "  CC      $@";$(CC)
> +cmd_CXX = $(Q)echo "  CXX     $@";$(CC)
>  cmd_LD = $(Q)echo "  LD      $@";$(LD)
>  cmd_AR = $(Q)echo "  AR      $@";$(AR)
>  cmd_MAKE = $(Q)echo "  MAKE    $@";$(MAKE)
>
>

Reply via email to