[PATCH 1/2] Don't crash if incorrect environment is set in kickstart (#1234890)

Vratislav Podzimek vpodzime at redhat.com
Mon Jun 29 10:47:58 UTC 2015


On Fri, 2015-06-26 at 15:31 +0200, Martin Kolman wrote:
> Instead of crashing prompt the user to select a valid environment from
> the list in the software spoke. Also make sure the selected environment
> is written in the installation describing kickstart file.
> 
> Resolves: rhbz#1234890
> Signed-off-by: Martin Kolman <mkolman at redhat.com>
> ---
>  pyanaconda/ui/gui/spokes/software.py | 76 ++++++++++++++++++++++++++----------
>  1 file changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
> index 17498af..4801643 100644
> --- a/pyanaconda/ui/gui/spokes/software.py
> +++ b/pyanaconda/ui/gui/spokes/software.py
> @@ -67,7 +67,6 @@ class SoftwareSelectionSpoke(NormalSpoke):
>  
>          self.selectedGroups = []
>          self.excludedGroups = []
> -        self.environment = None
>  
>          self._environmentListBox = self.builder.get_object("environmentListBox")
>          self._addonListBox = self.builder.get_object("addonListBox")
> @@ -100,15 +99,44 @@ class SoftwareSelectionSpoke(NormalSpoke):
>      def _downloading_group_md(self):
>          hubQ.send_message(self.__class__.__name__, _("Downloading group metadata..."))
>  
> +    @property
> +    def environment(self):
> +        """A wrapper for the environment specification in kickstart"""
> +        return self.data.packages.environment
> +
> +    @environment.setter
> +    def environment(self, value):
> +        self.data.packages.environment = value
> +
> +    @property
> +    def environment_valid(self):
> +        """Return if the currently set environment is valid
> +        (represents an environment known by the payload)
> +        """
> +        # None means the environment has not been set by the user,
> +        # which means:
> +        # * set the default environment during interactive installation
> +        # * ask user to specify an environment during kickstart installation
> +        if self.environment is None:
> +            return True
> +        else:
> +            return self.environment in self.payload.environments
> +
>      def _payload_finished(self):
> -        self.environment = self.data.packages.environment
> +        if self.environment_valid:
> +            log.info("using environment from kickstart: %s", self.data.packages.environment)
AFAICT this may be misleading because self.environment_valid is True if
self.environment is None (and thus not set in kickstart at all). Or is
this callback triggered only when some environment was set/requested?

Also, it should use self.environment not self.data.packages.environment.

> +        else:
> +            log.error("unknown environment has been specified in kickstart and will be ignored: %s",
> +                      self.data.packages.environment)
> +            # False means that the environment has been set to an invalid value and needs to
> +            # be manually set to a valid one.
> +            self.environment = False
>  
>      def _payload_error(self):
>          hubQ.send_message(self.__class__.__name__, payloadMgr.error)
>  
>      def _apply(self):
> -        env = self._get_selected_environment()
> -        if not env:
> +        if not self.environment:
>              return
>  
>          addons = self._get_selected_addons()
> @@ -118,8 +146,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
>  
>          self._selectFlag = False
>          self.payload.data.packages.groupList = []
> -        self.payload.selectEnvironment(env)
> -        self.environment = env
> +        self.payload.selectEnvironment(self.environment)
>          for group in self.selectedGroups:
>              self.payload.selectGroup(group)
>  
> @@ -161,20 +188,21 @@ class SoftwareSelectionSpoke(NormalSpoke):
>          # we should always check processingDone before checking the other variables,
>          # as they might be inconsistent until processing is finished
>          if flags.automatedInstall:
> -            return processingDone and self.data.packages.seen
> +            # we can't let the automated installatin proceed until a valid environment
                                           ^typo

> +            # has been set
> +            return processingDone and self.data.packages.seen and self.environment_valid
>          else:
> -            return processingDone and self._get_selected_environment() is not None
> +            return processingDone and self.environment is not None
>  
>      @property
>      def changed(self):
> -        env = self._get_selected_environment()
> -        if not env:
> +        if not self.environment:
>              return True
>  
>          addons = self._get_selected_addons()
>  
>          # Don't redo dep solving if nothing's changed.
> -        if env == self._origEnvironment and set(addons) == set(self._origAddons) and \
> +        if self.environment == self._origEnvironment and set(addons) == set(self._origAddons) and \
>             self.txid_valid:
>              return False
>  
> @@ -211,17 +239,21 @@ class SoftwareSelectionSpoke(NormalSpoke):
>          if not self.txid_valid:
>              return _("Source changed - please verify")
>  
> -        env = self._get_selected_environment()
> -        if not env:
> +        if not self.environment:
>              # Kickstart installs with %packages will have a row selected, unless
>              # they did an install without a desktop environment.  This should
>              # catch that one case.
>              if flags.automatedInstall and self.data.packages.seen:
> -                return _("Custom software selected")
> +                if self.environment_valid:
> +                    return _("Custom software selected")
> +                else:
> +                    # It is impossible to set an invalid environment in the GUI,
> +                    # os any invalid environment must have come from kickstart.
                        ^^so?

> +                    return _("Invalid environment specified in kickstart")
>  
>              return _("Nothing selected")
>  
> -        return self.payload.environmentDescription(env)[0]
> +        return self.payload.environmentDescription(self.environment)[0]
>  
>      def initialize(self):
>          NormalSpoke.initialize(self)
> @@ -308,6 +340,14 @@ class SoftwareSelectionSpoke(NormalSpoke):
>          self._environmentListBox.show_all()
>          self._addonListBox.show_all()
>  
> +        # Pre-select the default environment when the user enters the software spoke
> +        # to fix an invalid environment set in kickstart. Otherwise the default environment
> +        # would still look as selected but the selection would not be applied after returning
> +        # to the hub.
> +        if flags.automatedInstall and self.environment is None:
> +            row = self._environmentListBox.get_row_at_index(0)
> +            row.activate()
Shouldn't this check 'self.environment is False' as set above in
_payload_finished()?

-- 
Vratislav Podzimek

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



More information about the anaconda-patches mailing list