[Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring

Wed Jun 25 19:48:01 UTC 2014

On 06/25/2014 09:35 PM, Petr Viktorin wrote:
> On 06/25/2014 07:16 PM, Tomas Babej wrote:
>>
>> On 06/25/2014 04:59 PM, Tomas Babej wrote:
>>>
>>> On 06/25/2014 04:13 PM, Tomas Babej wrote:
>>>>
>>>> On 06/25/2014 04:01 PM, Tomas Babej wrote:
>>>>>
>>>>> On 06/25/2014 10:48 AM, Petr Viktorin wrote:
>>>>>> On 06/19/2014 03:52 PM, Tomas Babej wrote:
>>>>>>>
>>>>>>> On 06/19/2014 12:52 PM, Tomas Babej wrote:
>>>>>>>> On 06/18/2014 10:52 AM, Petr Viktorin wrote:
>>>>>>>>> On 06/17/2014 02:15 PM, Tomas Babej wrote:
>>>>>>>>>> On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
>>>>>>>>>>> On 17.06.2014 11:16, Martin Kosek wrote:
>>>>>>>>>> Attached is a new version of patch 226, and a new patch 228,
>>>>>>>>>> which moves
>>>>>>>>>> the paths from installers to the paths module.
>>>>>>>>> In patch 226, there's another "certificated" typo in
>>>>>>>>> remove_ca_cert_from_systemwide_ca_store
>>>>>>>>>
>>>>>>>>>> I greped the repository, and I do not see many paths lurking
>>>>>>>>>> around any
>>>>>>>>>> more, there are only some in the error messages (as these
>>>>>>>>>> can't be
>>>>>>>>>> reliably replaced automatically, and will need some manual
>>>>>>>>>> love).
>>>>>>>>>>
>>>>>>>>>> If you see any forgotten paths, which should be added to the
>>>>>>>>>> module, let
>>>>>>>>>> me know.
>>>>>>
>>>>>> Well, since you asked...
>>>>>>
>>>>>> install/tools/ipa-upgradeconfig:236:
>>>>>> ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib'
>>>>>> ipaserver/install/cainstance.py:1330: "-pki_instance_root=/var/lib",
>>>>>>
>>>>>> ipaserver/install/dsinstance.py:209:InstallLdifFile=
>>>>>> /var/lib/dirsrv/boot.ldif
>>>>>> ipaserver/install/dsinstance.py:210:inst_dir=
>>>>>> /var/lib/dirsrv/scripts-$SERVERID
>>>>>>
>>>>>> ipaserver/install/ipa_backup.py:464:
>>>>>> '--exclude=/var/lib/ipa/backup',
>>>>>>
>>>>>> ipatests/test_integration/tasks.py:451: host.run_command("find
>>>>>> /var/lib/sss/db -name '*.ldb' | "
>>>>>>
>>>>>> install/tools/ipa-replica-conncheck:403:
>>>>>> "/usr/sbin/ipa-replica-conncheck " +
>>>>>> install/tools/ipa-replica-conncheck:414:
>>>>>> print_info("/usr/sbin/ipa-replica-conncheck " + "
>>>>>> ".join(remote_check_opts))
>>>>>>
>>>>>> ipapython/ipautil.py:296:        env["PATH"] =
>>>>>> "/bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin"
>>>>>>
>>>>>> ipaserver/install/cainstance.py:88:ConfigFile =
>>>>>> /usr/share/pki/ca/conf/database.ldif
>>>>>>
>>>>>> ipaserver/install/bindinstance.py:829:
>>>>>> ipautil.run(['/usr/libexec/generate-rndc-key.sh'])
>>>>>>
>>>>>
>>>>> /me will think twice about teasing nex time.
>>>>>
>>>>> This are paths requiring manual changes in one way or the other and
>>>>> as such cannot be handled by my tool. Let's not stall the patcheset
>>>>> on this. We can fix these (and surely there are other) as we go
>>>>> along.
>
> OK, not a reason to hold the patch back.
> But, the fact that the tool can't handle them doesn't make them less
> important. Let's keep the ticket open, or open a new one.
>
>>>>>> I guess it'll be a while before we catch them all, but now it's at
>>>>>> least clear where these paths should be, so anyone porting to
>>>>>> another distro can send patches (or tickets) upstream.
>>>>>>
>>>>>>>>> I see another duplicate:
>>>>>>>>>      SSS_KRB5_INCLUDE_D = "/var/lib/sss/pubconf/krb5.include.d"
>>>>>>>>>      SSSD_PUBCONF_KRB5_INCLUDE_D_DIR =
>>>>>>>>> "/var/lib/sss/pubconf/krb5.include.d/"
>>>>>>
>>>>>> Could you just pick one instead? Would ipa_backup.py break if it
>>>>>> had a trailing slash here?
>>>>>>
>>>>>
>>>>> Yes. I verified it produces the same result with or without trailing
>>>>> slash, fixed.
>>>>>
>>>>>
>>>>>> In ipa-client-install, if you set:
>>>>>>     NSSWITCH_CONF = paths.NSSWITCH_CONF
>>>>>> then you should only use one of those later. (Preferably paths.*,
>>>>>> to get rid of the redundant constants.)
>>>>>> Perhaps this is for another patch that would clean up all the cases
>>>>>> where these trivial module variables are used.
>>>>>>
>>>>>
>>>>> I agree. Fixed this occurence.
>
> I've opened an easyfix ticket for the rest:
> https://fedorahosted.org/freeipa/ticket/4399
>
>>>>>>>> Fixed all mentioned issues. I also attached a patch 230, which
>>>>>>>> removes
>>>>>>>> the base Authconfig class.
>>>>>>
>>>>>>
>>>>>>> Attaching one additional patch, which removes unnecessary build
>>>>>>> warnings.
>>>>>>>
>>>>>>
>>>>>> 226, 230, 231 look good
>>>>>>
>>>>>
>>>>> Attaching whole updated patchset.
>>>>
>>>> Attaching one more patch which should fix broken CI tests.
>>>>
>>>
>>> Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in
>>> ipa-client-install, fixed now.
>>>
>>> Attaching the whole patchset for your convenience.
>>> -- 
>> Attaching a correct patchset this time.
>
> I hate to break it to you, but...
>             you sent the wrong patch 228 :(
>

No way! That's a official fail combo on my part. Here's the correct patch.

I checked and it applies cleanly on master.

>
> They're pretty independent, and there are fixes for failing tests, so
> I went ahead with the other ones.
>
> 226, 230, 231, 235: ACK, pushed to master:
> c8511d3b3baa389069156bf9991a9f4c7d64cf4a
>

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0228-6-ipaplatform-Move-paths-from-installers-to-paths-modu.patch
Type: text/x-patch
Size: 62606 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140625/39a39756/attachment.bin>