[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [f17-branch master] Be more verbose about upgrade failures (#735060)



Ack, but see a comment below.

On Thu, 2012-02-09 at 13:51 -0800, Brian C. Lane wrote:
> From: "Brian C. Lane" <bcl redhat com>
> 
> When an installed system cannot be upgraded it may be helpful to know if
> it was the product, version or arch that didn't match. This propagates
> that info up from productUpgradable so that it can be show to the user.
> ---
>  pyanaconda/installclass.py     |   12 +++++++++++-
>  pyanaconda/storage/__init__.py |   22 +++++++++++++++++-----
>  pyanaconda/upgrade.py          |   15 +++++++++++----
>  3 files changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/pyanaconda/installclass.py b/pyanaconda/installclass.py
> index f2774da..8aea1bb 100644
> --- a/pyanaconda/installclass.py
> +++ b/pyanaconda/installclass.py
> @@ -201,6 +201,11 @@ class BaseInstallClass(object):
>          pass
>  
>      def productUpgradable(self, arch, oldprod, oldver):
> +        """ Return a tuple with:
> +            (Upgradable True|False, dict of tests and status)
> +
> +            The dict has True|False for: product, version, arch tests.
> +        """
>          def archesEq(a, b):
>              import re
>  
> @@ -209,7 +214,12 @@ class BaseInstallClass(object):
>              else:
>                  return a == b
>  
> -        return self.productMatches(oldprod) and self.versionMatches(oldver) and archesEq(arch, productArch)
> +        result = { "product" : self.productMatches(oldprod),
> +                   "version"  : self.versionMatches(oldver),
> +                   "arch"     : archesEq(arch, productArch)
> +                 }
> +
> +        return (all(result.values()), result)
>  
>      def setNetworkOnbootDefault(self, network):
>          pass
> diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py
> index f52d7b7..9f1601d 100644
> --- a/pyanaconda/storage/__init__.py
> +++ b/pyanaconda/storage/__init__.py
> @@ -1554,9 +1554,20 @@ def getReleaseString():
>      return (relArch, relName, relVer)
>  
>  def findExistingRootDevices(anaconda, upgradeany=False):
> -    """ Return a tuple of:
> -        list of all root filesystems in the device tree.
> +    """ upgradeany will cause it to ignore version, product and
> +    arch mismatches. Use with caution.
> +
> +    Return a tuple of:
> +        list of all upgradable root filesystems.
>          list of previous installs that cannot be upgraded.
> +
> +    The upgradable tuple is:
> +        (device, "%product %version")
> +
> +    The non-upgradable tuple is:
> +        (product, version, arch, device.name, test_dict)
> +        The test_dict contains the results of the upgrade test for the
> +        product, version and arch.
>      """
>      rootDevs = []
>      notUpgradable = []
> @@ -1605,13 +1616,14 @@ def findExistingRootDevices(anaconda, upgradeany=False):
>                  log.info("findExistingRootDevices: no arch.")
>                  continue
>  
> -            if upgradeany or \
> -               anaconda.instClass.productUpgradable(arch, product, version):
> +            upgradable = anaconda.instClass.productUpgradable(arch, product, version)
What about: 
(upgradable, tests) = anaconda.instClass.productUpgradable(arch...)?
Or without brackets. Since both items from the tuple are used I think we
could improve readability.

> +            if upgradeany or upgradable[0]:
>                  rootDevs.append((device, "%s %s" % (product, version)))
>              else:
> -                notUpgradable.append((product, version, device.name))
> +                notUpgradable.append((product, version, arch, device.name, upgradable[1]))
>                  log.info("product %s, version %s, arch %s found on %s is not upgradable"
>                           % (product, version, arch, device.name))
> +                log.debug("test results: %s" % (upgradable[1],))
>          else:
>              # this handles unmounting the filesystem
>              device.teardown(recursive=True)
> diff --git a/pyanaconda/upgrade.py b/pyanaconda/upgrade.py
> index 91be3c2..a15f1c2 100644
> --- a/pyanaconda/upgrade.py
> +++ b/pyanaconda/upgrade.py
> @@ -82,11 +82,18 @@ def findRootParts(anaconda):
>  
>          if notUpgradable and not anaconda.rootParts:
>              oldInstalls = ""
> -            for info in notUpgradable:
> -                if None in info[:2]:
> -                    oldInstalls += _("Unknown release on %s") % (info[2])
> +            for product, version, arch, name, tests in notUpgradable:
> +                if None in (product, version):
> +                    oldInstalls = _("Unknown release on %s -") % (name)
>                  else:
> -                    oldInstalls += "%s %s on %s" % (info)
> +                    oldInstalls = "%s %s %s on %s -" % (product, version, arch, name)
> +
> +                if not tests["product"]:
> +                    oldInstalls += _(" Product mismatch.")
> +                if not tests["version"]:
> +                    oldInstalls += _(" Version mismatch.")
> +                if not tests["arch"]:
> +                    oldInstalls += _(" Architecture mismatch.")
>                  oldInstalls += "\n"
>              rc = anaconda.intf.messageWindow(_("Cannot Upgrade"),
>                      _("Your current installation cannot be upgraded. This "


-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]