ibm-s390-linux / s390-tools

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

should not use lib64 on distros that don't use that - please support configuring LIBDIR or libexec #98

Closed xnox closed 3 years ago

xnox commented 3 years ago

hardcoding /usr/lib64 is not nice. On many systems nothing uses /usr/lib64 anymore.

Please either use $(LIBDIR) (for shared libraries) or /usr/libexec (for plugins).

For example on Debian/Ubuntu, the multiarch locations are used for libdir such as /usr/lib/s390x-linux-gnu

/usr/libexec is nowadays commonly used as the location for dynamically loaded plugins.

Let's change /usr/lib64/zkey/zkey-ekmfweb.so to somthing more standard and modern. /usr/libexec would be a much nicer location for it, which a lot of applications use for plugins.

ifranzki commented 3 years ago

Installing the plugin (zkey-ekmfweb.so) into /usr/libexec (actually /usr/libexec/zkey/ I guess) is fine with me.

Regarding installing shared libraries into $(LIBDIR): I guess you are talking about libekmfweb.so(.*) ? What exactly do you mean by 'use $(LIBDIR)' ? LIBDIR is defined in common.mak as '$(INSTALLDIR)/lib'. The default INSTALLDIR is empty, so it would be installed to /lib. Unfortunately /lib is not in LD_LIBRARY_PATH, at least not on my system, so it won't find libekmfweb.so.

xnox commented 3 years ago

Installing the plugin (zkey-ekmfweb.so) into /usr/libexec (actually /usr/libexec/zkey/ I guess) is fine with me.

Ack, yeap.

Regarding installing shared libraries into $(LIBDIR): I guess you are talking about libekmfweb.so(.*) ? What exactly do you mean by 'use $(LIBDIR)' ? LIBDIR is defined in common.mak as '$(INSTALLDIR)/lib'. The default INSTALLDIR is empty, so it would be installed to /lib. Unfortunately /lib is not in LD_LIBRARY_PATH, at least not on my system, so it won't find libekmfweb.so.

So, with meson/autoconf/make etc. buildsystems debian build system would pass /usr/lib/s390x-linux-gnu which is in the ld.so searchpath. And rpmmacros would pass $rmproot/usr/lib64 as appropriate.

What is not ok, to hardcode "/lib64" which cannot be influenced from the buildsystem. Since LIBDIR variable can be passed to make invocation.

I'll try to submit patches for both issues above.

ifranzki commented 3 years ago

I do have a patch for the first part. If you could come up with a patch for the second part (that still allows us to run zkey after a local make install) that would be great.

From cb80019d3a034bb5a6893d2f1876ecc57afbd77c Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Mon, 16 Nov 2020 14:38:41 +0100
Subject: [PATCH] zkey/zkey-ekmfweb: Install KMS plugins into /usr/libexec/zkey

Install KMS plugins into /usr/libexec/zkey/ instead of /usr/lib64/zkey,
since plugins should rather go to /usr/libexec.

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

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 common.mak            | 3 ++-
 zkey/ekmfweb/Makefile | 6 +++---
 zkey/kms.c            | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/common.mak b/common.mak
index ae6208d2f..c8713cd4b 100644
--- a/common.mak
+++ b/common.mak
@@ -164,6 +164,7 @@ USRBINDIR       = $(INSTALLDIR)/usr/bin
 BINDIR          = $(INSTALLDIR)/sbin
 LIBDIR          = $(INSTALLDIR)/lib
 USRLIB64DIR     = $(INSTALLDIR)/usr/lib64
+USRLIBEXECDIR   = $(INSTALLDIR)/usr/libexec
 SYSCONFDIR      = $(INSTALLDIR)/etc
 MANDIR          = $(INSTALLDIR)/usr/share/man
 VARDIR          = $(INSTALLDIR)/var
@@ -179,7 +180,7 @@ INSTDIRS        = $(USRSBINDIR) $(USRBINDIR) $(BINDIR) $(LIBDIR) $(MANDIR) \
            $(SYSCONFDIR) $(SYSCONFDIR)/sysconfig \
            $(TOOLS_LIBDIR) $(TOOLS_DATADIR) \
            $(ZFCPDUMP_DIR) $(SYSTEMDSYSTEMUNITDIR) \
