System: use paths for download folders too (#29818)

* use paths for download folders too

* trailing slash

* reset that

selfdrive/locationd/laikad.py

@@ -24,11 +24,11 @@ from laika.opt import calc_pos_fix, get_posfix_sympy_fun, calc_vel_fix, get_velf
 from openpilot.selfdrive.locationd.models.constants import GENERATED_DIR, ObservationKind
 from openpilot.selfdrive.locationd.models.gnss_kf import GNSSKalman
 from openpilot.selfdrive.locationd.models.gnss_kf import States as GStates
+from openpilot.system.hardware.hw import Paths
 from openpilot.system.swaglog import cloudlog
 
 MAX_TIME_GAP = 10
 EPHEMERIS_CACHE = 'LaikadEphemerisV3'
-DOWNLOADS_CACHE_FOLDER = "/tmp/comma_download_cache/"
 CACHE_VERSION = 0.2
 POS_FIX_RESIDUAL_THRESHOLD = 100.0
 
@@ -83,7 +83,7 @@ class Laikad:
     save_ephemeris: If true saves and loads nav and orbit ephemeris to cache.
     """
     self.astro_dog = AstroDog(valid_const=valid_const, auto_update=auto_update, valid_ephem_types=valid_ephem_types,
-                              clear_old_ephemeris=True, cache_dir=DOWNLOADS_CACHE_FOLDER)
+                              clear_old_ephemeris=True, cache_dir=Paths.download_cache_root())
     self.gnss_kf = GNSSKalman(GENERATED_DIR, cython=True, erratic_clock=use_qcom)
 
     self.auto_fetch_navs = auto_fetch_navs
@@ -435,9 +435,9 @@ def kf_add_observations(gnss_kf: GNSSKalman, t: float, measurements: List[GNSSMe
 
 
 def clear_tmp_cache():
-  if os.path.exists(DOWNLOADS_CACHE_FOLDER):
+  if os.path.exists(Paths.download_cache_root()):
-    shutil.rmtree(DOWNLOADS_CACHE_FOLDER)
+    shutil.rmtree(Paths.download_cache_root())
-  os.mkdir(DOWNLOADS_CACHE_FOLDER)
+  os.mkdir(Paths.download_cache_root())
 
 
 def main(sm=None, pm=None):

selfdrive/test/helpers.py

@@ -1,9 +1,6 @@
 import os
 import time
-import tempfile
-from typing import List, Union
 
-from unittest import mock
 from functools import wraps
 
 import cereal.messaging as messaging
@@ -72,43 +69,3 @@ def with_processes(processes, init_time=0, ignore_stopped=None):
 
     return wrap
   return wrapper
-
-
-def temporary_mock_dir(mock_paths_in: Union[List[str], str], kwarg: Union[str, None] = None, generator = tempfile.TemporaryDirectory):
-  """
-  mock_paths_in: string or string list representing the full path of the variable you want to mock.
-  kwarg: str or None representing the kwarg that gets passed into the test function, in case the test needs access to the temporary directory.
-  generator: a context to use to generate the temporary directory
-  """
-  def wrapper(func):
-    @wraps(func)
-    def wrap(*args, **kwargs):
-      mock_paths = mock_paths_in if isinstance(mock_paths_in, list) else [mock_paths_in]
-      with generator() as temp_dir:
-        mocks = []
-        for mock_path in mock_paths:
-          mocks.append(mock.patch(mock_path, str(temp_dir)))
-        [mock.start() for mock in mocks]
-        try:
-          if kwarg is not None:
-            kwargs[kwarg] = temp_dir
-          func(*args, **kwargs)
-        finally:
-          [mock.stop() for mock in mocks]
-
-    return wrap
-  return wrapper
-
-def string_context(context):
-  class StringContext:
-    def __enter__(self):
-      return context
-
-    def __exit__(self, *args):
-      pass
-
-  return StringContext
-
-temporary_dir = temporary_mock_dir([], "temp_dir")
-temporary_cache_dir = temporary_mock_dir("openpilot.tools.lib.url_file.CACHE_DIR")
-temporary_laikad_downloads_dir = temporary_mock_dir("openpilot.selfdrive.locationd.laikad.DOWNLOADS_CACHE_FOLDER")

system/hardware/hw.h

@@ -40,4 +40,11 @@ namespace Path {
   inline std::string swaglog_ipc() {
     return "ipc:///tmp/logmessage" + Path::openpilot_prefix();
   }
+
+  inline std::string download_cache_root() {
+    if (const char *env = getenv("COMMA_CACHE")) {
+      return env;
+    }
+    return "/tmp/comma_download_cache" + Path::openpilot_prefix() + "/";
+  }
 }  // namespace Path

system/hardware/hw.py

@@ -26,4 +26,10 @@ class Paths:
 
   @staticmethod
   def swaglog_ipc() -> str:
-    return "ipc:///tmp/logmessage" + os.environ.get("OPENPILOT_PREFIX", "")
+    return "ipc:///tmp/logmessage" + os.environ.get("OPENPILOT_PREFIX", "")
+
+  @staticmethod
+  def download_cache_root() -> str:
+    if os.environ.get('COMMA_CACHE', False):
+      return os.environ['COMMA_CACHE']
+    return "/tmp/comma_download_cache" + os.environ.get("OPENPILOT_PREFIX", "") + "/"

system/loggerd/bootlog.cc

@@ -50,11 +50,11 @@ static kj::Array<capnp::word> build_boot_log() {
 
 int main(int argc, char** argv) {
   const std::string timestr = logger_get_route_name();
-  const std::string path = LOG_ROOT + "/boot/" + timestr;
+  const std::string path = Path::log_root() + "/boot/" + timestr;
   LOGW("bootlog to %s", path.c_str());
 
   // Open bootlog
-  bool r = util::create_directories(LOG_ROOT + "/boot/", 0775);
+  bool r = util::create_directories(Path::log_root() + "/boot/", 0775);
   assert(r);
 
   RawFile file(path.c_str());

system/loggerd/logger.h

@@ -16,8 +16,6 @@
 #include "common/swaglog.h"
 #include "system/hardware/hw.h"
 
-const std::string LOG_ROOT = Path::log_root();
-
 #define LOGGER_MAX_HANDLES 16
 
 class RawFile {

system/loggerd/loggerd.cc

@@ -24,7 +24,7 @@ struct LoggerdState {
 
 void logger_rotate(LoggerdState *s) {
   int segment = -1;
-  int err = logger_next(&s->logger, LOG_ROOT.c_str(), s->segment_path, sizeof(s->segment_path), &segment);
+  int err = logger_next(&s->logger, Path::log_root().c_str(), s->segment_path, sizeof(s->segment_path), &segment);
   assert(err == 0);
   s->rotate_segment = segment;
   s->ready_to_rotate = 0;

system/loggerd/tests/test_loggerd.py

@@ -32,6 +32,9 @@ CEREAL_SERVICES = [f for f in log.Event.schema.union_fields if f in service_list
 
 
 class TestLoggerd(unittest.TestCase):
+  def setUp(self):
+    os.environ.pop("LOG_ROOT", None)
+
   def _get_latest_log_dir(self):
     log_dirs = sorted(Path(Paths.log_root()).iterdir(), key=lambda f: f.stat().st_mtime)
     return log_dirs[-1]

system/ubloxd/tests/test_ublox_processing.py

@@ -8,9 +8,9 @@ from laika.helpers import ConstellationId
 from laika.raw_gnss import correct_measurements, process_measurements, read_raw_ublox
 from laika.opt import calc_pos_fix
 from openpilot.selfdrive.test.openpilotci import get_url
+from openpilot.system.hardware.hw import Paths
 from openpilot.tools.lib.logreader import LogReader
 from openpilot.selfdrive.test.helpers import with_processes
-from openpilot.selfdrive.test.helpers import temporary_dir
 import cereal.messaging as messaging
 
 def get_gnss_measurements(log_reader):
@@ -56,9 +56,8 @@ class TestUbloxProcessing(unittest.TestCase):
     self.assertEqual(count_gps, 5036)
     self.assertEqual(count_glonass, 3651)
 
-  @temporary_dir
+  def test_get_fix(self):
-  def test_get_fix(self, temp_dir):
+    dog = AstroDog(cache_dir=Paths.download_cache_root())
-    dog = AstroDog(cache_dir=temp_dir)
     position_fix_found = 0
     count_processed_measurements = 0
     count_corrected_measurements = 0

tools/gpstest/rpc_server.py

@@ -3,6 +3,7 @@ import time
 import shutil
 from datetime import datetime
 from collections import defaultdict
+from openpilot.system.hardware.hw import Paths
 
 import rpyc
 from rpyc.utils.server import ThreadedServer
@@ -18,7 +19,6 @@ MATCH_NUM = 10
 REPORT_STATS = 10
 
 EPHEM_CACHE = "/data/params/d/LaikadEphemerisV3"
-DOWNLOAD_CACHE = "/tmp/comma_download_cache"
 
 SERVER_LOG_FILE = "/tmp/fuzzy_server.log"
 server_log = open(SERVER_LOG_FILE, "w+")
@@ -162,7 +162,7 @@ class RemoteCheckerService(rpyc.Service):
 
         if os.path.exists(EPHEM_CACHE):
           os.remove(EPHEM_CACHE)
-        shutil.rmtree(DOWNLOAD_CACHE, ignore_errors=True)
+        shutil.rmtree(Paths.download_cache_root(), ignore_errors=True)
 
       ret = self.run_checker(slat, slon, salt, sockets, procs, timeout)
       kill_procs(procs)

tools/lib/tests/test_caching.py

@@ -2,7 +2,6 @@
 import os
 import unittest
 from openpilot.tools.lib.url_file import URLFile
-from openpilot.selfdrive.test.helpers import temporary_cache_dir
 
 
 class TestFileDownload(unittest.TestCase):
@@ -31,7 +30,6 @@ class TestFileDownload(unittest.TestCase):
     self.assertEqual(file_cached.get_length(), file_downloaded.get_length())
     self.assertEqual(response_cached, response_downloaded)
 
-  @temporary_cache_dir
   def test_small_file(self):
     # Make sure we don't force cache
     os.environ["FILEREADER_CACHE"] = "0"
@@ -52,7 +50,6 @@ class TestFileDownload(unittest.TestCase):
     for i in range(length // 100):
       self.compare_loads(small_file_url, 100 * i, 100)
 
-  @temporary_cache_dir
   def test_large_file(self):
     large_file_url = "https://commadataci.blob.core.windows.net/openpilotci/0375fdf7b1ce594d/2019-06-13--08-32-25/3/qlog.bz2"
     #  Load the end 100 bytes of both files

tools/lib/url_file.py

@@ -8,12 +8,11 @@ from hashlib import sha256
 from io import BytesIO
 from tenacity import retry, wait_random_exponential, stop_after_attempt
 from openpilot.common.file_helpers import mkdirs_exists_ok, atomic_write_in_dir
+from openpilot.system.hardware.hw import Paths
 #  Cache chunk size
 K = 1000
 CHUNK_SIZE = 1000 * K
 
-CACHE_DIR = os.environ.get("COMMA_CACHE", "/tmp/comma_download_cache/")
-
 
 def hash_256(link):
   hsh = str(sha256((link.split("?")[0]).encode('utf-8')).hexdigest())
@@ -38,7 +37,7 @@ class URLFile:
       self._curl = self._tlocal.curl
     except AttributeError:
       self._curl = self._tlocal.curl = pycurl.Curl()
-    mkdirs_exists_ok(CACHE_DIR)
+    mkdirs_exists_ok(Paths.download_cache_root())
 
   def __enter__(self):
     return self
@@ -66,7 +65,7 @@ class URLFile:
   def get_length(self):
     if self._length is not None:
       return self._length
-    file_length_path = os.path.join(CACHE_DIR, hash_256(self._url) + "_length")
+    file_length_path = os.path.join(Paths.download_cache_root(), hash_256(self._url) + "_length")
     if os.path.exists(file_length_path) and not self._force_download:
       with open(file_length_path) as file_length:
           content = file_length.read()
@@ -93,7 +92,7 @@ class URLFile:
       self._pos = position
       chunk_number = self._pos / CHUNK_SIZE
       file_name = hash_256(self._url) + "_" + str(chunk_number)
-      full_path = os.path.join(CACHE_DIR, str(file_name))
+      full_path = os.path.join(Paths.download_cache_root(), str(file_name))
       data = None
       #  If we don't have a file, download it
       if not os.path.exists(full_path):

tools/replay/filereader.cc

@@ -3,11 +3,12 @@
 #include <fstream>
 
 #include "common/util.h"
+#include "system/hardware/hw.h"
 #include "tools/replay/util.h"
 
 std::string cacheFilePath(const std::string &url) {
   static std::string cache_path = [] {
-    const std::string comma_cache = util::getenv("COMMA_CACHE", "/tmp/comma_download_cache/");
+    const std::string comma_cache = Path::download_cache_root();
     util::create_directories(comma_cache, 0755);
     return comma_cache.back() == '/' ? comma_cache : comma_cache + "/";
   }();