[Freeipa-devel] [PULL REQUEST, master] Platform-specific adaptation

[Martin Kosek]

On Mon, 2011-09-12 at 11:18 +0300, Alexander Bokovoy wrote:
> Hi,
> 
> As the patchset is rather big, I'm sending pull request from my 
> fedorapeople.org git repository instead of separate patches.
> 
> This is pull request for master branch.
> 
> The following changes since commit c97eb871c53c4a8c3bbd0f9f4b2ff23bc390bc71:
> 
>   Don't allow a OTP to be set on an enrolled host (2011-09-10 00:03:19 +0000)
> 
> are available in the git repository at:
>   git://fedorapeople.org/home/fedora/abbra/public_git/freeipa.git platform-master
> 
> Alexander Bokovoy (5):
>       Introduce platform-specific adaptation
>       Convert server install code to platform-independent access to system services
>       Convert client install code to platform-independent access to system services
>       Convert installation tools to platform-independent access to installation services
>       Add ipapython/services.py to auto-generated files and ignore it
> 
>  .gitignore                                |    2 +-
>  Makefile                                  |    8 +
>  freeipa.spec.in                           |    5 +
>  install/tools/ipa-ca-install              |    4 +-
>  install/tools/ipa-nis-manage              |   13 +-
>  install/tools/ipa-replica-install         |   15 +-
>  install/tools/ipa-server-install          |   11 +-
>  install/tools/ipactl                      |   43 ++++---
>  ipa-client/ipa-install/ipa-client-install |  211 +++++++++++++----------------
>  ipa-client/ipaclient/ntpconf.py           |    5 +-
>  ipapython/Makefile                        |    2 +-
>  ipapython/ipautil.py                      |   48 +-------
>  ipapython/platform/__init__.py            |   23 +++
>  ipapython/platform/base.py                |  150 ++++++++++++++++++++
>  ipapython/platform/redhat.py              |  176 ++++++++++++++++++++++++
>  ipapython/services.py.in                  |   48 +++++++
>  ipapython/setup.py.in                     |    2 +-
>  ipapython/sysrestore.py                   |    5 +-
>  ipaserver/install/bindinstance.py         |    2 +-
>  ipaserver/install/cainstance.py           |   26 +---
>  ipaserver/install/certs.py                |   25 ++--
>  ipaserver/install/dsinstance.py           |   19 +--
>  ipaserver/install/httpinstance.py         |    9 +-
>  ipaserver/install/krbinstance.py          |    3 +-
>  ipaserver/install/ntpinstance.py          |    5 +-
>  ipaserver/install/replication.py          |    4 +-
>  ipaserver/install/service.py              |   68 +++-------
>  27 files changed, 625 insertions(+), 307 deletions(-)
>  create mode 100644 ipapython/platform/__init__.py
>  create mode 100644 ipapython/platform/base.py
>  create mode 100644 ipapython/platform/redhat.py
>  create mode 100644 ipapython/services.py.in
> 
> 

Good job! This all looks very good, I found no installation error in
various scenarios I tried. I only found a problem with mixed tabs-spaces
indentation. You introduced it at least in install/tools/ipactl. You can
easily check these cases with:

$ ./make-lint --enable-noerror | grep W0312
ipa-client/ipa-install/ipa-client-install:221: [W0312] Found indentation with tabs instead of spaces
ipa-client/ipa-install/ipa-client-install:237: [W0312] Found indentation with tabs instead of spaces
...
install/tools/ipactl:194: [W0312] Found indentation with tabs instead of spaces
install/tools/ipactl:203: [W0312] Found indentation with tabs instead of spaces
install/tools/ipactl:241: [W0312] Found indentation with tabs instead of spaces
...

We don't have to fix the old indentation problems right in your patches
but we should not introduce new tab-spaces problems as they have a
potential to cause nasty problems.

Martin