-           $(USRLIB64DIR) $(USRINCLUDEDIR)
+           $(USRLIB64DIR) $(USRINCLUDEDIR) $(USRLIBEXECDIR)
 OWNER           = $(shell id -un)
 GROUP      = $(shell id -gn)
 export INSTALLDIR BINDIR LIBDIR USRLIB64DIR MANDIR OWNER GROUP
diff --git a/zkey/ekmfweb/Makefile b/zkey/ekmfweb/Makefile
index 1a3a49775..8e48d26af 100644
--- a/zkey/ekmfweb/Makefile
+++ b/zkey/ekmfweb/Makefile
@@ -44,9 +44,9 @@ libekmfweb.dep:
 install: all install-libekmfweb.dep zkey-ekmfweb.so
    $(INSTALL) -d -m 755 $(DESTDIR)$(MANDIR)/man1
    $(INSTALL) -m 644 -c zkey-ekmfweb.1 $(DESTDIR)$(MANDIR)/man1
-   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIB64DIR)
-   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIB64DIR)/zkey
-   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T zkey-ekmfweb.so $(DESTDIR)$(USRLIB64DIR)/zkey/zkey-ekmfweb.so
+   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIBEXECDIR)
+   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIBEXECDIR)/zkey
+   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T zkey-ekmfweb.so $(DESTDIR)$(USRLIBEXECDIR)/zkey/zkey-ekmfweb.so

 clean:
    rm -f *.o zkey-ekmfweb.so install-libekmfweb.dep libekmfweb.dep
diff --git a/zkey/kms.c b/zkey/kms.c
index 85e3ff26e..a32d7bfe0 100644
--- a/zkey/kms.c
+++ b/zkey/kms.c
@@ -40,7 +40,7 @@

 #define ENVVAR_ZKEY_KMS_PLUGINS        "ZKEY_KMS_PLUGINS"
 #define DEFAULT_KMS_PLUGINS        "/etc/zkey/kms-plugins.conf"
-#define KMS_PLUGIN_LOCATION        "/usr/lib64/zkey"
+#define KMS_PLUGIN_LOCATION        "/usr/libexec/zkey"

 #define KMS_CONFIG_FILE            "kms.conf"
 #define KMS_CONFIG_PROP_KMS        "kms"
-- 
hoeppnerj commented 3 years ago

As @sharkcz mentioned here https://github.com/ibm-s390-tools/s390-tools/issues/93#issuecomment-728850417 /usr/libexec is not prefered by Fedora. I think this needs to be considered before moving on. Especially because Debian/Ubuntu seems to be the only distribution that abandoned /usr/lib64 so far. So, this change would actually be very distro (Ubuntu) specific.

However, I'm not against /usr/libexec here, I just want to make sure all distros are being considered and that we have a generic solution for this.

hoeppnerj commented 3 years ago

@markkp what is Suses view on this topic?

sharkcz commented 3 years ago

IMO the plugin path can be passed from the compiler command line with -DKMS_PLUGIN_LOCATION (via CPPFLAGS), you can set a default, but distros can easily override. To avoid hard-coding the path in the source file.

ifranzki commented 3 years ago

OK, how about this?

The default would still remain /usr/lib64/zkey/, but you can pass/override ZKEYKMSPLUGINDIR to make to choose whatever location you want.

From 76a419523f18e840fcab83753b9b8b5c3e7317ac Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Mon, 16 Nov 2020 14:38:41 +0100
Subject: [PATCH] zkey/zkey-ekmfweb: Install KMS plugins into configurable
 location

