Merge pull request #5255 from M4rtinK/master-drop_screenshot_support

Remove screenshot support

anaconda.spec.in

@@ -292,7 +292,6 @@ Requires: libxklavier >= %{libxklavierver}
 Requires: libgnomekbd
 Requires: libtimezonemap >= %{libtimezonemapver}
 Requires: nm-connection-editor
-Requires: keybinder3
 %ifnarch s390 s390x
 Requires: NetworkManager-wifi
 %endif

docs/release-notes/remove-screenshot-support.rst

@@ -0,0 +1,20 @@
+:Type: GUI
+:Summary: Remove screenshot support
+
+:Description:
+    It was previously possible to take a screenshot of the
+    Anaconda GUI by pressing a global hotkey. This was
+    never widely advertised & rather hard to use for anything
+    useful, as it was also necessary to manually extract the
+    resulting screenshots from the installation environment.
+
+    Furthermore, with many installations happening in VMs,
+    it is usually more convenient to take a screenshot using
+    the VM software anyway.
+
+    By dropping screenshot support, we can remove dependency
+    on the ``keybinder3`` library as well as the necessary
+    screenshot code in the GUI.
+
+:Links:
+    - https://github.com/rhinstaller/anaconda/pull/5255

pyanaconda/core/constants.py

@@ -239,9 +239,6 @@ class SecretStatus(Enum):
 # all ASCII characters
 PW_ASCII_CHARS = string.digits + string.ascii_letters + string.punctuation + " "
 
