fix(install): discard managed lockfile churn before stashing
This commit is contained in:
parent
cb7d1f68f8
commit
3cf900eb67
4 changed files with 207 additions and 0 deletions
|
|
@ -245,7 +245,41 @@ function Invoke-NativeWithRelaxedErrorAction {
|
|||
$ErrorActionPreference = $prevEAP
|
||||
}
|
||||
}
|
||||
function Discard-LockfileChurn {
|
||||
param([string]$Repo = $InstallDir)
|
||||
|
||||
if (-not $Repo -or -not (Test-Path (Join-Path $Repo ".git"))) { return }
|
||||
|
||||
try {
|
||||
$diff = & git -c windows.appendAtomically=false -C $Repo diff --name-only 2>$null
|
||||
if ($LASTEXITCODE -ne 0 -or -not $diff) { return }
|
||||
|
||||
$dirtyPackageDirs = [System.Collections.Generic.HashSet[string]]::new(
|
||||
[System.StringComparer]::OrdinalIgnoreCase
|
||||
)
|
||||
foreach ($path in $diff) {
|
||||
if ($path -like "*package.json") {
|
||||
$null = $dirtyPackageDirs.Add((Split-Path $path -Parent))
|
||||
}
|
||||
}
|
||||
|
||||
$dirtyLocks = [System.Collections.Generic.List[string]]::new()
|
||||
foreach ($path in $diff) {
|
||||
if ($path -notlike "*package-lock.json") { continue }
|
||||
$lockDir = Split-Path $path -Parent
|
||||
if ($dirtyPackageDirs.Contains($lockDir)) { continue }
|
||||
$dirtyLocks.Add($path)
|
||||
}
|
||||
|
||||
if ($dirtyLocks.Count -eq 0) { return }
|
||||
& git -c windows.appendAtomically=false -C $Repo checkout -- @($dirtyLocks) 2>$null
|
||||
if ($LASTEXITCODE -eq 0) {
|
||||
Write-Info "Discarded npm lockfile churn ($($dirtyLocks.Count) file(s))"
|
||||
}
|
||||
} catch {
|
||||
# Best-effort only; never let cleanup block the installer update path.
|
||||
}
|
||||
}
|
||||
# Inspect npm output for a TLS-trust failure and, if found, print actionable
|
||||
# remediation. npm/Node surface corporate MITM proxies and missing root CAs as
|
||||
# "unable to get local issuer certificate" / "self-signed certificate in
|
||||
|
|
@ -1281,6 +1315,7 @@ function Install-Repository {
|
|||
# users hit on update. Pin autocrlf=false so the dirt is never
|
||||
# created in the first place.
|
||||
git -c windows.appendAtomically=false config core.autocrlf false 2>$null
|
||||
Discard-LockfileChurn $InstallDir
|
||||
# Preserve any real local changes before the checkout instead of
|
||||
# discarding them with `reset --hard HEAD`. The old hard reset
|
||||
# silently destroyed agent-edited source on managed clones (the
|
||||
|
|
|
|||
|
|
@ -258,6 +258,58 @@ restore_dirty_lockfiles() {
|
|||
done
|
||||
}
|
||||
|
||||
# npm rewrites tracked package-lock.json files non-deterministically during
|
||||
# local builds. On a managed install those diffs are usually runtime churn, not
|
||||
# intentional user edits, so discard them before the repository-stage stash.
|
||||
# If package.json in the same directory is also dirty we keep both changes.
|
||||
discard_update_lockfile_churn() {
|
||||
local repo="${1:-$INSTALL_DIR}"
|
||||
[ -n "$repo" ] && [ -d "$repo/.git" ] || return 0
|
||||
command -v git >/dev/null 2>&1 || return 0
|
||||
|
||||
local dirty_diff
|
||||
dirty_diff=$(git -C "$repo" diff --name-only 2>/dev/null) || return 0
|
||||
[ -n "$dirty_diff" ] || return 0
|
||||
|
||||
local dirty_package_dirs=""
|
||||
while IFS= read -r path; do
|
||||
case "$path" in
|
||||
*package.json)
|
||||
dirty_package_dirs="${dirty_package_dirs}$(dirname "$path")"$'\n'
|
||||
;;
|
||||
esac
|
||||
done <<EOF
|
||||
$dirty_diff
|
||||
EOF
|
||||
|
||||
local dirty_locks=""
|
||||
local dirty_count=0
|
||||
while IFS= read -r path; do
|
||||
case "$path" in
|
||||
*package-lock.json)
|
||||
local lock_dir
|
||||
lock_dir=$(dirname "$path")
|
||||
case $'\n'"$dirty_package_dirs" in
|
||||
*$'\n'"$lock_dir"$'\n'*) continue ;;
|
||||
esac
|
||||
dirty_locks="${dirty_locks}${path}"$'\n'
|
||||
dirty_count=$((dirty_count + 1))
|
||||
;;
|
||||
esac
|
||||
done <<EOF
|
||||
$dirty_diff
|
||||
EOF
|
||||
|
||||
[ "$dirty_count" -gt 0 ] || return 0
|
||||
while IFS= read -r path; do
|
||||
[ -n "$path" ] || continue
|
||||
git -C "$repo" checkout -- "$path" 2>/dev/null || true
|
||||
done <<EOF
|
||||
$dirty_locks
|
||||
EOF
|
||||
log_info "Discarded npm lockfile churn (${dirty_count} file(s))"
|
||||
}
|
||||
|
||||
emit_manifest() {
|
||||
# Stage-Desktop is included only with --include-desktop, mirroring
|
||||
# install.ps1: the signed bootstrap installer (Hermes-Setup) passes it so
|
||||
|
|
@ -1136,6 +1188,7 @@ clone_repo() {
|
|||
cd "$INSTALL_DIR"
|
||||
|
||||
local autostash_ref=""
|
||||
discard_update_lockfile_churn "$INSTALL_DIR"
|
||||
if [ -n "$(git status --porcelain)" ]; then
|
||||
# A previously interrupted update can leave the index with
|
||||
# unmerged entries. In that state `git stash` aborts with
|
||||
|
|
|
|||
110
tests/test_install_lockfile_churn.py
Normal file
110
tests/test_install_lockfile_churn.py
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
"""Regression: installer update should discard pure npm lockfile churn.
|
||||
|
||||
Desktop/bootstrap installs update an existing managed checkout in place. Local
|
||||
build steps often rewrite tracked ``package-lock.json`` without touching the
|
||||
matching ``package.json``; treating that churn as a real local edit forces an
|
||||
autostash and can abort the repository stage before the desktop comes back up.
|
||||
|
||||
The installer should discard that generated churn before its stash/checkout
|
||||
logic, while still preserving intentional package edits where ``package.json``
|
||||
and ``package-lock.json`` changed together.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALL_SH = REPO_ROOT / "scripts" / "install.sh"
|
||||
INSTALL_PS1 = REPO_ROOT / "scripts" / "install.ps1"
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
shutil.which("git") is None or shutil.which("bash") is None,
|
||||
reason="needs git and bash",
|
||||
)
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
["git", "-c", "user.email=t@t", "-c", "user.name=t", *args],
|
||||
cwd=cwd,
|
||||
check=check,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
|
||||
def _extract_install_sh_function(name: str) -> str:
|
||||
text = INSTALL_SH.read_text()
|
||||
match = re.search(rf"{name}\(\) \{{.*?\n\}}", text, re.DOTALL)
|
||||
assert match is not None, f"{name}() not found in install.sh"
|
||||
return match.group(0)
|
||||
|
||||
|
||||
def _extract_install_sh_autostash_block() -> str:
|
||||
text = INSTALL_SH.read_text()
|
||||
match = re.search(
|
||||
r'local autostash_ref="".*?\n fi\n',
|
||||
text,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match is not None, "autostash block not found in install.sh"
|
||||
return match.group(0)
|
||||
|
||||
|
||||
@pytest.mark.live_system_guard_bypass
|
||||
def test_install_sh_discards_runtime_lockfile_churn_before_stash(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
repo = tmp_path / "hermes-agent"
|
||||
repo.mkdir()
|
||||
_git(repo, "init")
|
||||
(repo / "package.json").write_text('{"dependencies":{"a":"1"}}\n')
|
||||
(repo / "package-lock.json").write_text('{"lock":"old"}\n')
|
||||
_git(repo, "add", "package.json", "package-lock.json")
|
||||
_git(repo, "commit", "-m", "init")
|
||||
|
||||
(repo / "package-lock.json").write_text('{"lock":"runtime-churn"}\n')
|
||||
|
||||
script = (
|
||||
"set -e\n"
|
||||
'log_info() { echo "INFO: $*"; }\n'
|
||||
'INSTALL_DIR="$PWD"\n'
|
||||
f"{_extract_install_sh_function('discard_update_lockfile_churn')}\n"
|
||||
"run() {\n"
|
||||
f"{_extract_install_sh_autostash_block()}"
|
||||
"}\n"
|
||||
"run\n"
|
||||
)
|
||||
res = subprocess.run(
|
||||
["bash", "-c", script], cwd=repo, capture_output=True, text=True
|
||||
)
|
||||
|
||||
assert res.returncode == 0, res.stderr
|
||||
assert "Discarded npm lockfile churn (1 file(s))" in res.stdout
|
||||
assert _git(repo, "stash", "list").stdout.strip() == ""
|
||||
assert (repo / "package-lock.json").read_text() == '{"lock":"old"}\n'
|
||||
|
||||
|
||||
def test_install_sh_discards_lockfile_churn_before_status_probe() -> None:
|
||||
text = INSTALL_SH.read_text()
|
||||
idx_cleanup = text.index('discard_update_lockfile_churn "$INSTALL_DIR"')
|
||||
idx_status = text.index('if [ -n "$(git status --porcelain)" ]')
|
||||
idx_stash = text.index("git stash push --include-untracked")
|
||||
assert idx_cleanup < idx_status < idx_stash
|
||||
|
||||
|
||||
def test_install_ps1_discards_lockfile_churn_before_status_probe() -> None:
|
||||
text = INSTALL_PS1.read_text()
|
||||
assert "function Discard-LockfileChurn" in text
|
||||
idx_cleanup = text.index("Discard-LockfileChurn $InstallDir")
|
||||
idx_status = text.index(
|
||||
"$statusOut = git -c windows.appendAtomically=false status --porcelain"
|
||||
)
|
||||
idx_stash = text.index("stash push --include-untracked")
|
||||
assert idx_cleanup < idx_status < idx_stash
|
||||
|
|
@ -52,6 +52,13 @@ def _extract_autostash_block() -> str:
|
|||
return m.group(0)
|
||||
|
||||
|
||||
def _extract_install_sh_function(name: str) -> str:
|
||||
text = INSTALL_SH.read_text()
|
||||
match = re.search(rf"{name}\(\) \{{.*?\n\}}", text, re.DOTALL)
|
||||
assert match is not None, f"{name}() not found in install.sh"
|
||||
return match.group(0)
|
||||
|
||||
|
||||
def _make_unmerged_repo(repo: Path) -> None:
|
||||
"""Leave ``repo`` with a conflicted (unmerged) index, as an interrupted
|
||||
update would."""
|
||||
|
|
@ -92,6 +99,8 @@ def test_install_sh_clears_unmerged_index_then_stashes(tmp_path: Path) -> None:
|
|||
script = (
|
||||
"set -e\n"
|
||||
'log_info() { echo "INFO: $*"; }\n'
|
||||
f'INSTALL_DIR="{repo}"\n'
|
||||
f"{_extract_install_sh_function('discard_update_lockfile_churn')}\n"
|
||||
"run() {\n"
|
||||
f"{block}"
|
||||
"}\n"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue