Yes,

this is planned as part of https://fedorahosted.org/freeipa/ticket/4052

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

Updated patch attached.

Tomas


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<tba...@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@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

>From 2dcdfd94769340d2d507ec4a2ab073349453c98b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@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 | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 458ebba550d0fe7675bd874e23c7d730c53297e6..9c9b205ad5309c578b00b01ef3af00fc9930c865 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -89,6 +89,8 @@ VALID_SUBJECT_ATTRS = ['st', 'o', 'ou', 'dnqualifier', 'c',
                        'incorporationlocality', 'incorporationstate',
                        'incorporationcountry', 'businesscategory']
 
+SYSRESTORE_DIR_PATH = '/var/lib/ipa/sysrestore'
+
 def subject_callback(option, opt_str, value, parser):
     """
     Make sure the certificate subject base is a valid DN
@@ -525,16 +527,19 @@ def uninstall():
     sysupgrade.remove_upgrade_file()
 
     if fstore.has_files():
-        root_logger.error('Some files have not been restored, see /var/lib/ipa/sysrestore/sysrestore.index')
+        root_logger.error('Some files have not been restored, see %s/sysrestore.index' % SYSRESTORE_DIR_PATH)
     has_state = False
     for module in IPA_MODULES: # from installutils
         if sstore.has_state(module):
-            root_logger.error('Some installation state for %s has not been restored, see /var/lib/ipa/sysrestore/sysrestore.state' % module)
+            root_logger.error('Some installation state for %s has not been restored, see %s/sysrestore.state' % (module, SYSRESTORE_DIR_PATH))
             has_state = True
             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 %s/sysrestore.state but it may\n'
+                          'mean your system hasn\'t be restored to its pre-installation state.' % SYSRESTORE_DIR_PATH)
 
     # Note that this name will be wrong after the first uninstall.
     dirname = dsinstance.config_dirname(dsinstance.realm_to_serverid(api.env.realm))
@@ -603,9 +608,9 @@ def main():
     root_logger.debug("missing options might be asked for interactively later\n")
 
     global fstore
-    fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore')
+    fstore = sysrestore.FileStore(SYSRESTORE_DIR_PATH)
     global sstore
-    sstore = sysrestore.StateFile('/var/lib/ipa/sysrestore')
+    sstore = sysrestore.StateFile(SYSRESTORE_DIR_PATH)
 
     # Configuration for ipalib, we will bootstrap and finalize later, after
     # we are sure we have the configuration file ready.
-- 
1.8.4.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to