perf(update): skip the web build's npm ci when the manifests digest is unchanged
`hermes update` pass 1 installs ui-tui + web + root and records the manifests digest after success; `_do_build_web_ui` then ran `npm ci` over the same closure unconditionally, wiping and re-reifying node_modules. Reuse the pass-1 gate (which also verifies node_modules and the tsc/vite shims) so an unchanged lockfile builds straight away. The missing-tool reinstall fallback is kept. Ref #43837
This commit is contained in:
@@ -499,9 +499,17 @@ def _do_build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool:
|
||||
# looks identical to a hang and users reboot mid-install).
|
||||
return _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env)
|
||||
|
||||
r1 = _install_web_deps(silent=True)
|
||||
if r1.returncode != 0:
|
||||
return _report_web_build_failure("npm install", r1, fatal=fatal)
|
||||
# `hermes update` already installed this exact closure and recorded the manifests digest after
|
||||
# success; while it still matches (node_modules + toolchain checked inside), `npm ci` here would
|
||||
# only wipe and re-reify the identical tree. Only the root lockfile is digested, so a web/ that
|
||||
# owns its lockfile always installs. See #43837.
|
||||
from hermes_cli.main import PROJECT_ROOT
|
||||
from hermes_cli.update_cmd_deps import _npm_lockfile_changed
|
||||
from hermes_constants import get_default_hermes_root
|
||||
if npm_cwd != PROJECT_ROOT or _npm_lockfile_changed(get_default_hermes_root()):
|
||||
r1 = _install_web_deps(silent=True)
|
||||
if r1.returncode != 0:
|
||||
return _report_web_build_failure("npm install", r1, fatal=fatal)
|
||||
r2 = _build()
|
||||
if r2.returncode != 0:
|
||||
# The install can exit 0 over a half-installed tree (lockfile-hash skip,
|
||||
|
||||
@@ -419,3 +419,28 @@ class TestBuildRecoversFromMissingToolchain:
|
||||
assert mock_install.call_count == 1
|
||||
assert mock_build.call_count == 1
|
||||
|
||||
|
||||
|
||||
class TestBuildSkipsRedundantInstall:
|
||||
"""`hermes update` pass 1 installs the same closure; pass 2 must not `npm ci` it again. See #43837."""
|
||||
|
||||
@staticmethod
|
||||
def _run(tmp_path, monkeypatch, *, lock_changed: bool) -> tuple[bool, int, int]:
|
||||
import hermes_cli.main as main_mod
|
||||
web_dir, _ = _make_web_dir(tmp_path)
|
||||
(tmp_path / "package-lock.json").write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path)
|
||||
ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
|
||||
with patch("hermes_cli.main_install_repair._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \
|
||||
patch("hermes_cli.update_cmd_deps._npm_lockfile_changed", return_value=lock_changed), \
|
||||
patch("hermes_cli.main_web_build._run_npm_install_deterministic", return_value=ok) as mock_install, \
|
||||
patch("hermes_cli.main_web_build._run_with_idle_timeout", return_value=ok) as mock_build, \
|
||||
patch("hermes_cli.main_web_build._web_ui_build_needed", return_value=True), \
|
||||
patch("hermes_cli.main_web_build._write_web_ui_build_stamp"):
|
||||
return _build_web_ui(web_dir), mock_install.call_count, mock_build.call_count
|
||||
|
||||
def test_unchanged_manifests_build_without_reinstalling(self, tmp_path, monkeypatch):
|
||||
assert self._run(tmp_path, monkeypatch, lock_changed=False) == (True, 0, 1)
|
||||
|
||||
def test_changed_manifests_still_install_before_building(self, tmp_path, monkeypatch):
|
||||
assert self._run(tmp_path, monkeypatch, lock_changed=True) == (True, 1, 1)
|
||||
|
||||
Reference in New Issue
Block a user