ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
63 stars 59 forks source link

build failure in zkey when distro overrides the compiler flags #108

Closed sharkcz closed 3 years ago

sharkcz commented 3 years ago

I get this error when building version 2.16.0 in Fedora

gcc -I ../include -D_GNU_SOURCE -DHAVE_LUKS2_SUPPORT -DS390_TOOLS_RELEASE=2.16.0-1.fc35 -DS390_TOOLS_LIBDIR=/lib/s390-tools -DS390_TOOLS_DATADIR=/usr/share/s390-tools -DS390_TOOLS_SYSCONFDIR=/etc -DS390_TOOLS_BINDIR=/usr/sbin -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=zEC12 -mtune=z13 -fasynchronous-unwind-tables -fstack-clash-protection -c kms.c -o kms.o
kms.c:44:2: error: #error KMS_PLUGIN_LOCATION must be defined
   44 | #error KMS_PLUGIN_LOCATION must be defined
      |  ^~~~~
kms.c: In function 'load_kms_plugin':
kms.c:274:5: error: 'KMS_PLUGIN_LOCATION' undeclared (first use in this function)
  274 |     KMS_PLUGIN_LOCATION, so_name);
      |     ^~~~~~~~~~~~~~~~~~~
kms.c:274:5: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [../common.mak:253: kms.o] Error 1
make[1]: Leaving directory '/home/sharkcz/s390utils/s390-tools-2.16.0/zkey'

I think the root cause is that zkey uses CFLAGS variable for defining the KMS_PLUGIN_LOCATION in https://github.com/ibm-s390-linux/s390-tools/blob/master/zkey/Makefile#L37 while it should use ALL_CFLAGS or similar.

ifranzki commented 3 years ago

Thanks for reporting.

Does the following patch fix this?

From 91185d2c91fe853f9b3373002f87e938c1af701f Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Tue, 23 Feb 2021 08:52:26 +0100
Subject: [PATCH] zkey: Fix build error when when the compiler flags are
 overridden

When the compiler flags are overridden, the build of zkey may fail with:

kms.c:44:2: error: #error KMS_PLUGIN_LOCATION must be defined
   44 | #error KMS_PLUGIN_LOCATION must be defined
      |  ^~~~~

The Makefile uses CFLAGS variable for defining the KMS_PLUGIN_LOCATION,
but it should rather use ALL_CFLAGS.

Closes: https://github.com/ibm-s390-linux/s390-tools/issues/108

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 zkey/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zkey/Makefile b/zkey/Makefile
index b28754824..5c878e1cb 100644
--- a/zkey/Makefile
+++ b/zkey/Makefile
@@ -36,7 +36,7 @@ endif

 libs = $(rootdir)/libutil/libutil.a

-CFLAGS += -DKMS_PLUGIN_LOCATION=\"$(ZKEYKMSPLUGINDIR)\"
+ALL_CFLAGS += -DKMS_PLUGIN_LOCATION=\"$(ZKEYKMSPLUGINDIR)\"

 detect-libcryptsetup.dep:
    echo "#include <libcryptsetup.h>" > detect-libcryptsetup.dep
-- 
sharkcz commented 3 years ago

yes, it does and I think CPPFLAGS from https://github.com/ibm-s390-linux/s390-tools/blob/master/zkey/Makefile#L21 should be changed to ALL_CPPFLAGS too

ifranzki commented 3 years ago

Yes right. Actually it should be ALL_CFLAGS, not ALL_CPPFLAGS, since we don't have C++ there anyway.

Updated patch:

From 50b1607a63fd6850f28ac3962b2291336655a463 Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Tue, 23 Feb 2021 08:52:26 +0100
Subject: [PATCH] zkey: Fix build error when when the compiler flags are
 overridden

When the compiler flags are overridden, the build of zkey may fail with:

kms.c:44:2: error: #error KMS_PLUGIN_LOCATION must be defined
   44 | #error KMS_PLUGIN_LOCATION must be defined
      |  ^~~~~

The Makefile uses CFLAGS variable for defining the KMS_PLUGIN_LOCATION,
but it should rather use ALL_CFLAGS.

Also use ALL_CFLAGS for defining HAVE_LUKS2_SUPPORT.

Closes: https://github.com/ibm-s390-linux/s390-tools/issues/108

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 zkey/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zkey/Makefile b/zkey/Makefile
index b28754824..dd051690d 100644
--- a/zkey/Makefile
+++ b/zkey/Makefile
@@ -20,7 +20,7 @@ ifneq (${HAVE_CRYPTSETUP2},0)
        ifneq (${HAVE_OPENSSL},0)
            BUILD_TARGETS += zkey-cryptsetup
            INSTALL_TARGETS += install-zkey-cryptsetup
-           CPPFLAGS += -DHAVE_LUKS2_SUPPORT
+           ALL_CFLAGS += -DHAVE_LUKS2_SUPPORT
        else
            BUILD_TARGETS += zkey-cryptsetup-skip-openssl
            INSTALL_TARGETS += zkey-cryptsetup-skip-openssl
@@ -36,7 +36,7 @@ endif

 libs = $(rootdir)/libutil/libutil.a

-CFLAGS += -DKMS_PLUGIN_LOCATION=\"$(ZKEYKMSPLUGINDIR)\"
+ALL_CFLAGS += -DKMS_PLUGIN_LOCATION=\"$(ZKEYKMSPLUGINDIR)\"

 detect-libcryptsetup.dep:
    echo "#include <libcryptsetup.h>" > detect-libcryptsetup.dep
-- 
sharkcz commented 3 years ago

CPPFLAGS as preprocessor flags, which is correct here, for C++ compiler there would be CXXFLAGS

ifranzki commented 3 years ago

Ah, right, will use ALL_CPPFLAGS then.

sharkcz commented 3 years ago

thanks :-)