Install KMS plugins into a configurable location. The default KMS plugin
location is '/usr/lib64/zkey/', but one can set ZKEYKMSPLUGINDIR on the make
invocation to change the plugin location, e.g. to '/usr/libexec/zkey/'.

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

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 common.mak            | 3 ++-
 zkey/Makefile         | 2 ++
 zkey/ekmfweb/Makefile | 5 ++---
 zkey/kms.c            | 2 ++
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/common.mak b/common.mak
index ae6208d2f..debc2d016 100644
--- a/common.mak
+++ b/common.mak
@@ -174,12 +174,13 @@ ZFCPDUMP_DIR    = $(TOOLS_LIBDIR)/zfcpdump
 # for SYSTEMDSYSTEMUNITDIR (e.g. /lib/systemd/system)
 SYSTEMDSYSTEMUNITDIR =
 USRINCLUDEDIR   = $(INSTALLDIR)/usr/include
+ZKEYKMSPLUGINDIR = $(USRLIB64DIR)/zkey

 INSTDIRS        = $(USRSBINDIR) $(USRBINDIR) $(BINDIR) $(LIBDIR) $(MANDIR) \
            $(SYSCONFDIR) $(SYSCONFDIR)/sysconfig \
            $(TOOLS_LIBDIR) $(TOOLS_DATADIR) \
            $(ZFCPDUMP_DIR) $(SYSTEMDSYSTEMUNITDIR) \
-           $(USRLIB64DIR) $(USRINCLUDEDIR)
+           $(USRLIB64DIR) $(USRINCLUDEDIR) $(ZKEYKMSPLUGINDIR)
 OWNER           = $(shell id -un)
 GROUP      = $(shell id -gn)
 export INSTALLDIR BINDIR LIBDIR USRLIB64DIR MANDIR OWNER GROUP
diff --git a/zkey/Makefile b/zkey/Makefile
index 7411fd3d4..b28754824 100644
--- a/zkey/Makefile
+++ b/zkey/Makefile
@@ -36,6 +36,8 @@ endif

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

+CFLAGS += -DKMS_PLUGIN_LOCATION=\"$(ZKEYKMSPLUGINDIR)\"
+
 detect-libcryptsetup.dep:
    echo "#include <libcryptsetup.h>" > detect-libcryptsetup.dep
    echo "#ifndef CRYPT_LUKS2" >> detect-libcryptsetup.dep
diff --git a/zkey/ekmfweb/Makefile b/zkey/ekmfweb/Makefile
index 1a3a49775..f63c6293f 100644
--- a/zkey/ekmfweb/Makefile
+++ b/zkey/ekmfweb/Makefile
@@ -44,9 +44,8 @@ libekmfweb.dep:
 install: all install-libekmfweb.dep zkey-ekmfweb.so
    $(INSTALL) -d -m 755 $(DESTDIR)$(MANDIR)/man1
    $(INSTALL) -m 644 -c zkey-ekmfweb.1 $(DESTDIR)$(MANDIR)/man1
-   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIB64DIR)
-   $(INSTALL) -d -m 755 $(DESTDIR)$(USRLIB64DIR)/zkey
-   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T zkey-ekmfweb.so $(DESTDIR)$(USRLIB64DIR)/zkey/zkey-ekmfweb.so
+   $(INSTALL) -d -m 755 $(DESTDIR)$(ZKEYKMSPLUGINDIR)
+   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T zkey-ekmfweb.so $(DESTDIR)$(ZKEYKMSPLUGINDIR)/zkey-ekmfweb.so

 clean:
    rm -f *.o zkey-ekmfweb.so install-libekmfweb.dep libekmfweb.dep
diff --git a/zkey/kms.c b/zkey/kms.c
index 85e3ff26e..df026988b 100644
--- a/zkey/kms.c
+++ b/zkey/kms.c
@@ -40,7 +40,9 @@

 #define ENVVAR_ZKEY_KMS_PLUGINS        "ZKEY_KMS_PLUGINS"
 #define DEFAULT_KMS_PLUGINS        "/etc/zkey/kms-plugins.conf"
+#ifndef KMS_PLUGIN_LOCATION
 #define KMS_PLUGIN_LOCATION        "/usr/lib64/zkey"
+#endif

 #define KMS_CONFIG_FILE            "kms.conf"
 #define KMS_CONFIG_PROP_KMS        "kms"
