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

Martin Kolman mkolman at redhat.com
Wed Oct 1 13:52:41 UTC 2014


On Wed, 2014-10-01 at 11:14 +0200, Vratislav Podzimek wrote:
> 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.
Yeah, why not. :)

> 
> > +    log.info("saving screenshots taken during the installation to:")
> > +    log.info(target_path)
> This should be a single log.info() call.
Done. :)
> 
> > +    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.
Handy! Replaced. :)
> 
> > +        # 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.
Also replaced.
> 
> > +
> > +        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.
Good point, changed. :)

> 
> > +        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
> 
Fixed!
> > +        """
> > +        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])
Thanks, looks better now. :)
> 
> > +        # 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
I've used enumerate & dumped spoke_name as the key is
just .__class__.__name__ anyway, so we don't really need it.

> .
> 
> > +            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.
> 
Thanks! Sending V3 just to be sure. :)



More information about the anaconda-patches mailing list