----- Original Message -----
From: "Martin Kolman" <mkolman(a)redhat.com>
To: anaconda-patches(a)lists.fedorahosted.org
Sent: Thursday, September 25, 2014 8:49:36 AM
Subject: [PATCH] Add support for autostep and --autoscreenshot (#1059295)
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(a)redhat.com>
---
data/post-scripts/90-copy-screenshots.ks | 10 ---
pyanaconda/constants.py | 4 +
pyanaconda/iutil.py | 23 ++++++
pyanaconda/ui/gui/__init__.py | 127
++++++++++++++++++++++++++-----
pyanaconda/ui/gui/hubs/__init__.py | 43 +++++++++++
5 files changed, 179 insertions(+), 28 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"
If this path is ever going to be relative to anything,
should make it relative, too.
+SCREENSHOTS_TARGET_DIRECTORY =
"/root/anaconda-screenshots"
Should make above relative, not absolute, path.
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)
Should use os.path.join() here.
+ log.info("saving screenshots taken during the installation
to:")
+ log.info(target_path)
Why not combine above lines into one line?
+ 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)
Why not just remove the directory and make it w/out the check?
Are you expecting to be adding screenshots incrementally?
Method header should probably indicate whether this method appends or replaces.
+ # copy all screenshots
+ for filename in os.listdir(SCREENSHOTS_DIRECTORY):
+ shutil.copy(os.path.join(SCREENSHOTS_DIRECTORY, filename),
target_path)
To be really punctilious, you should set things up so that you can continue
if copy of one file fails.
+
+ 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..afe3901 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,99 @@ 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)
+
+ 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__)
+ self._doAutostep()
+ # done
+ self.autostepRunning = False
+ self.autostepDone = True
+
Is there any way to handle this autostepRunning on/off setting using a context manager?
+ 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)
Could you specify what the method does in the docstring? Do the methods that
override it have to observe any constraints? Can they rely on any guarantees?
I'm puzzled, because many things override GUIobject, and now they all have
autostep() method but it doesn't seem appropriate for all.
Should some of this be lower in class hierarchy?
+ """
+ pass
+
@property
def window(self):
"""Return the top-level object out of the GtkBuilder
representation
@@ -447,8 +520,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,
It actually seems to be the _doAutostep() function in HUB, but maybe autostep()
is the correct place?
+ # 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)
I'm puzzled...isn't this the last possible place you could save the screenshots?
Why do kickstart post scripts matter?
+ 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..c0e7fde 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,37 @@ 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])
+ # take a screenshot of all spokes in the alphabetical order of their
names
+ for spoke_name, spoke in spokes_kv:
+ 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__)
+ # 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")
+
+ 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.
"got to" -> "go to" ?
+ """
+ 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()
+
### SIGNAL HANDLERS
def register_event_cb(self, event, cb):
--
1.9.3
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
- mulhern