-- 
sharkcz commented 3 years ago

works for me, I believe you could drop the whole #ifndef section from kms.c, -DKMS_PLUGIN_LOCATION is part of the compiler flags, so it should always work.

xnox commented 3 years ago

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_libexecdir

Fedora uses libexec, and many upstream packages use it, and fedora packages them with libexec too....

E.g. fwupd that is install on all baremetal x86 installs

/usr/libexec/fwupd /usr/libexec/fwupd/efi/fwupdx64.efi /usr/libexec/fwupd/efi/fwupdx64.efi.signed /usr/libexec/fwupd/fwupd /usr/libexec/fwupd/fwupd-detect-cet /usr/libexec/fwupd/fwupdoffline

there are lots of other examples too. My machine has 70+ packages that use /usr/libexec.

Unlike Fedora, Ubuntu/Debian supports arbitrary multiarch including installing and cross-building to ppc64le/arm64/x86_64 from s390x, hence /usr/lib64 is ambigious and worthless. All of that was upstreamed to gcc/python/etc. such that fedora could start using multiarch lib prefixes, but they don't for the time being. s390-tools zkey library is not a good enough argument to break all of that =)

xnox commented 3 years ago

BTW why is zkey-ekmfweb.so a public library? Is the ABI going to be maintined, with symbols versioned, public header compiled, and many userspace and 3rd party packages and apps expecting to use it?

Or is it private, but shared among s390-tools components only? For example, libsystemd-shared-246.so is a private shared library, that is not shipped in the public ld paths, and instead all binaries ship with runpath set to /usr/lib/systemd, such that only systemd things are allowed to link with it.

ifranzki commented 3 years ago

zkey-ekmfweb.so is a plugin for zkey. I don't see that it will ever be used by anyone else than zkey. And /usr/lib64/zkey/ (or /usr/libexec/zkey) is not in the ld path either. Nevertheless, it uses versioned symbols, since the plugin interface may change (get enhanced) over time.

xnox commented 3 years ago

zkey-ekmfweb.so is a plugin for zkey. I don't see that it will ever be used by anyone else than zkey. And /usr/lib64/zkey/ (or /usr/libexec/zkey) is not in the ld path either. Nevertheless, it uses versioned symbols, since the plugin interface may change (get enhanced) over time.

right sure.

I was more about usr/lib64/libekmfweb.so.1.0 => which does have versioned symbols and looks very general purpose and useful, and making it private would be a shame. Thus getting shipped publicly is probably for the best.

ifranzki commented 3 years ago

Yes, libekmfweb.so1.0 is a public library by intention, and can be used by others, too.

How about the following change to allow to override the installation path for libekmfweb.so1.0?

From ef83f0600d932a7fb295c7661c2f74ef39517d3d Mon Sep 17 00:00:00 2001
From: Ingo Franzki <ifranzki@linux.ibm.com>
Date: Wed, 18 Nov 2020 10:09:49 +0100
Subject: [PATCH] libekmfweb: Make install directory for shared library
 configurable

If LIBDIR is not passed to the make invocation, install shared libraries
to USRLIB64DIR (/usr/lib64/), even though LIBDIR defaults to /lib/. If
LIBDIR is overridden for the make invocation, install shared libraries to
whatever is specified as LIBDIR.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 common.mak          | 9 ++++++++-
 libekmfweb/Makefile | 6 +++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/common.mak b/common.mak
index debc2d016..e981d2bc3 100644
--- a/common.mak
+++ b/common.mak
@@ -176,11 +176,18 @@ SYSTEMDSYSTEMUNITDIR =
 USRINCLUDEDIR   = $(INSTALLDIR)/usr/include
 ZKEYKMSPLUGINDIR = $(USRLIB64DIR)/zkey

