Fix issues with the custom cache-dir feature

The None result from get_cache_path() was leaking into the Preferences dialog, so now it never returns None - there's always ~/.cache/lutris/installer to fall back on, after all. A separate function checks if it is even configured.

Also, the sanity check for whether the custom dir even exists was inconsistently applied and would accept a file; it needs to be a directory. So now it checks with os.path.isdir().

lutris/cache.py

@@ -8,14 +8,31 @@ from lutris.util.system import merge_folders
 
 
 def get_cache_path():
-    """Return the path of the PGA cache"""
-    pga_cache_path = settings.read_setting("pga_cache_path")
-    if pga_cache_path:
-        return os.path.expanduser(pga_cache_path)
-    return None
+    """Returns the directory under which Lutris caches install files. This can be specified
+    by the user, but defaults to a location in ~/.cache."""
+    cache_path = settings.read_setting("pga_cache_path")
+    if cache_path:
+        cache_path = os.path.expanduser(cache_path)
+        if os.path.isdir(cache_path):
+            return cache_path
+
+    return settings.INSTALLER_CACHE_DIR
 
 
-def save_cache_path(path):
+def has_custom_cache_path() -> bool:
+    """True if the user has selected a custom cache location, in which case we
+    keep the files there, rather than removing them during installation."""
+    cache_path = settings.read_setting("pga_cache_path")
+    if not cache_path:
+        return False
+    if not os.path.isdir(cache_path):
+        logger.warning("Cache path %s does not exist", cache_path)
+        return False
+
+    return True
+
+
+def save_custom_cache_path(path):
     """Saves the PGA cache path to the settings"""
     settings.write_setting("pga_cache_path", path)
 

lutris/gui/config/storage_box.py

@@ -3,7 +3,7 @@ from gettext import gettext as _
 
 from gi.repository import Gtk
 
-from lutris.cache import get_cache_path, save_cache_path
+from lutris.cache import get_cache_path, has_custom_cache_path, save_custom_cache_path
 from lutris.config import LutrisConfig
 from lutris.gui.config.base_config_box import BaseConfigBox
 from lutris.gui.widgets.common import FileChooserEntry, Label
@@ -32,7 +32,7 @@ class StorageBox(BaseConfigBox):
                 "name": "Installer cache",
                 "setting": "pga_cache_path",
                 "default": "",
