[anaconda][rhel7-branch/master][v2][PATCH] Add support for autostep and --autoscreenshot (#1059295)

Vratislav Podzimek vpodzime at redhat.com
Wed Oct 1 09:14:15 UTC 2014


On Mon, 2014-09-29 at 21:12 +0200, Martin Kolman wrote:
> Add the missing support for the autostep [--autoscreenshot] kickstart
> command to Anaconda. If just autostep is specified, Anaconda will
> iterate over all active spokes just before leaving a hub. If the
> --autoscreenshot option specified, it will also take a screenshot of
> all hubs and of each spoke.
> 
> Also turn the take-screenshot code into a proper function that can be
> called when auto-screenshotting and remove the old copy-screenshot
> post-script (the last batch of screenshots is taken after the
> postscripts have run, so screenshot copying is now handled
> elsewhere).
> 
> Resolves: rhbz#1059295
> Signed-off-by: Martin Kolman <mkolman at redhat.com>
> ---
>  data/post-scripts/90-copy-screenshots.ks |  10 ---
>  pyanaconda/constants.py                  |   4 +
>  pyanaconda/iutil.py                      |  23 ++++++
>  pyanaconda/ui/gui/__init__.py            | 135 ++++++++++++++++++++++++++-----
>  pyanaconda/ui/gui/hubs/__init__.py       |  45 +++++++++++
>  pyanaconda/ui/gui/spokes/__init__.py     |   9 +++
>  pyanaconda/ui/gui/spokes/welcome.py      |   5 +-
>  7 files changed, 201 insertions(+), 30 deletions(-)
>  delete mode 100644 data/post-scripts/90-copy-screenshots.ks
> 
> diff --git a/data/post-scripts/90-copy-screenshots.ks b/data/post-scripts/90-copy-screenshots.ks
> deleted file mode 100644
> index 471e2b1..0000000
> --- a/data/post-scripts/90-copy-screenshots.ks
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -%post --nochroot
> -
> -if [ ! -d /tmp/anaconda-screenshots ]; then
> -    exit 0
> -fi
> -
> -mkdir -m 0750 -p $ANA_INSTALL_PATH/root/anaconda-screenshots
> -cp -a /tmp/anaconda-screenshots/*.png $ANA_INSTALL_PATH/root/anaconda-screenshots/
> -
> -%end
> diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
> index 2c8ca11..45b2ca8 100644
> --- a/pyanaconda/constants.py
> +++ b/pyanaconda/constants.py
> @@ -150,3 +150,7 @@ IPMI_FAILED   = 0xA
>  
>  # Recognizing a tarfile
>  TAR_SUFFIX = (".tar", ".tbz", ".tgz", ".txz", ".tar.bz2", "tar.gz", "tar.xz")
> +
> +# screenshots
> +SCREENSHOTS_DIRECTORY = "/tmp/anaconda-screenshots"
> +SCREENSHOTS_TARGET_DIRECTORY = "/root/anaconda-screenshots"
> diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> index d30947a..b13ce09 100644
> --- a/pyanaconda/iutil.py
> +++ b/pyanaconda/iutil.py
> @@ -28,11 +28,13 @@ import errno
>  import subprocess
>  import tempfile
>  import unicodedata
> +import shutil
>  from threading import Thread
>  from Queue import Queue, Empty
>  
>  from pyanaconda.flags import flags
>  from pyanaconda.constants import DRACUT_SHUTDOWN_EJECT, ROOT_PATH, TRANSLATIONS_UPDATE_DIR, UNSUPPORTED_HW
> +from pyanaconda.constants import SCREENSHOTS_DIRECTORY, SCREENSHOTS_TARGET_DIRECTORY
>  from pyanaconda.regexes import PROXY_URL_PARSE
>  
>  import logging
> @@ -856,3 +858,24 @@ def ipmi_report(event):
>      execWithCapture("ipmitool", ["sel", "add", path])
>  
>      os.remove(path)
> +
> +def save_screenshots():
> +    """Save screenshots to the installed system"""
> +    if not os.path.exists(SCREENSHOTS_DIRECTORY):
> +        # there are no screenshots to copy
> +        return
> +    target_path = os.path.normpath(getSysroot() + SCREENSHOTS_TARGET_DIRECTORY)
I think a function like 'iutil.sysroot_path(path)' that would correctly
make the 'path' argument "sysrooted" whether it is a relative or an
absolute path could be useful.

> +    log.info("saving screenshots taken during the installation to:")
> +    log.info(target_path)
This should be a single log.info() call.

> +    try:
> +        # create the screenshots directory
> +        if not os.path.exists(target_path):
> +            # it is unlikely the directory exists as we format the root directory,
> +            # but better be safe here
> +            os.makedirs(target_path)
Believe or not, but we have a function iutil.mkdirChain that does this
with the check included.

> +        # copy all screenshots
> +        for filename in os.listdir(SCREENSHOTS_DIRECTORY):
> +            shutil.copy(os.path.join(SCREENSHOTS_DIRECTORY, filename), target_path)
> +
> +    except OSError:
> +        log.exception("saving screenshots to installed system failed")
> diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py
> index b64d983..9bc6a0c 100644
> --- a/pyanaconda/ui/gui/__init__.py
> +++ b/pyanaconda/ui/gui/__init__.py
> @@ -25,10 +25,10 @@ from gi.repository import Gdk, Gtk, AnacondaWidgets, Keybinder
>  
>  from pyanaconda.i18n import _
>  from pyanaconda.constants import IPMI_ABORTED
> -from pyanaconda import product, iutil
> +from pyanaconda import product, iutil, constants
>  
>  from pyanaconda.ui import UserInterface, common
> -from pyanaconda.ui.gui.utils import enlightbox, gtk_action_wait, busyCursor, unbusyCursor
> +from pyanaconda.ui.gui.utils import enlightbox, gtk_action_wait, gtk_call_once, busyCursor, unbusyCursor
>  import os.path
>  
>  import logging
> @@ -84,7 +84,7 @@ class GUIObject(common.UIObject):
>      uiFile = ""
>      translationDomain = "anaconda"
>  
> -    screenshots_directory = "/tmp/anaconda-screenshots"
> +    handles_autostep = False
>  
>      def __init__(self, data):
>          """Create a new UIObject instance, including loading its uiFile and
> @@ -132,6 +132,9 @@ class GUIObject(common.UIObject):
>          Keybinder.init()
>          Keybinder.bind("<Shift>Print", self._handlePrntScreen, [])
>  
> +        self._automaticEntry = False
> +        self._autostepRunning = False
> +        self._autostepDone = False
>  
>      def _findUIFile(self):
>          path = os.environ.get("UIPATH", "./:/tmp/updates/:/tmp/updates/ui/:/usr/share/anaconda/ui/")
> @@ -148,29 +151,107 @@ class GUIObject(common.UIObject):
>          raise IOError("Could not load UI file '%s' for object '%s'" % (self.uiFile, self))
>  
>      def _handlePrntScreen(self, *args, **kwargs):
> -        global _screenshotIndex
>          global _last_screenshot_timestamp
>          # as a single press of the assigned key generates
>          # multiple callbacks, we need to skip additional
>          # callbacks for some time once a screenshot is taken
>          if (time.time() - _last_screenshot_timestamp) >= SCREENSHOT_DELAY:
> -            # Make sure the screenshot directory exists.
> -            if not os.access(self.screenshots_directory, os.W_OK):
> -                os.makedirs(self.screenshots_directory)
> -
> -            fn = os.path.join(self.screenshots_directory,
> -                              "screenshot-%04d.png" % _screenshotIndex)
> -
> -            root_window = Gdk.get_default_root_window()
> -            pixbuf = Gdk.pixbuf_get_from_window(root_window, 0, 0,
> -                                                root_window.get_width(),
> -                                                root_window.get_height())
> -            pixbuf.savev(fn, 'png', [], [])
> -            log.info("screenshot nr. %d taken", _screenshotIndex)
> -            _screenshotIndex += 1
> +            self.take_screenshot()
>              # start counting from the time the screenshot operation is done
>              _last_screenshot_timestamp = time.time()
>  
> +    def take_screenshot(self, name=None):
> +        """Take a screenshot of the whole screen (works even with multiple displays)
> +
> +        :param name: optional name for the screenshot that will be appended to the filename,
> +                     after the standard prefix & screenshot number
> +        :type name: str or NoneType
> +        """
> +        global _screenshotIndex
> +        # Make sure the screenshot directory exists.
> +        if not os.access(constants.SCREENSHOTS_DIRECTORY, os.W_OK):
> +            os.makedirs(constants.SCREENSHOTS_DIRECTORY)
iutil.mkdirChain could be used here as well.

> +
> +        if name is None:
> +            screenshot_filename = "screenshot-%04d.png" % _screenshotIndex
> +        else:
> +            screenshot_filename = "screenshot-%04d-%s.png" % (_screenshotIndex, name)
> +
> +        fn = os.path.join(constants.SCREENSHOTS_DIRECTORY, screenshot_filename)
> +
> +        root_window = Gdk.get_default_root_window()
> +        pixbuf = Gdk.pixbuf_get_from_window(root_window, 0, 0,
> +                                            root_window.get_width(),
> +                                            root_window.get_height())
> +        pixbuf.savev(fn, 'png', [], [])
> +        log.info("%s taken", screenshot_filename)
> +        _screenshotIndex += 1
> +
> +    @property
> +    def automaticEntry(self):
> +        """Report if the given GUIObject has been displayed under automatic control
> +
> +        This is needed for example for installations with an incomplete kickstart,
> +        as we need to differentiate the automatic screenshot pass from the user
> +        entering a spoke to manually configure things. We also need to skip applying
> +        changes if the spoke is entered automatically.
> +        """
> +        return self._automaticEntry
> +
> +    @automaticEntry.setter
> +    def automaticEntry(self, value):
> +        self._automaticEntry = value
> +
> +    @property
> +    def autostepRunning(self):
> +        """Report if the GUIObject is currently running autostep"""
> +        return self._autostepRunning
> +
> +    @autostepRunning.setter
> +    def autostepRunning(self, value):
> +        self._autostepRunning = value
> +
> +    @property
> +    def autostepDone(self):
> +        """Report if autostep for this GUIObject has been finished"""
> +        return self._autostepDone
> +
> +    @autostepDone.setter
> +    def autostepDone(self, value):
> +        self._autostepDone = value
> +
> +    def autostep(self):
> +        """Autostep through this graphical object and through
> +        any graphical objects managed by it (such as through spokes for a hub)
> +        """
> +        # report that autostep is running to prevent another from starting
> +        self.autostepRunning = True
> +        # take a screenshot of the current graphical object
> +        if self.data.autostep.autoscreenshot:
> +            # as autostep is triggered just before leaving a screen,
> +            # we can safely take a screenshot of the "parent" object at once
> +            # without using idle_add
> +            self.take_screenshot("%s" % self.__class__.__name__)
Is self.__class__.__name__ something else than a string that we need to
convert it to a string? Otherwise you can drop the string formatting
here.

> +        self._doAutostep()
> +        # done
> +        self.autostepRunning = False
> +        self.autostepDone = True
> +        self._doPostAutostep()
> +
> +    def _doPostAutostep(self):
> +        """To be overridden by the given GUIObject sub-class with custom code
> +        that brings the GUI from the autostepping mode back to the normal mode.
> +        This usually means to "click" the continue button or its equivalent.
> +        """
> +        pass
> +
> +    def _doAutostep(self):
> +        """To be overridden by the given GUIObject sub-class with customized
> +        autostepping code - if needed
> +        (this is for example used to step though spokes in a hub)
                                            ^through

> +        """
> +        pass
> +
>      @property
>      def window(self):
>          """Return the top-level object out of the GtkBuilder representation
> @@ -447,8 +528,26 @@ class GraphicalUserInterface(UserInterface):
>      ### SIGNAL HANDLING METHODS
>      ###
>      def _on_continue_clicked(self):
> +        # Autostep needs to be triggered just before switching to the next screen
> +        # (or before quiting the installation if there are no more screens) to be consistent
> +        # in both fully automatic kickstart installation and for installation with an incomplete
> +        # kickstart. Therefore we basically "hook" the continue-clicked signal, start autostepping
> +        # and ignore any other continue-clicked signals until autostep is done.
> +        # Once autostep finishes, it emits the appropriate continue-clicked signal itself,
> +        # switching to the next screen (if any).
> +        if self.data.autostep.seen and self._currentAction.handles_autostep:
> +            if self._currentAction.autostepRunning:
> +                log.debug("ignoring the continue-clicked signal - autostep is running")
> +                return
> +            elif not self._currentAction.autostepDone:
> +                self._currentAction.autostep()
> +                return
> +
>          # If we're on the last screen, clicking Continue quits.
>          if len(self._actions) == 1:
> +            # save the screenshots to the installed system before killing Anaconda
> +            # (the kickstart post scripts run to early, so we need to copy the screenshots now)
> +            iutil.save_screenshots()
>              Gtk.main_quit()
>              return
>  
> diff --git a/pyanaconda/ui/gui/hubs/__init__.py b/pyanaconda/ui/gui/hubs/__init__.py
> index ce71929..00be429 100644
> --- a/pyanaconda/ui/gui/hubs/__init__.py
> +++ b/pyanaconda/ui/gui/hubs/__init__.py
> @@ -60,6 +60,8 @@ class Hub(GUIObject, common.Hub):
>         additional standalone screens either before or after them.
>      """
>  
> +    handles_autostep = True
> +
>      def __init__(self, data, storage, payload, instclass):
>          """Create a new Hub instance.
>  
> @@ -111,6 +113,11 @@ class Hub(GUIObject, common.Hub):
>          action.window.set_transient_for(self.window)
>          action.window.show_all()
>  
> +        # step through the spoke if required
> +        if action.automaticEntry:
> +            # we need to use idle_add here to give GTK time to render the spoke
> +            gtk_call_once(self._autostep_spoke, action)
> +
>          # Start a recursive main loop for this spoke, which will prevent
>          # signals from going to the underlying (but still displayed) Hub and
>          # prevent the user from switching away.  It's up to the spoke's back
> @@ -118,6 +125,11 @@ class Hub(GUIObject, common.Hub):
>          Gtk.main()
>          action.window.set_transient_for(None)
>  
> +        # don't apply any actions if the spoke was visited automatically
> +        if action.automaticEntry:
> +            action.automaticEntry = False
> +            return
> +
>          action._visitedSinceApplied = True
>  
>          # Don't take _visitedSinceApplied into account here.  It will always be
> @@ -394,6 +406,39 @@ class Hub(GUIObject, common.Hub):
>      def quitButton(self):
>          return None
>  
> +    def _doAutostep(self):
> +        """Autostep through all spokes managed by this hub"""
> +        log.info("autostepping through all spokes on hub %s" % self.__class__.__name__)
> +        index = 1
> +        spokes_kv = self._spokes.items()
> +        spoke_count = len(spokes_kv)
> +        spokes_kv.sort(key=lambda x:x[0])
I think this could be rewritten as:
spokes_count = len(self._spokes)
spokes_kv = sorted(self._spokes.items(), key=lambda x:x[0])

> +        # take a screenshot of all spokes in the alphabetical order of their names
> +        for spoke_name, spoke in spokes_kv:
You can use enumerate() here and define the index variable here. The 'i'
would be a better name then.

> +            log.debug("stepping to spoke %s (%d/%d)" % (spoke_name, index, spoke_count))
> +            spoke.automaticEntry = True
> +            self._on_spoke_clicked(None, None, spoke)
> +            index += 1
> +        log.info("autostep for hub %s finished" % self.__class__.__name__)
> +
> +    def _autostep_spoke(self, spoke):
> +        """Step through a spoke and make a screenshot if required and
> +        then kill it's recursive GTK main loop. :)
> +        This will return control back to the primary main loop,
> +        so it can show another spoke or got to next hub.
> +        """
> +        from gi.repository import Gtk
> +        # it might be possible that autostep is specified, but autoscreenshot isn't
> +        if self.data.autostep.autoscreenshot:
> +            self.take_screenshot(spoke.__class__.__name__)
> +        spoke.window.hide()
> +        Gtk.main_quit()
> +
> +    def _doPostAutostep(self):
> +        # we are done, re-emit the continue clicked signal we "consumed" previously
> +        # so that the Anaconda GUI can switch to the next screen
> +        gtk_call_once(self.continueButton.emit, "clicked")
> +
>      ### SIGNAL HANDLERS
>  
>      def register_event_cb(self, event, cb):
> diff --git a/pyanaconda/ui/gui/spokes/__init__.py b/pyanaconda/ui/gui/spokes/__init__.py
> index 32a1469..75231ac 100644
> --- a/pyanaconda/ui/gui/spokes/__init__.py
> +++ b/pyanaconda/ui/gui/spokes/__init__.py
> @@ -22,6 +22,7 @@
>  from pyanaconda.ui import common
>  from pyanaconda.ui.common import collect
>  from pyanaconda.ui.gui import GUIObject
> +from pyanaconda.ui.gui.utils import gtk_call_once
>  import os.path
>  
>  __all__ = ["StandaloneSpoke", "NormalSpoke", "PersonalizationSpoke",
> @@ -65,6 +66,9 @@ class Spoke(GUIObject):
>  # Inherit abstract methods from common.StandaloneSpoke
>  # pylint: disable-msg=W0223
>  class StandaloneSpoke(Spoke, common.StandaloneSpoke):
> +
> +    handles_autostep = True
> +
>      def __init__(self, data, storage, payload, instclass):
>          Spoke.__init__(self, data)
>          common.StandaloneSpoke.__init__(self, data, storage, payload, instclass)
> @@ -79,6 +83,11 @@ class StandaloneSpoke(Spoke, common.StandaloneSpoke):
>          elif event == "quit":
>              self.window.connect("quit-clicked", lambda *args: cb())
>  
> +    def _doPostAutostep(self):
> +        # we are done, re-emit the continue clicked signal we "consumed" previously
> +        # so that the Anaconda GUI can switch to the next screen
> +        gtk_call_once(self.window.emit, "continue-clicked")
> +
>  # Inherit abstract methods from common.NormalSpoke
>  # pylint: disable-msg=W0223
>  class NormalSpoke(Spoke, common.NormalSpoke):
> diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py
> index 548dd02..80017f6 100644
> --- a/pyanaconda/ui/gui/spokes/welcome.py
> +++ b/pyanaconda/ui/gui/spokes/welcome.py
> @@ -273,8 +273,9 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
>      # Override the default in StandaloneSpoke so we can display the beta
>      # warning dialog first.
>      def _on_continue_clicked(self, cb):
> -        # Don't display the betanag dialog if this is the final release.
> -        if not isFinal:
> +        # Don't display the betanag dialog if this is the final release or
> +        # when autostep has been requested as betanag breaks the autostep logic.
> +        if not isFinal and not self.data.autostep.seen:
>              dlg = self.builder.get_object("betaWarnDialog")
>              with enlightbox(self.window, dlg):
>                  rc = dlg.run()
Otherwise this looks good to me as long as it works.

-- 
Vratislav Podzimek

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



More information about the anaconda-patches mailing list