+ifeq ($(LIBDIR),$(INSTALLDIR)/lib)
+SOINSTALLDIR = $(USRLIB64DIR)
+else
+SOINSTALLDIR = $(LIBDIR)
+endif
+
 INSTDIRS        = $(USRSBINDIR) $(USRBINDIR) $(BINDIR) $(LIBDIR) $(MANDIR) \
            $(SYSCONFDIR) $(SYSCONFDIR)/sysconfig \
            $(TOOLS_LIBDIR) $(TOOLS_DATADIR) \
            $(ZFCPDUMP_DIR) $(SYSTEMDSYSTEMUNITDIR) \
-           $(USRLIB64DIR) $(USRINCLUDEDIR) $(ZKEYKMSPLUGINDIR)
+           $(USRLIB64DIR) $(USRINCLUDEDIR) $(ZKEYKMSPLUGINDIR) \
+           $(SOINSTALLDIR)
 OWNER           = $(shell id -un)
 GROUP      = $(shell id -gn)
 export INSTALLDIR BINDIR LIBDIR USRLIB64DIR MANDIR OWNER GROUP
diff --git a/libekmfweb/Makefile b/libekmfweb/Makefile
index 07d64e0f3..5d0b1b920 100644
--- a/libekmfweb/Makefile
+++ b/libekmfweb/Makefile
@@ -77,9 +77,9 @@ libekmfweb.so.$(VERSION): ekmfweb.o utilities.o cca.o
    ln -srf libekmfweb.so.$(VERSION) libekmfweb.so

 install-libekmfweb.so.$(VERSION): libekmfweb.so.$(VERSION)
-   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T libekmfweb.so.$(VERSION) $(DESTDIR)$(USRLIB64DIR)/libekmfweb.so.$(VERSION)
-   ln -srf $(DESTDIR)$(USRLIB64DIR)/libekmfweb.so.$(VERSION) $(DESTDIR)$(USRLIB64DIR)/libekmfweb.so.$(VERM)
-   ln -srf $(DESTDIR)$(USRLIB64DIR)/libekmfweb.so.$(VERSION) $(DESTDIR)$(USRLIB64DIR)/libekmfweb.so
+   $(INSTALL) -g $(GROUP) -o $(OWNER) -m 755 -T libekmfweb.so.$(VERSION) $(DESTDIR)$(SOINSTALLDIR)/libekmfweb.so.$(VERSION)
+   ln -srf $(DESTDIR)$(SOINSTALLDIR)/libekmfweb.so.$(VERSION) $(DESTDIR)$(SOINSTALLDIR)/libekmfweb.so.$(VERM)
+   ln -srf $(DESTDIR)$(SOINSTALLDIR)/libekmfweb.so.$(VERSION) $(DESTDIR)$(SOINSTALLDIR)/libekmfweb.so
    $(INSTALL) -d -m 770 $(DESTDIR)$(USRINCLUDEDIR)/ekmfweb
    $(INSTALL) -g $(GROUP) -o $(OWNER) -m 644 $(rootdir)include/ekmfweb/ekmfweb.h $(DESTDIR)$(USRINCLUDEDIR)/ekmfweb

-- 
xnox commented 3 years ago

Yes, libekmfweb.so1.0 is a public library by intention, and can be used by others, too.

How about the following change to allow to override the installation path for libekmfweb.so1.0?

yeap, that looks good.

markkp commented 3 years ago

@markkp what is Suses view on this topic?

From what I can tell, neither SUSE nor openSUSE uses /usr/libexec at all. The directory doesn't even exist on my Leap 15.1 system.

xnox commented 3 years ago

@markkp what is Suses view on this topic?

From what I can tell, neither SUSE nor openSUSE uses /usr/libexec at all. The directory doesn't even exist on my Leap 15.1 system.

I see rpms that do use libexec in OBS for 15.2 Update for example.

I do agree that in the past libexec was not used at all, but all distros in question are using libexec in the development series across a wide array of packages. Even if they didn't use them in past. Lots of system utils, programmes, gnome components are using it.

xnox commented 3 years ago

if libexec is so controversion, should we not put the plugins into /usr/lib/s390-tools/zkey/ ?

given that s390-tools on all distros already ships many things in /usr/lib/s390-tools.