From 237d611b89f67ff7c25c94684bcef25446ea03f9 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 29 Oct 2026 14:13:09 -0400 Subject: [PATCH] fix(update): stop a timed-out git from leaving .git/index.lock behind (#802) * fix(update): stop a timed-out git from leaving .git/index.lock behind subprocess.run's timeout SIGKILLs only the direct git child, so the lock is left behind and git's helpers are orphaned. On a slow SD card the same pull then times out on every retry and recreates the lock, so removing it by hand does not help. - run_git starts git in its own session, stops the group with SIGTERM before SIGKILL, and clears the lock if nothing owns it. Update Code's pull, checkout, reset and stash go through it, with a 600s pull timeout. - Read-only calls (describe --dirty, status, the update-check fetch) set GIT_OPTIONAL_LOCKS=0 so a timed-out poll cannot take the lock. - A timed-out pull returns a clear error; a rebase left by an interrupted update is aborted before pulling. - The manual-fix hint says to stop the services and kill running git. Co-Authored-By: Claude Sonnet 5.5 * fix(update): address review of the index.lock fix - The automatic update's status check now sets GIT_OPTIONAL_LOCKS=0 whichever runner it is given, not only the default one. - Update Code no longer runs `git rebase --abort` on any rebase dir: a rebase in progress (detached HEAD) holds the user's conflict work and blocks the update; an abandoned one is dropped with `rebase --quit`, as the automatic update does, and a failure to clear it stops the update. - run_git waits out the lock-freshness window after stopping a timed-out git, so a lock created just before the timeout is not left behind. - Mark the Popen/subprocess use with the repo's nosec comments. Co-Authored-By: Claude Sonnet 5.5 * chore(update): mark run_git's CompletedProcess as a result object, not a subprocess call Co-Authored-By: Claude Sonnet 5.5 --------- Co-authored-by: Claude Sonnet 5.5 --- test/test_auto_update.py | 1 + test/test_git_index_lock.py | 131 +++++++++++++++++++- test/test_update_prompts_restart.py | 22 ++-- web_interface/auto_update.py | 6 +- web_interface/blueprints/api_v3/__init__.py | 4 +- web_interface/blueprints/api_v3/system.py | 72 ++++++++--- web_interface/git_index_lock.py | 81 +++++++++++- web_interface/update_channel.py | 4 +- 8 files changed, 284 insertions(+), 37 deletions(-) diff --git a/test/test_auto_update.py b/test/test_auto_update.py index 840b00f6..2464d33e 100644 --- a/test/test_auto_update.py +++ b/test/test_auto_update.py @@ -823,6 +823,7 @@ def test_core_update_names_the_requirement_files_that_failed(): pkg.api_v3.plugin_store_manager = None failed_install = subprocess.CompletedProcess([], 1, stdout='', stderr='boom') with patch.object(pkg.subprocess, 'run', run), \ + patch.object(system.git_index_lock, 'run_git', run), \ patch.object(system, '_pip_install_requirements', return_value=failed_install): result = system.perform_core_update() assert result['status'] == 'success' diff --git a/test/test_git_index_lock.py b/test/test_git_index_lock.py index d49d337b..0233d169 100644 --- a/test/test_git_index_lock.py +++ b/test/test_git_index_lock.py @@ -11,6 +11,7 @@ import subprocess import sys import time from pathlib import Path +import contextlib from unittest.mock import patch import pytest @@ -27,6 +28,13 @@ needs_git = pytest.mark.skipif(shutil.which('git') is None, reason='git not inst OLD = 1_000_000.0 +@contextlib.contextmanager +def _patched(fake): + """Fake git for both the plain subprocess calls and the timeout-safe runner.""" + with patch.object(mod.subprocess, 'run', fake), patch.object(mod.git_index_lock, 'run_git', fake): + yield + + def _checkout(tmp_path, lock_age=None): repo = tmp_path / 'LEDMatrix' (repo / '.git').mkdir(parents=True) @@ -173,7 +181,7 @@ def test_update_code_clears_the_lock_before_running_git(client): return gil.REMOVED calls = [] with patch.object(gil, 'clear_stale_index_lock', clear), \ - patch.object(mod.subprocess, 'run', _fake_git(calls)): + _patched(_fake_git(calls)): data = _update(client) assert seen == [str(mod.PROJECT_ROOT)] assert data['status'] == 'success' @@ -182,7 +190,7 @@ def test_update_code_clears_the_lock_before_running_git(client): def test_update_code_waits_for_a_git_that_is_still_running(client): calls = [] with patch.object(gil, 'clear_stale_index_lock', lambda d: gil.IN_USE), \ - patch.object(mod.subprocess, 'run', _fake_git(calls)): + _patched(_fake_git(calls)): data = _update(client) assert data['status'] == 'error' assert 'Another git command is running' in data['message'] @@ -193,8 +201,125 @@ def test_a_lock_it_could_not_clear_gets_the_manual_fix(client): err = "fatal: Unable to create '/home/ledpi/LEDMatrix/.git/index.lock': File exists.\n" calls = [] with patch.object(gil, 'clear_stale_index_lock', lambda d: gil.UNKNOWN), \ - patch.object(mod.subprocess, 'run', _fake_git(calls, pull_rc=128, pull_err=err)): + _patched(_fake_git(calls, pull_rc=128, pull_err=err)): data = _update(client) assert data['status'] == 'error' assert data['message'].startswith('Update failed: fatal: Unable to create') assert 'rm -f ' in data['message'] and 'index.lock' in data['message'] + + +# -- run_git: a timed-out git must not leave a lock or an orphaned helper ------ + +def _lock_holder_script(tmp_path, lock): + """A stand-in for a slow git: takes the lock, spawns a helper, then hangs.""" + script = tmp_path / 'slow.sh' + script.write_text(f'#!/bin/sh\n: > "{lock}"\nsleep 60 &\necho $! > "{tmp_path}/helper.pid"\nwait\n') + script.chmod(0o755) + return script + + +def test_run_git_returns_a_completed_process(tmp_path): + result = gil.run_git([sys.executable, '-c', 'print("hi")'], cwd=str(tmp_path), timeout=30) + assert (result.returncode, result.stdout.strip()) == (0, 'hi') + + +@pytest.mark.skipif(not os.path.isdir('/proc'), reason='needs /proc') +def test_run_git_timeout_stops_the_group_and_clears_the_lock(tmp_path, monkeypatch): + (tmp_path / '.git').mkdir() + lock = tmp_path / '.git' / 'index.lock' + script = _lock_holder_script(tmp_path, lock) + monkeypatch.setattr(gil, 'MIN_STALE_AGE_SECONDS', 0) + monkeypatch.setattr(gil, 'TERM_GRACE_SECONDS', 1) + # The scan only sees processes named git, so the stand-in must not match; + # the lock is then provably unowned once the group is gone. + with pytest.raises(subprocess.TimeoutExpired): + gil.run_git([str(script)], cwd=str(tmp_path), timeout=1) + assert not lock.exists() + helper = int((tmp_path / 'helper.pid').read_text()) + time.sleep(0.2) + with pytest.raises(ProcessLookupError): + os.kill(helper, 0) + + +def test_read_only_env_disables_optional_locks(): + assert gil.read_only_env()['GIT_OPTIONAL_LOCKS'] == '0' + + +def test_run_git_waits_out_the_freshness_window_before_giving_up_on_the_lock(tmp_path, monkeypatch): + # The lock is created a moment before the timeout, so on age alone it is + # still "in use" once git is stopped; it must not be left to block the retry. + (tmp_path / '.git').mkdir() + lock = tmp_path / '.git' / 'index.lock' + script = tmp_path / 'slow.sh' + script.write_text(f'#!/bin/sh\n: > "{lock}"\nexec sleep 60\n') + script.chmod(0o755) + monkeypatch.setattr(gil, 'MIN_STALE_AGE_SECONDS', 2) + monkeypatch.setattr(gil, 'TERM_GRACE_SECONDS', 1) + with pytest.raises(subprocess.TimeoutExpired): + gil.run_git([str(script)], cwd=str(tmp_path), timeout=1) + assert not lock.exists() + + +# -- a rebase in the checkout ---------------------------------------------------- + +def _rebase_checkout(tmp_path, monkeypatch): + from web_interface.blueprints.api_v3 import system + (tmp_path / '.git' / 'rebase-merge').mkdir(parents=True) + monkeypatch.setattr(system, 'PROJECT_ROOT', tmp_path) + return tmp_path / '.git' / 'rebase-merge' + + +def _rebase_git(calls, on_branch, quit_rc=0, remove_dir=None): + base = _fake_git(calls) + + def run(args, **kwargs): + if args[:2] == ['git', 'symbolic-ref']: + calls.append(list(args)) + return subprocess.CompletedProcess(args, 0 if on_branch else 1, stdout='', stderr='') + if args[:3] == ['git', 'rebase', '--quit']: + calls.append(list(args)) + if quit_rc == 0 and remove_dir is not None: + remove_dir.rmdir() + return subprocess.CompletedProcess(args, quit_rc, stdout='', stderr='cannot quit' if quit_rc else '') + return base(args, **kwargs) + return run + + +def test_a_rebase_in_progress_is_left_alone(client, tmp_path, monkeypatch): + _rebase_checkout(tmp_path, monkeypatch) + calls = [] + with _patched(_rebase_git(calls, on_branch=False)): + data = _update(client) + assert data['status'] == 'error' and 'rebase is in progress' in data['message'] + assert not any(c[:2] in (['git', 'pull'], ['git', 'checkout']) or c[:3] == ['git', 'rebase', '--quit'] + for c in calls), "a paused rebase holds the user's conflict work" + + +def test_an_abandoned_rebase_is_quit_before_the_pull(client, tmp_path, monkeypatch): + rebase = _rebase_checkout(tmp_path, monkeypatch) + calls = [] + with _patched(_rebase_git(calls, on_branch=True, remove_dir=rebase)): + _update(client) + names = [c[1] for c in calls] + assert ['git', 'rebase', '--quit'] in calls + assert names.index('rebase') < names.index('pull') + + +def test_a_rebase_that_cannot_be_cleared_stops_the_update(client, tmp_path, monkeypatch): + _rebase_checkout(tmp_path, monkeypatch) + calls = [] + with _patched(_rebase_git(calls, on_branch=True, quit_rc=1)): + data = _update(client) + assert data['status'] == 'error' and 'could not be cleared' in data['message'] + assert not any(c[:2] == ['git', 'pull'] for c in calls) + + +def test_the_automatic_status_check_is_read_only_whatever_the_runner(tmp_path): + from web_interface import auto_update + seen = {} + + def run(args, **kwargs): + seen.update(kwargs) + return subprocess.CompletedProcess(args, 0, stdout='', stderr='') + auto_update.local_changes(tmp_path, run=run) + assert seen['env']['GIT_OPTIONAL_LOCKS'] == '0' diff --git a/test/test_update_prompts_restart.py b/test/test_update_prompts_restart.py index b4924c29..f535532b 100644 --- a/test/test_update_prompts_restart.py +++ b/test/test_update_prompts_restart.py @@ -9,6 +9,7 @@ restart is owed, and the UI raises the existing restart-pending banner. import subprocess import sys from pathlib import Path +import contextlib from unittest.mock import patch import pytest @@ -20,6 +21,13 @@ from web_interface.blueprints import api_v3 as mod # noqa: E402 from web_interface.blueprints.api_v3 import api_v3 # noqa: E402 +@contextlib.contextmanager +def _patched(fake): + """Fake git for both the plain subprocess calls and the timeout-safe runner.""" + with patch.object(mod.subprocess, 'run', fake), patch.object(mod.git_index_lock, 'run_git', fake): + yield + + @pytest.fixture def client(): app = Flask(__name__) @@ -62,7 +70,7 @@ def _pull(client): class TestRestartIsRequestedWhenCodeChanged: def test_a_pull_that_moved_head_asks_for_a_restart(self, client): - with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])): + with _patched(_git(['aaa111', 'bbb222'])): data = _pull(client) assert data['status'] == 'success' assert data['restart_required'] is True, ( @@ -70,15 +78,14 @@ class TestRestartIsRequestedWhenCodeChanged: "nothing told the user to restart") def test_already_up_to_date_does_not(self, client): - with patch.object(mod.subprocess, 'run', - _git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')): + with _patched(_git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')): data = _pull(client) assert data['status'] == 'success' assert data['restart_required'] is False, ( "prompting after a no-op update trains users to ignore the prompt") def test_a_failed_pull_does_not(self, client): - with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)): + with _patched(_git(['aaa111'], pull_rc=1)): data = _pull(client) assert data['status'] == 'error' assert data['restart_required'] is False @@ -100,19 +107,18 @@ class TestUnitsAreRefreshedWithTheCode: return calls def test_an_update_that_moved_head_refreshes_the_units(self, client, refresh): - with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])): + with _patched(_git(['aaa111', 'bbb222'])): data = _pull(client) assert refresh == [True] assert data['unit_refresh']['status'] == 'refreshed' assert 'Service settings updated' in data['message'] def test_nothing_new_touches_no_units(self, client, refresh): - with patch.object(mod.subprocess, 'run', - _git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')): + with _patched(_git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')): data = _pull(client) assert refresh == [] and data['unit_refresh'] is None def test_a_failed_pull_touches_no_units(self, client, refresh): - with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)): + with _patched(_git(['aaa111'], pull_rc=1)): data = _pull(client) assert refresh == [] and data['unit_refresh'] is None diff --git a/web_interface/auto_update.py b/web_interface/auto_update.py index 51d4e702..35ce1b6f 100644 --- a/web_interface/auto_update.py +++ b/web_interface/auto_update.py @@ -302,10 +302,12 @@ def local_changes(project_dir, run=None, timeout=30): Returns the changed paths, or None when git could not tell. Raises what ``run`` raises (e.g. ``subprocess.TimeoutExpired``). """ - run = run or subprocess.run + # A status that times out must not leave index.lock: don't refresh the index. + run = run or git_index_lock.run_git result = run(['git', '-c', 'core.fileMode=false', 'status', '--porcelain', '--untracked-files=no', '-z'], - cwd=str(project_dir), capture_output=True, text=True, timeout=timeout) + cwd=str(project_dir), capture_output=True, text=True, timeout=timeout, + env=git_index_lock.read_only_env()) if result.returncode != 0: return None changed = [] diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index d98fe958..8998da2d 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -35,6 +35,7 @@ from typing import Dict, Any, Optional, Tuple, Type from urllib.parse import urlparse, urlunparse logger = logging.getLogger(__name__) # Import new infrastructure +from web_interface import git_index_lock from src.web_interface.api_helpers import (success_response, error_response, exception_error_response, validate_request_json) from src.web_interface.errors import ErrorCode @@ -558,7 +559,8 @@ def get_git_version(project_dir=None): capture_output=True, text=True, timeout=5, - cwd=str(project_dir) + cwd=str(project_dir), + env=git_index_lock.read_only_env() ) if result.returncode == 0: diff --git a/web_interface/blueprints/api_v3/system.py b/web_interface/blueprints/api_v3/system.py index b5c6198e..eb07ec98 100644 --- a/web_interface/blueprints/api_v3/system.py +++ b/web_interface/blueprints/api_v3/system.py @@ -14,6 +14,7 @@ from web_interface.blueprints.api_v3 import ( ) import threading +from web_interface import git_index_lock from web_interface.cache import get_cached, set_cached from web_interface.system_metrics import collect_system_metrics, format_uptime import web_interface.blueprints.api_v3 as _pkg @@ -163,6 +164,7 @@ def check_for_update(): ['git', 'fetch', '--quiet', '--tags', '--force', update_channel.REMOTE, update_channel.BETA_BRANCH], capture_output=True, timeout=10, cwd=cwd, + env=git_index_lock.read_only_env(), ) if fetch_result.returncode != 0: stderr = fetch_result.stderr.decode(errors='replace').strip() @@ -332,7 +334,7 @@ def channel_status(project_dir=None, fetch=True): def _perform_core_update_locked(stash_local_changes=True): project_dir = str(PROJECT_ROOT) - from web_interface import update_channel, git_index_lock + from web_interface import update_channel # A git killed mid-command (a timed-out pull, a power cut) leaves # .git/index.lock behind, and every later update then fails with @@ -343,6 +345,28 @@ def _perform_core_update_locked(stash_local_changes=True): 'message': 'Another git command is running in the LEDMatrix folder; ' 'try again in a minute.'} + # A pull killed mid-rebase leaves .git/rebase-merge behind, and every + # later `pull --rebase` then fails. Same rule as the automatic update's + # preflight: a rebase really in progress leaves HEAD detached and holds + # the user's conflict work, so leave it; with HEAD back on a branch it was + # abandoned, and `--quit` drops its state without touching the tree. + git_dir = Path(project_dir) / '.git' + rebase_dirs = [git_dir / name for name in ('rebase-merge', 'rebase-apply') if (git_dir / name).exists()] + if rebase_dirs: + def _git(*args): + return git_index_lock.run_git(['git', *args], capture_output=True, text=True, + timeout=60, cwd=project_dir) + if _git('symbolic-ref', '-q', 'HEAD').returncode != 0: + return {'status': 'error', 'restart_required': False, 'dependency_failures': [], + 'message': 'A git rebase is in progress in the LEDMatrix folder. ' + 'Finish or abort it (git rebase --continue / --abort), then update again.'} + cleared = _git('rebase', '--quit') + if cleared.returncode != 0 or any(path.exists() for path in rebase_dirs): + return {'status': 'error', 'restart_required': False, 'dependency_failures': [], + 'message': 'An abandoned git rebase in the LEDMatrix folder could not be cleared: ' + f'{(cleared.stderr or "").strip() or "unknown error"}.'} + logger.warning("Cleared an abandoned git rebase from %s", ', '.join(str(p) for p in rebase_dirs)) + # Which code to move to: the newest release (stable) or main (beta). # See web_interface/update_channel.py; it never picks an older commit. channel, fetch_error = channel_status(project_dir) @@ -434,25 +458,33 @@ def _perform_core_update_locked(stash_local_changes=True): # Stable checks out the release tag, carrying edits across the way the # pull's --autostash does. channel_note = '' - if action == update_channel.ACTION_CHECKOUT_TAG: - result, autostash_note = update_channel.checkout_release(project_dir, channel.newest_release) - channel_note = f"Now on release {channel.newest_release} (stable channel). {autostash_note}".strip() - elif action == update_channel.ACTION_SWITCH_TO_BETA: - result, autostash_note = update_channel.checkout_beta_branch(project_dir) - if result.returncode == 0: - result = subprocess.run(['git', 'pull', '--rebase', '--autostash'], - capture_output=True, text=True, timeout=60, cwd=project_dir) - channel_note = f"Now following {update_channel.BETA_BRANCH} (beta channel). {autostash_note}".strip() - else: - result = subprocess.run( - pull_args, - capture_output=True, - text=True, - timeout=60, - cwd=project_dir - ) - if channel.waiting and channel.newest_release: - channel_note = channel.message + try: + if action == update_channel.ACTION_CHECKOUT_TAG: + result, autostash_note = update_channel.checkout_release(project_dir, channel.newest_release) + channel_note = f"Now on release {channel.newest_release} (stable channel). {autostash_note}".strip() + elif action == update_channel.ACTION_SWITCH_TO_BETA: + result, autostash_note = update_channel.checkout_beta_branch(project_dir) + if result.returncode == 0: + result = git_index_lock.run_git(['git', 'pull', '--rebase', '--autostash'], + capture_output=True, text=True, + timeout=git_index_lock.MUTATING_TIMEOUT_SECONDS, cwd=project_dir) + channel_note = f"Now following {update_channel.BETA_BRANCH} (beta channel). {autostash_note}".strip() + else: + result = git_index_lock.run_git( + pull_args, + capture_output=True, + text=True, + timeout=git_index_lock.MUTATING_TIMEOUT_SECONDS, + cwd=project_dir + ) + if channel.waiting and channel.newest_release: + channel_note = channel.message + except subprocess.TimeoutExpired: + # run_git has stopped git and removed the lock it left. + logger.warning("git timed out during Update Code") + return {'status': 'error', 'restart_required': False, 'dependency_failures': [], + 'message': 'Update failed: git took too long (a slow network or SD card). ' + 'The partial update was cleaned up; try again.'} # Give the branch tracking information so the next pull is a plain # `git pull` — otherwise every update repeats the fallback. diff --git a/web_interface/git_index_lock.py b/web_interface/git_index_lock.py index 8ccaa05e..c37d507b 100644 --- a/web_interface/git_index_lock.py +++ b/web_interface/git_index_lock.py @@ -18,6 +18,8 @@ the file alone. Standard library only. """ import logging import os +import signal +import subprocess # nosec B404 - list-form argv only, no shell # nosemgrep import time from pathlib import Path @@ -33,6 +35,15 @@ REMOVED = 'removed' # it was stale and has been removed IN_USE = 'in_use' # a git process is running here (or the lock is brand new) UNKNOWN = 'unknown' # could not tell whether git is running; left alone +#: How long a timed-out git gets to exit after SIGTERM (which lets it remove +#: its own lock) before the whole process group is SIGKILLed. +TERM_GRACE_SECONDS = 5 + +#: Timeout for commands that rewrite the checkout (pull, checkout, reset, +#: stash). A Pi on an SD card can take minutes; killing git is what leaves the +#: lock behind, so this is generous. +MUTATING_TIMEOUT_SECONDS = 600 + _PROC = Path('/proc') @@ -137,7 +148,73 @@ def clear_stale_index_lock(project_dir, proc_root=_PROC, now=None): return REMOVED +def _stop_group(proc): + """SIGTERM the process group, then SIGKILL whatever is left of it.""" + try: + os.killpg(proc.pid, signal.SIGTERM) + except (ProcessLookupError, PermissionError): + pass + try: + proc.wait(timeout=TERM_GRACE_SECONDS) + except subprocess.TimeoutExpired: + pass + # Also reaps git's children (git-remote-https, rebase helpers) that + # outlive a killed parent. + try: + os.killpg(proc.pid, signal.SIGKILL) + except (ProcessLookupError, PermissionError): + pass + + +def _clear_after_stop(project_dir): + """Remove the lock a stopped git left, waiting out the freshness window. + + A lock created within ``MIN_STALE_AGE_SECONDS`` of the stop reads as IN_USE + on age alone; with no git process left it only needs to age, so wait for + that rather than leave it to block the next attempt. + """ + deadline = time.monotonic() + MIN_STALE_AGE_SECONDS + 1 + while clear_stale_index_lock(project_dir) == IN_USE and git_running_in(project_dir) is False: + if time.monotonic() >= deadline: + break + time.sleep(0.5) + + +def run_git(cmd, *, cwd=None, timeout=None, capture_output=True, text=True, **kwargs): + """``subprocess.run`` for git, without leaving ``.git/index.lock`` behind. + + ``subprocess.run`` SIGKILLs only the direct child when ``timeout`` expires, + which skips git's cleanup and orphans its helpers. This starts git in its + own session, stops the whole group with SIGTERM before SIGKILL, removes a + lock nothing owns any more, then raises ``TimeoutExpired`` as + ``subprocess.run`` would. + """ + if capture_output: + kwargs['stdout'] = kwargs['stderr'] = subprocess.PIPE + with subprocess.Popen(cmd, cwd=cwd, text=text, start_new_session=True, **kwargs) as proc: # nosec B603 - list-form argv # nosemgrep + try: + stdout, stderr = proc.communicate(timeout=timeout) + except subprocess.TimeoutExpired as exc: + _stop_group(proc) + exc.stdout, exc.stderr = proc.communicate() + if cwd is not None: + _clear_after_stop(cwd) + raise + except BaseException: + _stop_group(proc) + raise + return subprocess.CompletedProcess(proc.args, proc.returncode, stdout, stderr) # result object only, nothing is run # nosemgrep + + +def read_only_env(): + """Environment for git calls that only read: ``GIT_OPTIONAL_LOCKS=0`` + stops ``status`` and ``describe --dirty`` refreshing (and so locking) the + index, so a poll that times out cannot leave a lock.""" + return {**os.environ, 'GIT_OPTIONAL_LOCKS': '0'} + + def manual_fix_hint(project_dir): """The sentence shown when a lock blocks the update and was not removed.""" - return (f"If no other git command is running, remove the stale lock with " - f"rm -f {Path(project_dir) / '.git' / 'index.lock'} and try again.") + lock = Path(project_dir) / '.git' / 'index.lock' + return (f"If it keeps coming back, stop the LEDMatrix services, kill any running git " + f"(ps aux | grep git), then remove it with rm -f {lock} and try again.") diff --git a/web_interface/update_channel.py b/web_interface/update_channel.py index f7fdb3f2..597bed1a 100644 --- a/web_interface/update_channel.py +++ b/web_interface/update_channel.py @@ -24,6 +24,8 @@ import re import subprocess # nosec B404 - list-form argv only, no shell # nosemgrep from pathlib import Path +from web_interface import git_index_lock + PROJECT_ROOT = Path(__file__).resolve().parent.parent CHANNELS = ('stable', 'beta') DEFAULT_CHANNEL = 'stable' @@ -105,7 +107,7 @@ class ChannelStatus(dict): def _git(project_dir, run, *args, timeout=30): - run = run or subprocess.run + run = run or git_index_lock.run_git return run(['git', *args], cwd=str(project_dir), capture_output=True, text=True, timeout=timeout)