-# screenshots
-SCREENSHOTS_DIRECTORY = "/tmp/anaconda-screenshots"
-
 PACKAGES_LIST_FILE = "/root/lorax-packages.log"
 
 CMDLINE_FILES = [

pyanaconda/modules/boss/installation.py

@@ -21,15 +21,13 @@
 
 from pyanaconda.anaconda_loggers import get_module_logger
 from pyanaconda.core.configuration.anaconda import conf
-from pyanaconda.core.constants import SCREENSHOTS_DIRECTORY
 from pyanaconda.core.path import make_directories, join_paths
 from pyanaconda.core.util import execWithRedirect, restorecon
 from pyanaconda.modules.common.task import Task
 
 log = get_module_logger(__name__)
 
 TARGET_LOG_DIR = "/var/log/anaconda/"
-TARGET_SCREENSHOT_DIR = "/root/anaconda-screenshots/"
 
 
 class CopyLogsTask(Task):
@@ -50,31 +48,13 @@ def name(self):
     def run(self):
         """Copy installation logs and other related files, and do necessary operations on them.
 
-        - Copy screenshots
         - Copy logs of all kinds, incl. ks scripts, journal dump, and lorax pkg list
         - Copy input kickstart file
         - Autorelabel everything in the destination
         """
-        self._copy_screenshots()
         self._copy_kickstart()
         self._copy_logs()
 
-    def _copy_screenshots(self):
-        """Copy screenshots from the installation to the target system."""
-        log.info("Copying screenshots from installation.")
-
-        screenshots = glob.glob(SCREENSHOTS_DIRECTORY + "/*.png")
-        if not screenshots:
-            return
-
-        os.makedirs(join_paths(self._sysroot, TARGET_SCREENSHOT_DIR), 0o750, True)
-
-        for screenshot in screenshots:
-            self._copy_file_to_sysroot(
-                screenshot,
-                join_paths(TARGET_SCREENSHOT_DIR, os.path.basename(screenshot))
-            )
-
     def _copy_logs(self):
         """Copy installation logs to the target system"""
         if not conf.target.can_save_installation_logs:

pyanaconda/ui/gui/__init__.py

@@ -28,18 +28,16 @@
 gi.require_version("Gdk", "3.0")
 gi.require_version("Gtk", "3.0")
 gi.require_version("AnacondaWidgets", "3.4")
-gi.require_version("Keybinder", "3.0")
 gi.require_version("GdkPixbuf", "2.0")
 gi.require_version("GObject", "2.0")
 
-from gi.repository import Gdk, Gtk, AnacondaWidgets, Keybinder, GdkPixbuf, GObject
+from gi.repository import Gdk, Gtk, AnacondaWidgets, GdkPixbuf, GObject
 
 from pyanaconda.flags import flags
 from pyanaconda.core.i18n import _, C_
 from pyanaconda.core.constants import WINDOW_TITLE_TEXT
 from pyanaconda.core.configuration.anaconda import conf
 from pyanaconda.core import util, constants
-from pyanaconda.core.path import make_directories
 from pyanaconda.core.product import get_product_is_final_release
 from pyanaconda.core.threads import thread_manager
 
@@ -362,12 +360,6 @@ def __init__(self, fullscreen=False, decorated=False):
         self._language = None
         self.reapply_language()
 
-        # Keybinder from GI needs to be initialized before use
-        Keybinder.init()
-        Keybinder.bind("<Shift>Print", self._handle_print_screen, [])
-
-        self._screenshot_index = 0
-
     def _on_delete_event(self, widget, event, user_data=None):
         # Use the quit-clicked signal on the the current standalone, even if the
         # standalone is not currently displayed.
@@ -539,34 +531,6 @@ def reapply_language(self):
         self._language = os.environ["LANG"]
         watch_children(self, self._on_child_added, self._language)
 
-    def _handle_print_screen(self, *args, **kwargs):
-        self.take_screenshot()
-
-    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
-        """
-        # Make sure the screenshot directory exists.
-        make_directories(constants.SCREENSHOTS_DIRECTORY)
-
-        if name is None:
-            screenshot_filename = "screenshot-%04d.png" % self._screenshot_index
-        else:
-            screenshot_filename = "screenshot-%04d-%s.png" % (self._screenshot_index, name)
-
-        fn = os.path.join(constants.SCREENSHOTS_DIRECTORY, screenshot_filename)
-
-        root_window = self.current_window.get_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)
-        self._screenshot_index += 1
-
 
 class GraphicalUserInterface(UserInterface):
     """This is the standard GTK+ interface we try to steer everything to using.

tests/unit_tests/pyanaconda_tests/modules/boss/test_copy_logs_task.py

@@ -31,7 +31,6 @@ def test_run_all(self, open_mock, conf_mock, mkdir_mock, restore_mock, exec_wr_m
                      glob_mock):
         """Test the log copying task."""
         glob_mock.side_effect = [
-            ["/tmp/anaconda-screenshots/screenshot-0001.png"],
             ["/tmp/ks-script-blabblah.log"],
             ["/somewhere/var/log/anaconda/anaconda.log"]
         ]
@@ -54,8 +53,6 @@ def test_run_all(self, open_mock, conf_mock, mkdir_mock, restore_mock, exec_wr_m
             )
 
         copy_file_mock.assert_has_calls([
-            call("/tmp/anaconda-screenshots/screenshot-0001.png",
-                 "/root/anaconda-screenshots/screenshot-0001.png"),
             call("/root/lorax-packages.log", "/var/log/anaconda/lorax-packages.log"),
             call("/tmp/ks-script-blabblah.log", "/var/log/anaconda/ks-script-blabblah.log"),
             call("/tmp/journal.log", "/var/log/anaconda/journal.log")
@@ -67,7 +64,6 @@ def test_run_all(self, open_mock, conf_mock, mkdir_mock, restore_mock, exec_wr_m
         ])
 
         glob_mock.assert_has_calls([
-            call("/tmp/anaconda-screenshots/*.png"),
             call("/tmp/ks-script*.log")
         ])
         open_mock.assert_called_once_with("/tmp/journal.log", "w")
@@ -96,7 +92,6 @@ def test_run_all(self, open_mock, conf_mock, mkdir_mock, restore_mock, exec_wr_m
     def test_nosave_logs(self, open_mock, conf_mock, mkdir_mock, exec_wr_mock, glob_mock):
         """Test nosave for logs"""
         glob_mock.side_effect = [
-            [],  # no screenshots
             []   # no script logs
         ]
         conf_mock.target.can_save_installation_logs = False
@@ -112,7 +107,6 @@ def test_nosave_logs(self, open_mock, conf_mock, mkdir_mock, exec_wr_mock, glob_
             "/root/original-ks.cfg"
         )
 
-        glob_mock.assert_called_once_with("/tmp/anaconda-screenshots/*.png")
         exec_wr_mock.assert_not_called()
         mkdir_mock.assert_not_called()
         copy_tree_mock.assert_not_called()
@@ -126,7 +120,6 @@ def test_nosave_logs(self, open_mock, conf_mock, mkdir_mock, exec_wr_mock, glob_
     def test_nosave_input_ks(self, open_mock, conf_mock, mkdir_mock, exec_wr_mock, glob_mock):
         """Test nosave for kickstart"""
         glob_mock.side_effect = [
-            [],  # no screenshots
             ["/somewhere/var/log/anaconda/anaconda.log"]
         ]
         conf_mock.target.can_save_installation_logs = True
@@ -156,7 +149,6 @@ def test_nosave_logs_and_input_ks(self, open_mock, conf_mock, mkdir_mock, exec_w
                                       glob_mock):
         """Test nosave for both logs and kickstart"""
         glob_mock.side_effect = [
-            [],  # no screenshots
             []   # no script logs
         ]
         conf_mock.target.can_save_installation_logs = False
@@ -167,8 +159,6 @@ def test_nosave_logs_and_input_ks(self, open_mock, conf_mock, mkdir_mock, exec_w
             with patch.object(CopyLogsTask, "_copy_tree_to_sysroot") as copy_tree_mock:
                 task.run()
 
-        glob_mock.assert_called_once_with("/tmp/anaconda-screenshots/*.png")
-
         exec_wr_mock.assert_not_called()
         mkdir_mock.assert_not_called()
         copy_file_mock.assert_not_called()