Skip to content

Commit

Permalink
simplify git remote is comma check (commaai#31284)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
gregjhogan authored Feb 3, 2024
1 parent 52be380 commit 7affba0
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 40 deletions.
6 changes: 3 additions & 3 deletions selfdrive/athena/athenad.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}


Expand Down
2 changes: 1 addition & 1 deletion selfdrive/controls/controlsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def __init__(self, CI=None):
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',
Expand Down
2 changes: 1 addition & 1 deletion selfdrive/controls/lib/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def func(CP: car.CarParams, CS: car.CarState, sm: messaging.SubMaster, metric: b
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"

Expand Down
6 changes: 3 additions & 3 deletions selfdrive/manager/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
2 changes: 1 addition & 1 deletion selfdrive/sentry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion selfdrive/test/process_replay/test_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def test_process(cfg, lr, segment, ref_log_path, new_log_path, ignore_fields=Non
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} *****")
Expand Down
2 changes: 1 addition & 1 deletion selfdrive/tombstoned.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 20 additions & 29 deletions system/version.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -13,8 +13,8 @@
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)


Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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
Expand Down

0 comments on commit 7affba0

Please sign in to comment.