[Freeipa-devel] [PATCH] Debian client support

[Petr Viktorin]

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 (<nss/..> not <nss3/..>)

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³