Re: [Freeipa-devel] [PATCH] Debian client support
On 01/27/2014 12:17 PM, Timo Aaltonen wrote: > On 28.11.2013 22:26, Lukas Slebodnik wrote: >> On (05/09/13 23:25), Lukas Slebodnik wrote: >>> On (03/09/13 00:43), Timo Aaltonen wrote: This fixes https://fedorahosted.org/freeipa/ticket/1887 and https://fedorahosted.org/freeipa/ticket/2455 the first three patches fix some bugs in how python is used fourth patch checks if dbus is already running before trying to start it fifth fixes some compilation warnings sixth finally adds the Debian platform module there are also distro patches that aren't upstreamable as-is, that do stuff like - give--install-layout=deb to setup.py - disable make-testcert since it needs a server running - fix hardcoded NFS related paths and a variable in ipa-client-automount - fix ldap.conf path in ipa-client-install - fix ntpdate options in ntpconf.py (Debian doesn't patch ntpdate like Fedora) - change nss includes in ipa_pwd.c ( not ) >>> Solution is simple. Use pkg-config generated NSS_CFLAGS >>> >>> bash$ pkg-config --cflags nss >>> -I/usr/include/nss -I/usr/include/nspr >>> bash$ uname -a >>> Linux positron 3.10-2-686-pae #1 SMP Debian 3.10.5-1 (2013-08-07) i686 >>> GNU/Linux >>> >>> bash$pkg-config --cflags nss >>> -I/usr/include/nss3 -I/usr/include/nspr4 >>> bash$uname -a >>> Linux unused-4-233.brq.redhat.com 3.10.10-200.fc19.x86_64 #1 SMP Thu Aug 29 >>> 19:05:45 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >>> >>> It works in sssd. I can send a patch. >>> >>> LS >>> >> Attached patch should fix problem with compilation on different distros. >> >> debian: >> http://anonscm.debian.org/gitweb/?p=pkg-freeipa/freeipa.git;a=blob;f=debian/patches/fix-nss-include.diff;h=1dac0709ed7344c7546c55225365c9434e6a930a;hb=HEAD >> arch: >> https://github.com/chenxiaolong/ArchLinux-Packages/blob/master/freeipa/0006_Fix_nss_includes.patch >> >> Timo can you test patch on debian/ubuntu? > > finally did last week, so > > Tested-by: Timo Aaltonen Thanks Timo. I did a mild rebase to current master, tested FreeIPA in Fedora still builds and pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On 28.11.2013 22:26, Lukas Slebodnik wrote: > On (05/09/13 23:25), Lukas Slebodnik wrote: >> On (03/09/13 00:43), Timo Aaltonen wrote: >>> >>> This fixes https://fedorahosted.org/freeipa/ticket/1887 >>> and >>> https://fedorahosted.org/freeipa/ticket/2455 >>> >>> the first three patches fix some bugs in how python is used >>> fourth patch checks if dbus is already running before trying to start it >>> fifth fixes some compilation warnings >>> sixth finally adds the Debian platform module >>> >>> >>> >>> there are also distro patches that aren't upstreamable as-is, that do >>> stuff like >>> - give--install-layout=deb to setup.py >>> - disable make-testcert since it needs a server running >>> - fix hardcoded NFS related paths and a variable in ipa-client-automount >>> - fix ldap.conf path in ipa-client-install >>> - fix ntpdate options in ntpconf.py (Debian doesn't patch ntpdate like >>> Fedora) >>> - change nss includes in ipa_pwd.c ( not ) >> Solution is simple. Use pkg-config generated NSS_CFLAGS >> >> bash$ pkg-config --cflags nss >> -I/usr/include/nss -I/usr/include/nspr >> bash$ uname -a >> Linux positron 3.10-2-686-pae #1 SMP Debian 3.10.5-1 (2013-08-07) i686 >> GNU/Linux >> >> bash$pkg-config --cflags nss >> -I/usr/include/nss3 -I/usr/include/nspr4 >> bash$uname -a >> Linux unused-4-233.brq.redhat.com 3.10.10-200.fc19.x86_64 #1 SMP Thu Aug 29 >> 19:05:45 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >> >> It works in sssd. I can send a patch. >> >> LS >> > Attached patch should fix problem with compilation on different distros. > > debian: > http://anonscm.debian.org/gitweb/?p=pkg-freeipa/freeipa.git;a=blob;f=debian/patches/fix-nss-include.diff;h=1dac0709ed7344c7546c55225365c9434e6a930a;hb=HEAD > arch: > https://github.com/chenxiaolong/ArchLinux-Packages/blob/master/freeipa/0006_Fix_nss_includes.patch > > Timo can you test patch on debian/ubuntu? finally did last week, so Tested-by: Timo Aaltonen ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On (05/09/13 23:25), Lukas Slebodnik wrote: >On (03/09/13 00:43), Timo Aaltonen wrote: >> >>This fixes https://fedorahosted.org/freeipa/ticket/1887 >>and >>https://fedorahosted.org/freeipa/ticket/2455 >> >>the first three patches fix some bugs in how python is used >>fourth patch checks if dbus is already running before trying to start it >>fifth fixes some compilation warnings >>sixth finally adds the Debian platform module >> >> >> >>there are also distro patches that aren't upstreamable as-is, that do >>stuff like >>- give--install-layout=deb to setup.py >>- disable make-testcert since it needs a server running >>- fix hardcoded NFS related paths and a variable in ipa-client-automount >>- fix ldap.conf path in ipa-client-install >>- fix ntpdate options in ntpconf.py (Debian doesn't patch ntpdate like >>Fedora) >>- change nss includes in ipa_pwd.c ( not ) >Solution is simple. Use pkg-config generated NSS_CFLAGS > >bash$ pkg-config --cflags nss >-I/usr/include/nss -I/usr/include/nspr >bash$ uname -a >Linux positron 3.10-2-686-pae #1 SMP Debian 3.10.5-1 (2013-08-07) i686 >GNU/Linux > >bash$pkg-config --cflags nss >-I/usr/include/nss3 -I/usr/include/nspr4 >bash$uname -a >Linux unused-4-233.brq.redhat.com 3.10.10-200.fc19.x86_64 #1 SMP Thu Aug 29 >19:05:45 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux > >It works in sssd. I can send a patch. > >LS > Attached patch should fix problem with compilation on different distros. debian: http://anonscm.debian.org/gitweb/?p=pkg-freeipa/freeipa.git;a=blob;f=debian/patches/fix-nss-include.diff;h=1dac0709ed7344c7546c55225365c9434e6a930a;hb=HEAD arch: https://github.com/chenxiaolong/ArchLinux-Packages/blob/master/freeipa/0006_Fix_nss_includes.patch Timo can you test patch on debian/ubuntu? LS >From 2d9e290970e71d373b91cd0cd1db52b991636889 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Thu, 28 Nov 2013 15:32:07 +0100 Subject: [PATCH] BUILD: Fix portability of NSS in file ipa_pwd.c --- daemons/ipa-kdb/Makefile.am | 4 +++- daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am | 1 + util/ipa_pwd.c | 8 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/daemons/ipa-kdb/Makefile.am b/daemons/ipa-kdb/Makefile.am index dc543dd..b3d6a1b 100644 --- a/daemons/ipa-kdb/Makefile.am +++ b/daemons/ipa-kdb/Makefile.am @@ -21,6 +21,7 @@ AM_CPPFLAGS = \ $(KRB5_CFLAGS) \ $(WARN_CFLAGS) \ $(NDRPAC_CFLAGS)\ + $(NSS_CFLAGS) \ $(NULL) plugindir = $(libdir)/krb5/plugins/kdb @@ -51,6 +52,7 @@ ipadb_la_LIBADD = \ $(LDAP_LIBS)\ $(NDRPAC_LIBS) \ $(UNISTRING_LIBS) \ + $(NSS_LIBS) \ $(NULL) if HAVE_CHECK @@ -77,7 +79,7 @@ ipa_kdb_tests_LDADD = \ $(KRB5_LIBS)\ $(LDAP_LIBS)\ $(NDRPAC_LIBS) \ - -lnss3 \ + $(NSS_LIBS) \ -lkdb5 \ -lsss_idmap \ $(NULL) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am index b53b2e1..3323d72 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am @@ -22,6 +22,7 @@ AM_CPPFLAGS = \ $(LDAP_CFLAGS) \ $(KRB5_CFLAGS) \ $(SSL_CFLAGS) \ + $(NSS_CFLAGS) \ $(WARN_CFLAGS) \ $(NULL) diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c index 761d1ef..f6564c8 100644 --- a/util/ipa_pwd.c +++ b/util/ipa_pwd.c @@ -27,10 +27,10 @@ #include #include #include -#include -#include -#include -#include +#include +#include +#include +#include #include #include "ipa_pwd.h" -- 1.8.4.2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On 09/03/2013 11:00 AM, Petr Viktorin wrote: On 09/02/2013 11:43 PM, Timo Aaltonen wrote: This fixes https://fedorahosted.org/freeipa/ticket/1887 and https://fedorahosted.org/freeipa/ticket/2455 Thank you! the first three patches fix some bugs in how python is used These look okay, I'll check when other build errors are fixed. [...] fifth fixes some compilation warnings Looks good to my eyes, perhaps a C expert can look at this one too. I wonder why these warnings aren't enabled in our builds, though. I've built and checked patches 1, 2, 3, 5 now, and found no regressions. I've asked Rob about why IPA explicitly disallowed to load plugins from symlinks. That code's author is not around anymore. The reason seems to be vague concerns about security, but I think we have better things to worry about than admins (or distros) that symlink from /usr/lib/** to untrusted places. (For the record: AFAIK, Debian uses symlinks for all Python modules, so distro-installed plugins will be symlinks.) ACK to those 4 patches, pushed to master: 8c03b1dbcdf75ba76b96ccfcc148afe0e134e2d3 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On (03/09/13 00:43), Timo Aaltonen wrote:
>
>This fixes https://fedorahosted.org/freeipa/ticket/1887
>and
>https://fedorahosted.org/freeipa/ticket/2455
>
>the first three patches fix some bugs in how python is used
>fourth patch checks if dbus is already running before trying to start it
>fifth fixes some compilation warnings
>sixth finally adds the Debian platform module
>
>
>
>there are also distro patches that aren't upstreamable as-is, that do
>stuff like
>- give--install-layout=deb to setup.py
>- disable make-testcert since it needs a server running
>- fix hardcoded NFS related paths and a variable in ipa-client-automount
>- fix ldap.conf path in ipa-client-install
>- fix ntpdate options in ntpconf.py (Debian doesn't patch ntpdate like
>Fedora)
>- change nss includes in ipa_pwd.c ( not )
Solution is simple. Use pkg-config generated NSS_CFLAGS
bash$ pkg-config --cflags nss
-I/usr/include/nss -I/usr/include/nspr
bash$ uname -a
Linux positron 3.10-2-686-pae #1 SMP Debian 3.10.5-1 (2013-08-07) i686 GNU/Linux
bash$pkg-config --cflags nss
-I/usr/include/nss3 -I/usr/include/nspr4
bash$uname -a
Linux unused-4-233.brq.redhat.com 3.10.10-200.fc19.x86_64 #1 SMP Thu Aug 29
19:05:45 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
It works in sssd. I can send a patch.
LS
>
>dunno what to do about those, the packaging can keep on carrying those
>but if you have ideas how to make them configurable so that upstream
>git/tarball could be used for development/testing on Debian then that
>would be nice.
>
>t
>From b08da1b7712f9621283719b190134586e59fb333 Mon Sep 17 00:00:00 2001
>From: Timo Aaltonen
>Date: Tue, 3 Sep 2013 00:01:12 +0300
>Subject: [PATCH 1/6] Use /usr/bin/python as fallback python path
>
>---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Makefile b/Makefile
>index
>a21cf7e33275fd1a783e89baf237c8dcd8db6508..428f19b1a83da8c424893ea35c901f52dafaf546
> 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -50,7 +50,7 @@ ifneq ($(DEVELOPER_MODE),0)
> LINT_OPTIONS=--no-fail
> endif
>
>-PYTHON ?= $(shell rpm -E %__python)
>+PYTHON ?= $(shell rpm -E %__python || echo /usr/bin/python)
>
> all: bootstrap-autogen server tests
> @for subdir in $(SUBDIRS); do \
>--
>1.8.3.2
>
>From 7360486d7ed343202062716c0eb4f731923bb509 Mon Sep 17 00:00:00 2001
>From: Timo Aaltonen
>Date: Tue, 3 Sep 2013 00:03:12 +0300
>Subject: [PATCH 2/6] Don't search platform path
>
>Don't use Python.h from the platform specific path
>---
> ipapython/py_default_encoding/setup.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/ipapython/py_default_encoding/setup.py
>b/ipapython/py_default_encoding/setup.py
>index
>de2478c1962aba7a78919efdb266bf0600965905..6a1af628272c6cd3eaa755c5728a7a5d020050ec
> 100644
>--- a/ipapython/py_default_encoding/setup.py
>+++ b/ipapython/py_default_encoding/setup.py
>@@ -22,7 +22,7 @@ from distutils.sysconfig import get_python_inc
> import sys
> import os
>
>-python_header = os.path.join(get_python_inc(plat_specific=1), 'Python.h')
>+python_header = os.path.join(get_python_inc(plat_specific=0), 'Python.h')
> if not os.path.exists(python_header):
> sys.exit("Cannot find Python development packages that provide Python.h")
>
>--
>1.8.3.2
>
>From be86f0a9bbc3196aa8808243aba6d7e68c6d083b Mon Sep 17 00:00:00 2001
>From: Nick Hatch
>Date: Tue, 3 Sep 2013 00:08:13 +0300
>Subject: [PATCH 3/6] Don't exclude symlinks when loading plugins
>
>---
> ipalib/util.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/ipalib/util.py b/ipalib/util.py
>index
>3c52e4fd9a3e08d160dd4ae7076590be8b869d2c..e14077487e979f077ddc1f9e925678884a64b5b5
> 100644
>--- a/ipalib/util.py
>+++ b/ipalib/util.py
>@@ -81,7 +81,7 @@ def find_modules_in_dir(src_dir):
> if not name.endswith(suffix):
> continue
> pyfile = os.path.join(src_dir, name)
>-if os.path.islink(pyfile) or not os.path.isfile(pyfile):
>+if not os.path.isfile(pyfile):
> continue
> module = name[:-len(suffix)]
> if module == '__init__':
>--
>1.8.3.2
>
>From 34d002d5483b9853a8329215ab12c946b8df7525 Mon Sep 17 00:00:00 2001
>From: Nick Hatch
>Date: Tue, 3 Sep 2013 00:10:30 +0300
>Subject: [PATCH 4/6] Check dbus before starting it
>
>Check to see if the messagebus (dbus) is running before attempting to start it
>---
> ipa-client/ipa-install/ipa-client-install | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/ipa-client/ipa-install/ipa-client-install
>b/ipa-client/ipa-install/ipa-client-install
>index
>280edd793326150c416fe1b82f9866435e9c6509..7241a3421e348666c47f03a9b4fdac472b2ccabb
> 100755
>--- a/ipa-client/ipa-install/ipa-client-install
>+++ b/ipa-client/ipa-install/ipa-client-install
>@@ -372,10 +372,11 @@ def uninstall(options, env):
> # Always start certmonger. We can't untrack something if it isn't
> # running
> messagebus = ipaservices.knownservices.messagebus
>-try
Re: [Freeipa-devel] [PATCH] Debian client support
On 03.09.2013 12:00, Petr Viktorin wrote:
> On 09/02/2013 11:43 PM, Timo Aaltonen wrote:
>>
>> This fixes https://fedorahosted.org/freeipa/ticket/1887
>> and
>> https://fedorahosted.org/freeipa/ticket/2455
>
> Thank you!
>
>> the first three patches fix some bugs in how python is used
>
> These look okay, I'll check when other build errors are fixed.
>
>> fourth patch checks if dbus is already running before trying to start it
>
> Please handle this in platform/debian/service.py.
>
> Is this only for D-Bus or do all start() methods for Debian need this?
> If it's all of them, add it in DebianService.start.
> If it's just D-Bus you'll want to make a special service there, like
> DebianSSHService.
>
>> fifth fixes some compilation warnings
>
> Looks good to my eyes, perhaps a C expert can look at this one too.
> I wonder why these warnings aren't enabled in our builds, though.
>
>> sixth finally adds the Debian platform module
>
> Please add copyright headers to the new files.
>
> in debian/auth.py:DebianAuthConfig.execute, you should use a dictionary
> for env:
> env = {'DEBCONF_FRONTEND': 'noninteractive'}
>
> You need to import ipautil to use ipautil.run in auth.py. This trips
> pylint and prevents building the code for me. Do you include pylint in
> your build procedure?
>
> platform/debian/auth.py: Git complains about a new blank line at EOF
Ok I have the platform module patch updated, but testing is blocked
because client join fails with '401' error (authorization). This worked
fine in June, still investigating what's wrong this time..
thanks for the review!
--
t
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On Tue, Sep 03, 2013 at 01:34:48PM +0200, Petr Viktorin wrote: > On 09/03/2013 11:22 AM, Jakub Hrozek wrote: > >On Tue, Sep 03, 2013 at 11:00:07AM +0200, Petr Viktorin wrote: > >>>fifth fixes some compilation warnings > >> > >>Looks good to my eyes, perhaps a C expert can look at this one too. > >>I wonder why these warnings aren't enabled in our builds, though. > > > >They look good to me, too. (Does this answer make me a C expert? :-)) > > Why, yes! (Your certificate will arrive by mail shortly.) > > >-Wformat-security is not among default CFLAGS on Fedora. In SSSD, we > >only recently fixed these problems in our debug messages. > > I've taken the WARNINGS from SSSD's aliases, built with them, and > reported three additional warnings. > > https://fedorahosted.org/freeipa/ticket/3896 Yes, they are a good source of defensive CFLAGS. But make sure to grab a very recent version (git HEAD ideally), there was recently a typo that prevented the aliases from working correctly. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On 09/03/2013 11:22 AM, Jakub Hrozek wrote: On Tue, Sep 03, 2013 at 11:00:07AM +0200, Petr Viktorin wrote: fifth fixes some compilation warnings Looks good to my eyes, perhaps a C expert can look at this one too. I wonder why these warnings aren't enabled in our builds, though. They look good to me, too. (Does this answer make me a C expert? :-)) Why, yes! (Your certificate will arrive by mail shortly.) -Wformat-security is not among default CFLAGS on Fedora. In SSSD, we only recently fixed these problems in our debug messages. I've taken the WARNINGS from SSSD's aliases, built with them, and reported three additional warnings. https://fedorahosted.org/freeipa/ticket/3896 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On Tue, Sep 03, 2013 at 11:00:07AM +0200, Petr Viktorin wrote: > >fifth fixes some compilation warnings > > Looks good to my eyes, perhaps a C expert can look at this one too. > I wonder why these warnings aren't enabled in our builds, though. They look good to me, too. (Does this answer make me a C expert? :-)) -Wformat-security is not among default CFLAGS on Fedora. In SSSD, we only recently fixed these problems in our debug messages. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Debian client support
On 09/02/2013 11:43 PM, Timo Aaltonen wrote:
This fixes https://fedorahosted.org/freeipa/ticket/1887
and
https://fedorahosted.org/freeipa/ticket/2455
Thank you!
the first three patches fix some bugs in how python is used
These look okay, I'll check when other build errors are fixed.
fourth patch checks if dbus is already running before trying to start it
Please handle this in platform/debian/service.py.
Is this only for D-Bus or do all start() methods for Debian need this?
If it's all of them, add it in DebianService.start.
If it's just D-Bus you'll want to make a special service there, like
DebianSSHService.
fifth fixes some compilation warnings
Looks good to my eyes, perhaps a C expert can look at this one too.
I wonder why these warnings aren't enabled in our builds, though.
sixth finally adds the Debian platform module
Please add copyright headers to the new files.
in debian/auth.py:DebianAuthConfig.execute, you should use a dictionary
for env:
env = {'DEBCONF_FRONTEND': 'noninteractive'}
You need to import ipautil to use ipautil.run in auth.py. This trips
pylint and prevents building the code for me. Do you include pylint in
your build procedure?
platform/debian/auth.py: Git complains about a new blank line at EOF
I don't think anyone from the regular IPA team can really verify the
Debian code, so we may just trust you that it works and push it without
full tests :)
there are also distro patches that aren't upstreamable as-is, that do
stuff like
- give--install-layout=deb to setup.py
Add a SETUP_PY_ARGS variable to the Makefile.
- disable make-testcert since it needs a server running
For now you can just run ./make-test directly. Getting `make test`
working will be just icing on the cake at this point.
- fix hardcoded NFS related paths and a variable in ipa-client-automount
- fix ldap.conf path in ipa-client-install
ipa-client requires ipa-python, we can just stick these in the platform
module and include that.
- fix ntpdate options in ntpconf.py (Debian doesn't patch ntpdate like
Fedora)
A patch to replace ntpdate with ntp is on review right now; you'll want
to revisit this when it gets pushed.
- change nss includes in ipa_pwd.c ( not )
I'd ask for a C expert's opinion.
dunno what to do about those, the packaging can keep on carrying those
but if you have ideas how to make them configurable so that upstream
git/tarball could be used for development/testing on Debian then that
would be nice.
Patches to make them configurable are very welcome. You might want to
file a bug for each patch, so it's easier to keep track of what's left
to do.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
