simplify git remote is comma check (#31284)

* simplify git remote is comma check

* cast to str

* eliminate default and always return string

* add type annotation for cache decorator

* fix up default handling
old-commit-hash: 7affba06d8

selfdrive/athena/athenad.py

@@ -316,9 +316,9 @@ def getMessage(service: str, timeout: int = 1000) -> dict:
 def getVersion() -> Dict[str, str]:
   return {
     "version": get_version(),
-    "remote": get_normalized_origin(''),
-    "branch": get_short_branch(''),
-    "commit": get_commit(default=''),
+    "remote": get_normalized_origin(),
+    "branch": get_short_branch(),
+    "commit": get_commit(),
   }
 
 

selfdrive/controls/controlsd.py

@@ -60,7 +60,7 @@ class Controls:
     config_realtime_process(4, Priority.CTRL_HIGH)
 
     # Ensure the current branch is cached, otherwise the first iteration of controlsd lags
-    self.branch = get_short_branch("")
+    self.branch = get_short_branch()
 
     # Setup sockets
     self.pm = messaging.PubMaster(['sendcan', 'controlsState', 'carState',

selfdrive/controls/lib/events.py

@@ -224,7 +224,7 @@ def user_soft_disable_alert(alert_text_2: str) -> AlertCallbackType:
   return func
 
 def startup_master_alert(CP: car.CarParams, CS: car.CarState, sm: messaging.SubMaster, metric: bool, soft_disable_time: int) -> Alert:
-  branch = get_short_branch("")  # Ensure get_short_branch is cached to avoid lags on startup
+  branch = get_short_branch()  # Ensure get_short_branch is cached to avoid lags on startup
   if "REPLAY" in os.environ:
     branch = "replay"
 

selfdrive/manager/manager.py

@@ -65,9 +65,9 @@ def manager_init() -> None:
   params.put("Version", get_version())
   params.put("TermsVersion", terms_version)
   params.put("TrainingVersion", training_version)
-  params.put("GitCommit", get_commit(default=""))
-  params.put("GitBranch", get_short_branch(default=""))
-  params.put("GitRemote", get_origin(default=""))
+  params.put("GitCommit", get_commit())
+  params.put("GitBranch", get_short_branch())
+  params.put("GitRemote", get_origin())
   params.put_bool("IsTestedBranch", is_tested_branch())
   params.put_bool("IsReleaseBranch", is_release_branch())
 

selfdrive/sentry.py

@@ -44,7 +44,7 @@ def set_tag(key: str, value: str) -> None:
 
 def init(project: SentryProject) -> bool:
   # forks like to mess with this, so double check
-  comma_remote = is_comma_remote() and "commaai" in get_origin(default="")
+  comma_remote = is_comma_remote() and "commaai" in get_origin()
   if not comma_remote or not is_registered_device() or PC:
     return False
 

selfdrive/test/process_replay/test_processes.py

@@ -160,7 +160,7 @@ if __name__ == "__main__":
     sys.exit(1)
 
   cur_commit = get_commit()
-  if cur_commit is None:
+  if not cur_commit:
     raise Exception("Couldn't get current commit")
 
   print(f"***** testing against commit {ref_commit} *****")

selfdrive/tombstoned.py

@@ -124,7 +124,7 @@ def report_tombstone_apport(fn):
   clean_path = path.replace('/', '_')
   date = datetime.datetime.now().strftime("%Y-%m-%d--%H-%M-%S")
 
-  new_fn = f"{date}_{get_commit(default='nocommit')[:8]}_{safe_fn(clean_path)}"[:MAX_TOMBSTONE_FN_LEN]
+  new_fn = f"{date}_{(get_commit() or 'nocommit')[:8]}_{safe_fn(clean_path)}"[:MAX_TOMBSTONE_FN_LEN]
 
   crashlog_dir = os.path.join(Paths.log_root(), "crash")
   os.makedirs(crashlog_dir, exist_ok=True)

system/version.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 import os
 import subprocess
-from typing import List, Optional
+from typing import List, Optional, Callable, TypeVar
 from functools import lru_cache
 
 from openpilot.common.basedir import BASEDIR
@@ -13,8 +13,8 @@ TESTED_BRANCHES = RELEASE_BRANCHES + ['devel', 'devel-staging']
 training_version: bytes = b"0.2.0"
 terms_version: bytes = b"2"
 
-
-def cache(user_function, /):
+_RT = TypeVar("_RT")
+def cache(user_function: Callable[..., _RT], /) -> Callable[..., _RT]:
   return lru_cache(maxsize=None)(user_function)
 
 
@@ -30,41 +30,37 @@ def run_cmd_default(cmd: List[str], default: Optional[str] = None) -> Optional[s
 
 
 @cache
-def get_commit(branch: str = "HEAD", default: Optional[str] = None) -> Optional[str]:
-  return run_cmd_default(["git", "rev-parse", branch], default=default)
+def get_commit(branch: str = "HEAD") -> str:
+  return run_cmd_default(["git", "rev-parse", branch]) or ""
 
 
 @cache
-def get_short_branch(default: Optional[str] = None) -> Optional[str]:
-  return run_cmd_default(["git", "rev-parse", "--abbrev-ref", "HEAD"], default=default)
+def get_short_branch() -> str:
+  return run_cmd_default(["git", "rev-parse", "--abbrev-ref", "HEAD"]) or ""
 
 
 @cache
-def get_branch(default: Optional[str] = None) -> Optional[str]:
-  return run_cmd_default(["git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"], default=default)
+def get_branch() -> str:
+  return run_cmd_default(["git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"]) or ""
 
 
 @cache
-def get_origin(default: Optional[str] = None) -> Optional[str]:
+def get_origin() -> str:
   try:
     local_branch = run_cmd(["git", "name-rev", "--name-only", "HEAD"])
     tracking_remote = run_cmd(["git", "config", "branch." + local_branch + ".remote"])
     return run_cmd(["git", "config", "remote." + tracking_remote + ".url"])
   except subprocess.CalledProcessError:  # Not on a branch, fallback
-    return run_cmd_default(["git", "config", "--get", "remote.origin.url"], default=default)
+    return run_cmd_default(["git", "config", "--get", "remote.origin.url"]) or ""
 
 
 @cache
-def get_normalized_origin(default: Optional[str] = None) -> Optional[str]:
-  origin: Optional[str] = get_origin()
-
-  if origin is None:
-    return default
-
-  return origin.replace("git@", "", 1) \
-               .replace(".git", "", 1) \
-               .replace("https://", "", 1) \
-               .replace(":", "/", 1)
+def get_normalized_origin() -> str:
+  return get_origin() \
+    .replace("git@", "", 1) \
+    .replace(".git", "", 1) \
+    .replace("https://", "", 1) \
+    .replace(":", "/", 1)
 
 
 @cache
@@ -75,7 +71,7 @@ def get_version() -> str:
 
 @cache
 def get_short_version() -> str:
-  return get_version().split('-')[0]  # type: ignore
+  return get_version().split('-')[0]
 
 @cache
 def is_prebuilt() -> bool:
@@ -86,12 +82,7 @@ def is_prebuilt() -> bool:
 def is_comma_remote() -> bool:
   # note to fork maintainers, this is used for release metrics. please do not
   # touch this to get rid of the orange startup alert. there's better ways to do that
-  origin: Optional[str] = get_origin()
-  if origin is None:
-    return False
-
-  return origin.startswith(('git@github.com:commaai', 'https://github.com/commaai'))
-
+  return get_normalized_origin() == "github.com/commaai/openpilot"
 
 @cache
 def is_tested_branch() -> bool:
@@ -105,7 +96,7 @@ def is_release_branch() -> bool:
 def is_dirty() -> bool:
   origin = get_origin()
   branch = get_branch()
-  if (origin is None) or (branch is None):
+  if not origin or not branch:
     return True
 
   dirty = False