sys-auth/sssd: assortment of changes to 2.3.1 #17139
Conversation
Pull Request assignmentSubmitter: @Dessa sys-auth/sssd: @gentoo/base-system, @alexxy Linked bugsNo bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment. If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers. In order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
gentoo-bot
added
assigned
Pull request CI reportReport generated at: 2020-08-16 20:41 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
sys-auth/sssd/sssd-2.3.1.ebuild
Outdated
| secrets? ( systemd ) | ||
| test? ( ssh sudo )" | ||
|
|
||
| DEPEND=" | ||
| >=sys-libs/pam-0-r1[${MULTILIB_USEDEP}] |
There was a problem hiding this comment.
I'm a bit surprised that e.g. pam is a multilib dep, but openssl isn't. Can you verify which do/don't need the multilib dep?
There was a problem hiding this comment.
the only multilib files in this package are the plugins for pam, nss and kerberos, i think some binary only stuff segfaulted when i was using nss-pam-ldapd and it wasn't multilib but it has been a few years since that happened.
There was a problem hiding this comment.
Seems multilib(32bits) now needs samba? Didn't need that before:
checking for uintptr_t... yes
checking for ptrdiff_t... yes
checking for nscd... /usr/sbin/nscd
checking for nscd... yes
checking for nsupdate... /usr/bin/nsupdate
checking for executable nsupdate... yes
checking for nsupdate 'realm' support'... yes
checking keyutils.h usability... yes
checking keyutils.h presence... yes
checking for keyutils.h... yes
checking for add_key in -lkeyutils... yes
checking for sigprocmask... yes
checking for sigblock... yes
checking for sigaction... yes
checking for getpgrp... yes
checking for prctl... yes
checking for NDR_NBT... no
configure: error: Please install Samba 4 NDR NBT development libraries.
Samba 4 libraries are necessary for building ad and ipa provider.
If you do not want to build these providers it is possible to build SSSD
without them. In this case, you will need to execute configure script
with argument --without-samba
There was a problem hiding this comment.
good catch, try this (against tree version):
--- sssd-2.3.1.ebuild.orig 2020-08-25 01:21:11.538985183 +0200
+++ sssd-2.3.1.ebuild 2020-08-25 20:03:57.485186910 +0200
@@ -50,7 +50,7 @@
nls? ( >=sys-devel/gettext-0.18 )
pac? (
app-crypt/mit-krb5[${MULTILIB_USEDEP}]
- net-fs/samba[${MULTILIB_USEDEP}]
+ net-fs/samba
)
python? ( ${PYTHON_DEPS} )
samba? ( >=net-fs/samba-4.10.2[winbind] )
@@ -188,6 +188,7 @@
# ldb lib fails... but it does not seem to bother
{DHASH,COLLECTION,INI_CONFIG_V{0,1,1_1,1_3}}_{CFLAGS,LIBS}=' '
{PCRE,CARES,SYSTEMD_LOGIN,SASL,GLIB2,DBUS,CRYPTO,P11_KIT}_{CFLAGS,LIBS}=' '
+ {NDR_NBT,SMBCLIENT,NDR_KRB5PAC}_{CFLAGS,LIBS}=' '
# use native include path for dbus (needed for build)
DBUS_CFLAGS="${native_dbus_cflags}"
@@ -196,6 +197,7 @@
ac_cv_lib_ldap_ldap_search=yes
--without-secrets
--without-kcm
+ --with-smb-idmap-interface-version=6
)
fi
There was a problem hiding this comment.
If that works for Joakim, please submit a pull request and mention me. (Also include a patch to remove the pac USE flag if you like)
There was a problem hiding this comment.
Finally, a -9999 version would be appreciated as sssd is slow to release.
I use this patch to avoid the famous samba lib bug:
--- ./src/external/samba.m4.org 2020-06-25 11:04:18.221147249 +0200
+++ ./src/external/samba.m4 2020-06-25 11:05:04.654862849 +0200
@@ -60,7 +60,7 @@
SAVE_CFLAGS=$CFLAGS
SAVE_LIBS=$LIBS
CFLAGS="$CFLAGS $SMBCLIENT_CFLAGS $NDR_NBT_CFLAGS $NDR_KRB5PAC_CFLAGS"
- LIBS="$LIBS -L${sambalibdir} -lidmap-samba4 -Wl,-rpath ${sambalibdir}"
+ LIBS="$LIBS -L${sambalibdir} -Wl,--no-as-needed -lidmap-samba4 -lreplace-samba4 -lsmbd-shim-samba4 -Wl,-rpath ${sambalibdir}"
AC_RUN_IFELSE(
[AC_LANG_SOURCE([
#include <stdlib.h>
There was a problem hiding this comment.
That builds but I am not happy with that hack. Why not have --without-samba instead of
--with-smb-idmap-interface-version=6
NDR_NBT,SMBCLIENT,NDR_KRB5PAC}_{CFLAGS,LIBS}=' '
hacks ?
because the pac responder doesn't build with --without-samba, if upstream provided a way to build the krb and pam plugins separately this mess would not be required but i don't think there is much of a use-case for them.
don't really want to patch configure either.
Also, I have always used --with-crypto="" too in my ebuild, nothing in 32bits seems
to need openssl, does it?
there is no point HAVE_LIBCRYPTO is still 1 with that.
Not sure about sssd_krb5_locator_plugin.so either, can it be used for 32 bits?
probably. i guess my earlier comment still applies but i don't use any of the kerberos plugins myself
USE=manpages should probably be USE=man instead
yeah, probably (why is man even a local useflag still at this point?)
I use this patch to avoid the famous samba lib bug
its not famous enough that i know about it. what exactly does that fix?
There was a problem hiding this comment.
That builds but I am not happy with that hack. Why not have --without-samba instead of
--with-smb-idmap-interface-version=6
NDR_NBT,SMBCLIENT,NDR_KRB5PAC}_{CFLAGS,LIBS}=' '
hacks ?because the pac responder doesn't build with --without-samba, if upstream provided a way to build the krb and pam plugins separately this mess would not be required but i don't think there is much of a use-case for them.
don't really want to patch configure either.
Yes, I noticed that later.
Also, I have always used --with-crypto="" too in my ebuild, nothing in 32bits seems
to need openssl, does it?there is no point HAVE_LIBCRYPTO is still 1 with that.
hmm, that is a bit odd.
Not sure about sssd_krb5_locator_plugin.so either, can it be used for 32 bits?
probably. i guess my earlier comment still applies but i don't use any of the kerberos plugins myself
USE=manpages should probably be USE=man instead
yeah, probably (why is man even a local useflag still at this point?)
I use this patch to avoid the famous samba lib bug
its not famous enough that i know about it. what exactly does that fix?
There was a problem hiding this comment.
ugh. i'd... rather use --with-smb-idmap-interface-version=6 in native as well as we already depend on samba newer then 4.7 anyway (it by itself doesn't generate a dep on samba, at least on non-native but i'll re-test that of course.
There was a problem hiding this comment.
ugh. i'd... rather use
--with-smb-idmap-interface-version=6in native as well as we already depend on samba newer then 4.7 anyway (it by itself doesn't generate a dep on samba, at least on non-native but i'll re-test that of course.
For 32 bits, yes the --with-smb-idmap-interface-version=6 is best.
But the native part will fail everynow and then. Not sure what the I/F is in samba 4.12 ? if still 6 one could use with-smb-idmap-interface-version=6 overall.
sys-auth/sssd/sssd-2.3.1.ebuild
Outdated
| @@ -12,32 +14,35 @@ KEYWORDS="~amd64 ~arm ~arm64 ~hppa ~ia64 ~m68k ~mips ~ppc ~ppc64 ~s390 ~sparc ~x | |||
|
|
|||
| LICENSE="GPL-3" | |||
| SLOT="0" | |||
| IUSE="acl +autofs +locator +netlink nfsv4 nls +manpages samba selinux +sudo +ssh systemd test valgrind" | |||
| IUSE="acl +autofs doc kcm +locator +netlink nfsv4 nls +manpages pac python samba secrets selinux sudo ssh systemd test valgrind" | |||
There was a problem hiding this comment.
We have quite a lot of USE flags now, which is fine, but we don't always need to expose them jsut because an option exists. Are they all valuable?
There was a problem hiding this comment.
i guess pac can go since it doesn't add another dependency but im not so sure about the other ones...
There was a problem hiding this comment.
When I discovered that USE="ssh sudo" was required to build the tests, I thought about removing the flags entirely.
They, along with IUSE=autofs, seem like very low value options. They each install something like two extra files and have no additional dependencies.
Given the, seemingly inherent, complexity of this package I think removing USE flags like these is probably a good idea.
There was a problem hiding this comment.
i have gone ahead and yanked autofs and ssh. removed kcm and secrets to merge with systemd flag
sudo needs changes to app-admin/sudo
There was a problem hiding this comment.
Can you elaborate on what you mean about needing changes to app-admin/sudo?
Would be good to sort that out, because I think sssd's tests need sudo support enabled.
There was a problem hiding this comment.
sssd use on sudo itself need to turn into sssd(+) or whatever the correct syntax was.
There was a problem hiding this comment.
In other words, app-admin/sudo needs to have USE=sssd enabled by default? How could that help?
Maybe let's back up a step, and tell me what breaks if we remove IUSE=sudo from sys-auth/sssd and enable the option always?
There was a problem hiding this comment.
sorry, no, i meant something else
sssd? ( sys-auth/sssd[sudo] )
needs to be sssd? ( sys-auth/sssd[sudo(+)] )
i would imagine pkgcore yelling if we would just remove sudo use from sssd without doing that first.
and i also just realized i forgot to push something, pac doesn't really need to be a own useflag, it could be condensed into samba too.
but i guess its not a huge deal.
- re-add python useflag, add doc use, (re-)build localized manpages with nls use - hide secrets and kcm behind systemd - use upstream provided initscript - cleanups Package-Manager: Portage-3.0.2, Repoman-2.3.23 Signed-off-by: Robert Förster <Dessa@gmake.de>
|
tests do not run in full for me but at least the python changes did not add new failures in the mix. decided to switch to -single-r1 since tests can be run this way, also cleans up the mess that it was before, while it worked i really didn't like it. |
Pull request CI reportReport generated at: 2020-08-18 23:36 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
|
Thanks. I edited metadata.xml a little bit. I wasn't sure why you added the fedoraproject cpe, so I removed that. Did you intentionally add that? |
|
Okay, thanks. I've fixed that. |
Closes: #17139 Signed-off-by: Robert Förster <Dessa@gmake.de> Signed-off-by: Matt Turner <mattst88@gentoo.org>
- re-add python useflag, add doc use, (re-)build localized manpages with nls use - hide secrets and kcm behind systemd - use upstream provided initscript - cleanups Closes: gentoo#13308 Closes: gentoo#17139 Signed-off-by: Robert Förster <Dessa@gmake.de> Signed-off-by: Matt Turner <mattst88@gentoo.org>
Closes: gentoo#17139 Signed-off-by: Robert Förster <Dessa@gmake.de> Signed-off-by: Matt Turner <mattst88@gentoo.org>
cc @mattst88
(in-place changes since its still masked, also makes it easier to review )
i do realize that i have been a bit brief in the commit message; i'll be more verbose here.
added a few more useflags: doc, kcm, secrets and pac
using python-single-r1 in order to get python tests being run
libwbclient is gone [from here]. upstream default disabled it with the intent to remove it because it stopped working a while ago
migrate dependency stuff to {R,B,}DEPEND
add a bunch of new configure options
kerberos workaround do not seem to be required anymore, at least for my multilib build (no cross compilation tested)
caveats: