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(a)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.