[Freeipa-devel] [PATCH 0135] Fix incorrect path in error message on sysrestore failure

[Petr Viktorin]

On 12/19/2013 05:00 PM, Tomas Babej wrote:
> Yes,
>
> this is planned as part of https://fedorahosted.org/freeipa/ticket/4052

Yeah, the path also appears in client-install and a dozen other files. 
Let's leave it to that ticket.

> NACKs are welcome! It's a valid point, so I guess be fixing it now here
> wouldn't hurt.
>
> Updated patch attached.
>
> Tomas

Thanks for the patch!

I could think of a less redundant name than SYSRESTORE_DIR_PATH, but 
that's an insignificant nitpick.

ACK, pushed to master:  2a2f5ac4e68ed9a819ddc3de1d35366b2e2b58c7



> On 12/13/2013 08:49 AM, Petr Spacek wrote:
>> On 12.12.2013 15:02, Tomas Babej wrote:
>>> On sysrestore failure, user is prompted out to remove the sysrestore
>>> file. However, the path to the sysrestore file mentioned in the
>>> sentence is not correct.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4080
>>>
>>> --
>>> Tomas Babej
>>>
>>>
>>> freeipa-tbabej-0135-Fix-incorrect-path-in-error-message-on-sysrestore-fa.patch
>>>
>>>
>>>
>>>   From eac993e153c243b6359f57a7c051d3f373a9add0 Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej<tbabej at redhat.com>
>>> Date: Thu, 12 Dec 2013 15:01:14 +0100
>>> Subject: [PATCH] Fix incorrect path in error message on sysrestore
>>> failure
>>>
>>> On sysrestore failure, user is prompted out to remove the sysrestore
>>> file. However, the path to the sysrestore file mentioned in the
>>> sentence is not correct.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4080
>>> ---
>>>    install/tools/ipa-server-install | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/install/tools/ipa-server-install
>>> b/install/tools/ipa-server-install
>>> index
>>> 458ebba550d0fe7675bd874e23c7d730c53297e6..718fcee45550b9f65a17ecddc599fb4489f7ab3c
>>> 100755
>>> --- a/install/tools/ipa-server-install
>>> +++ b/install/tools/ipa-server-install
>>> @@ -534,7 +534,10 @@ def uninstall():
>>>                rv = 1
>>>
>>>        if has_state:
>>> -        root_logger.error('Some installation state has not been
>>> restored.\nThis may cause re-installation to fail.\nIt should be safe
>>> to remove /var/lib/ipa/sysrestore.state but it may\nmean your system
>>> hasn\'t be restored to its pre-installation state.')
>>> +        root_logger.error('Some installation state has not been
>>> restored.\n'
>>> +                          'This may cause re-installation to fail.\n'
>>> +                          'It should be safe to remove
>>> /var/lib/ipa/sysrestore/sysrestore.state but it may\n'
>>
>> (I know that this is bold ...) NACK.
>>
>> A path used in the error message should be extracted/shared with the
>> code. It will prevent inconsistencies like this in the future.
>>
>> Petr^2 Spacek
>>
>>> +                          'mean your system hasn\'t be restored to
>>> its pre-installation state.')
>>>
>>>        # Note that this name will be wrong after the first uninstall.
>>>        dirname =
>>> dsinstance.config_dirname(dsinstance.realm_to_serverid(api.env.realm))
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
>


-- 
Petr³