[PATCH 1/2] Don't crash if incorrect environment is set in kickstart (#1234890)
Martin Kolman
mkolman at redhat.com
Tue Jun 30 11:07:24 UTC 2015
On Mon, 2015-06-29 at 12:47 +0200, Vratislav Podzimek wrote:
> 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?
I think this triggers when the source changes - so on startup on when
the user changes the installation source during the installation.
>From this point of view environment being None is valid - it means it
has not been set and a default value will be used. And if there is no
%package section at all the user will be prompted to verify that the
default environment is OK.
>
> Also, it should use self.environment not
> self.data.packages.environment.
Fixing. :)
>
> > + 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
Fixing. :)
>
> > + # 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?
Also fixing. :)
>
> > + 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()?
>
I think I'll refactor this part a bit more because the current code
here is too insane.
More information about the anaconda-patches
mailing list