[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