-                "value": get_cache_path(),
+                "value": get_cache_path() if has_custom_cache_path() else "",
                 "help": _(
                     "If provided, files downloaded during game installs will be kept there\n"
                     "\nOtherwise, all downloaded files are discarded."
@@ -76,7 +76,7 @@ class StorageBox(BaseConfigBox):
     def on_file_chooser_changed(self, entry, setting):
         text = entry.get_text()
         if setting["setting"] == "pga_cache_path":
-            save_cache_path(text)
+            save_custom_cache_path(text)
         elif setting["setting"] == "game_path":
             lutris_config = LutrisConfig()
             lutris_config.raw_system_config["game_path"] = text

lutris/gui/dialogs/cache.py

@@ -2,7 +2,7 @@ from gettext import gettext as _
 
 from gi.repository import Gtk
 
-from lutris.cache import get_cache_path, save_cache_path
+from lutris.cache import get_cache_path, has_custom_cache_path, save_custom_cache_path
 from lutris.gui.dialogs import ModalDialog
 from lutris.gui.widgets.common import FileChooserEntry
 
@@ -18,7 +18,7 @@ class CacheConfigurationDialog(ModalDialog):
         self.timer_id = None
         self.set_size_request(480, 150)
 
-        self.cache_path = get_cache_path() or ""
+        self.cache_path = get_cache_path() if has_custom_cache_path() else ""
         self.get_content_area().add(self.get_cache_config())
 
         self.add_button(Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL)
@@ -28,7 +28,7 @@ class CacheConfigurationDialog(ModalDialog):
         result = self.run()
 
         if result == Gtk.ResponseType.OK:
-            save_cache_path(self.cache_path)
+            save_custom_cache_path(self.cache_path)
 
         self.destroy()
 

lutris/installer/commands.py

@@ -11,7 +11,7 @@ from pathlib import Path
 from gi.repository import GLib
 
 from lutris import runtime
-from lutris.cache import get_cache_path
+from lutris.cache import get_cache_path, has_custom_cache_path
 from lutris.command import MonitoredCommand
 from lutris.exceptions import MissingExecutableError, UnspecifiedVersionError
 from lutris.installer.errors import ScriptingError
@@ -85,9 +85,9 @@ class CommandsMixin:
     @staticmethod
     def _is_cached_file(file_path):
         """Return whether a file referenced by file_id is stored in the cache"""
-        pga_cache_path = get_cache_path()
-        if not pga_cache_path:
+        if not has_custom_cache_path():
             return False
+        pga_cache_path = get_cache_path()
         return file_path.startswith(pga_cache_path)
 
     def chmodx(self, filename):

lutris/installer/installer_file.py

@@ -3,8 +3,8 @@ import os
 from gettext import gettext as _
 from urllib.parse import urlparse
 
-from lutris import cache, settings
-from lutris.cache import save_to_cache
+from lutris import cache
+from lutris.cache import has_custom_cache_path, save_to_cache
 from lutris.gui.widgets.download_progress_box import DownloadProgressBox
 from lutris.installer.errors import ScriptingError
 from lutris.util import system
@@ -175,14 +175,7 @@ class InstallerFile:
         """
         if self.url.startswith("N/A"):
             return False
-        cache_path = cache.get_cache_path()
-        if not cache_path:
-            return False
-        if system.path_exists(cache_path):
-            return True
-
-        logger.warning("Cache path %s does not exist", cache_path)
-        return False
+        return has_custom_cache_path()
 
     @property
     def is_user_pga_caching_allowed(self):
@@ -193,7 +186,7 @@ class InstallerFile:
     @property
     def cache_path(self):
         """Return the directory used as a cache for the duration of the installation"""
-        _cache_path = cache.get_cache_path() or settings.INSTALLER_CACHE_DIR
+        _cache_path = cache.get_cache_path()
         url_parts = urlparse(self.url)
         if url_parts.netloc.endswith("gog.com"):
             folder = "gog"
@@ -269,8 +262,8 @@ class InstallerFile:
     def remove_previous(self):
         """Remove file at already at destination, prior to starting the download."""
         if (
-                not self.uses_pga_cache()
-                and system.path_exists(self.dest_file)
+            not self.uses_pga_cache()
+            and system.path_exists(self.dest_file)
         ):
             # If we've previously downloaded a directory, we'll need to get rid of it
             # to download a file now. Since we are not using the cache, we don't keep

lutris/installer/installer_file_collection.py

@@ -3,10 +3,10 @@ import os
 from gettext import gettext as _
 from urllib.parse import urlparse
 
-from lutris import cache, settings
+from lutris import cache
+from lutris.cache import has_custom_cache_path
 from lutris.gui.widgets.download_collection_progress_box import DownloadCollectionProgressBox
 from lutris.util import system
-from lutris.util.log import logger
 from lutris.util.strings import gtk_safe_urls
 
 AMAZON_DOMAIN = "a2z.com"
@@ -113,14 +113,7 @@ class InstallerFileCollection:
         Returns:
             bool
         """
-        cache_path = cache.get_cache_path()
-        if not cache_path:
-            return False
-        if system.path_exists(cache_path):
-            return True
-
-        logger.warning("Cache path %s does not exist", cache_path)
-        return False
+        return has_custom_cache_path()
 
     @property
     def is_user_pga_caching_allowed(self):
@@ -129,7 +122,7 @@ class InstallerFileCollection:
     @property
     def cache_path(self):
         """Return the directory used as a cache for the duration of the installation"""
-        _cache_path = cache.get_cache_path() or settings.INSTALLER_CACHE_DIR
+        _cache_path = cache.get_cache_path()
         return os.path.join(_cache_path, self.game_slug)
 
     def prepare(self):