From 50db87664d78163a7f28f158318281e935691867 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Wed, 22 Nov 2023 15:09:31 +0100
Subject: pakjoy: Remove existing work dir if unneeded

---
 qutebrowser/misc/pakjoy.py     | 24 +++++++++++++++---------
 tests/unit/misc/test_pakjoy.py | 11 +++++++++--
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py
index 016fc2b8e..6c681eb65 100644
--- a/qutebrowser/misc/pakjoy.py
+++ b/qutebrowser/misc/pakjoy.py
@@ -143,7 +143,7 @@ class PakParser:
         raise binparsing.ParseError("Couldn't find hangouts manifest")
 
 
-def copy_webengine_resources() -> pathlib.Path:
+def copy_webengine_resources() -> Optional[pathlib.Path]:
     """Copy qtwebengine resources to local dir for patching."""
     resources_dir = qtutils.library_path(qtutils.LibraryPath.data)
     if utils.is_mac:
@@ -156,15 +156,20 @@ def copy_webengine_resources() -> pathlib.Path:
         resources_dir /= "resources"
     work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk"
 
+    if work_dir.exists():
+        log.misc.debug(f"Removing existing {work_dir}")
+        shutil.rmtree(work_dir)
+
+    versions = version.qtwebengine_versions(avoid_init=True)
+    if versions.webengine != utils.VersionNumber(6, 6):
+        # No patching needed
+        return None
+
     log.misc.debug(
         "Copying webengine resources for quirk patching: "
         f"{resources_dir} -> {work_dir}"
     )
 
-    if work_dir.exists():
-        # TODO: make backup?
-        shutil.rmtree(work_dir)
-
     shutil.copytree(resources_dir, work_dir)
 
     os.environ["QTWEBENGINE_RESOURCES_PATH"] = str(work_dir)
@@ -194,14 +199,15 @@ def _patch(file_to_patch: pathlib.Path) -> None:
 
 def patch_webengine() -> None:
     """Apply any patches to webengine resource pak files."""
-    versions = version.qtwebengine_versions(avoid_init=True)
-    if versions.webengine != utils.VersionNumber(6, 6):
-        return
-
     try:
+        # Still calling this on Qt != 6.6 so that the directory is cleaned up
+        # when not needed anymore.
         webengine_resources_path = copy_webengine_resources()
     except OSError:
         log.misc.exception("Failed to copy webengine resources, not applying quirk")
         return
 
+    if webengine_resources_path is None:
+        return
+
     _patch(webengine_resources_path / "qtwebengine_resources.pak")
diff --git a/tests/unit/misc/test_pakjoy.py b/tests/unit/misc/test_pakjoy.py
index 26c386759..d4bb37d41 100644
--- a/tests/unit/misc/test_pakjoy.py
+++ b/tests/unit/misc/test_pakjoy.py
@@ -62,11 +62,18 @@ def affected_version(monkeypatch):
     patch_version(monkeypatch, 6, 6)
 
 
-def test_version_gate(unaffected_version, mocker):
-
+@pytest.mark.parametrize("workdir_exists", [True, False])
+def test_version_gate(cache_tmpdir, unaffected_version, mocker, workdir_exists):
+    workdir = cache_tmpdir / "webengine_resources_pak_quirk"
+    if workdir_exists:
+        workdir.mkdir()
+        (workdir / "some_patched_file.pak").ensure()
     fake_open = mocker.patch("qutebrowser.misc.pakjoy.open")
+
     pakjoy.patch_webengine()
+
     assert not fake_open.called
+    assert not workdir.exists()
 
 
 @pytest.fixture(autouse=True)
-- 
cgit v1.2.3-54-g00ecf