perf(nodejs): derive a monorepo function's dependency closure from the lockfile

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -10,6 +10,7 @@
 from aws_lambda_builders import utils
 from aws_lambda_builders.actions import ActionFailedError, BaseAction, Purpose
 from aws_lambda_builders.utils import extract_tarfile
+from aws_lambda_builders.workflows.nodejs_npm.lockfile_closure import production_closure
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError, SubprocessNpm
 
 LOG = logging.getLogger(__name__)
@@ -421,7 +422,10 @@ def __init__(self, install_dir, project_root, artifacts_dir, subprocess_npm, osu
         self._osutils = osutils
 
     def execute(self):
-        closure = self._subprocess_npm.resolve_dependency_closure(self._install_dir)
+        # read the install's own record rather than paying an npm process per function
+        closure = production_closure(self._project_root, self._install_dir)
+        if closure is None:
+            closure = self._subprocess_npm.resolve_dependency_closure(self._install_dir)
         destination = os.path.join(self._artifacts_dir, "node_modules")
 
         if closure is None:
@@ -562,8 +566,8 @@ def _outermost_packages(self, closure):
         is - hoisting it would shadow the top-level version for every other caller - and it is already
         reachable through the dependency that contains it, so only the outermost paths are linked.
         """
-        # Compare through normcase, link the original path: npm's spelling and the build's can differ in
-        # case on Windows and still name one directory. See aws/aws-lambda-builders#935.
+        # Compare through normcase, link the original path: npm's spelling and the build's can differ
+        # in case on Windows and still name one directory. See aws/aws-lambda-builders#935.
         paths = [os.path.realpath(path) for path in closure]
         excluded = {os.path.normcase(os.path.realpath(d)) for d in (self._project_root, self._install_dir)}
         candidates = [path for path in paths if os.path.normcase(path) not in excluded]

aws_lambda_builders/workflows/nodejs_npm/lockfile_closure.py

@@ -0,0 +1,143 @@
+"""
+Derive a workspace member's production dependency closure from a lockfile, with no npm subprocess.
+
+Replaces `npm ls --all --parseable --omit=dev` for the #933 artifact-narrowing step, and answers the
+same shape: absolute real paths, one per resolved package. None means the caller should ask npm.
+
+Reads npm's HIDDEN lockfile, `node_modules/.package-lock.json`, not the project's own: only the hidden
+one records the tree npm reified. See aws/aws-lambda-builders#938.
+"""
+
+import json
+import logging
+import os
+from typing import Dict, List, Optional, Set
+
+LOG = logging.getLogger(__name__)
+
+# lockfileVersion 1 predates the per-path `packages` map this resolver reads
+MINIMUM_LOCKFILE_VERSION = 2
+
+# npm's record of the tree it reified, written on every install from npm 7 on
+HIDDEN_LOCKFILE = os.path.join("node_modules", ".package-lock.json")
+
+
+def hidden_lockfile_path(project_root: str) -> str:
+    """Where npm records the tree it installed under `project_root`."""
+    return os.path.join(project_root, HIDDEN_LOCKFILE)
+
+
+def _describes_the_installed_tree(project_root: str) -> bool:
+    """
+    Is npm's hidden lockfile still a true record of `node_modules`?
+
+    npm's own staleness test, which it applies before trusting the file itself. `>=`, not `>`: a fresh
+    install writes both in one clock tick. See aws/aws-lambda-builders#938.
+    """
+    try:
+        hidden = os.path.getmtime(hidden_lockfile_path(project_root))
+        tree = os.path.getmtime(os.path.join(project_root, "node_modules"))
+    except OSError:
+        return False
+    return hidden >= tree
+
+
+def _resolve_key(packages: Dict, from_key: str, name: str) -> Optional[str]:
+    """
+    Find the packages-map key that `name` resolves to when required from `from_key`.
+
+    Walks up looking for `node_modules/<name>` the way node does. `from_key` is "" at the root,
+    else a member path like "endpoints/orders" or "node_modules/express".
+    """
+    prefix = from_key
+    while True:
+        candidate = f"{prefix}/node_modules/{name}" if prefix else f"node_modules/{name}"
+        if candidate in packages:
+            return candidate
+        if not prefix:
+            return None
+        # strip one path segment and try again, which is what walking up node_modules means
+        prefix = prefix.rsplit("/", 1)[0] if "/" in prefix else ""
+        if prefix.endswith("/node_modules"):
+            prefix = prefix[: -len("/node_modules")]
+
+
+def _entry_dir(project_root: str, key: str, entry: Dict) -> str:
+    """The directory a packages-map entry lives in."""
+    # a workspace link points at the real source directory rather than the link's own path
+    if entry.get("link") and entry.get("resolved"):
+        return os.path.join(project_root, *entry["resolved"].split("/"))
+    return os.path.join(project_root, *key.split("/")) if key else project_root
+
+
+def production_closure(project_root: str, install_dir: str) -> Optional[List[str]]:
+    """
+    The production dependency closure of `install_dir`, as absolute directories.
+
+    None when the hidden lockfile cannot answer: absent, stale, too old a version, unreadable, or
+    silent about this install directory.
+    """
+    if not _describes_the_installed_tree(project_root):
+        LOG.debug("NODEJS no current hidden lockfile under %s; asking npm instead", project_root)
+        return None
+
+    try:
+        with open(hidden_lockfile_path(project_root)) as handle:
+            lock = json.load(handle)
+    except (OSError, ValueError):
+        return None
+
+    if lock.get("lockfileVersion", 1) < MINIMUM_LOCKFILE_VERSION or "packages" not in lock:
+        return None
+
+    packages = lock["packages"]
+    project_root = os.path.realpath(project_root)
+    rel = os.path.relpath(os.path.realpath(install_dir), project_root).replace(os.sep, "/")
+    start = "" if rel == "." else rel
+    if start not in packages:
+        return None
+
+    # a link entry for a workspace member carries no dependencies; its source entry does
+    def deps_of(key: str) -> Dict[str, str]:
+        entry = packages.get(key) or {}
+        if entry.get("link") and entry.get("resolved"):
+            entry = packages.get(entry["resolved"], entry)
+        # Production only. Peers included because npm 7+ installs them and `npm ls --all` walks them;
+        # a declined optional peer is dropped by the on-disk check below.
+        out = dict(entry.get("dependencies") or {})
+        out.update(entry.get("optionalDependencies") or {})
+        out.update(entry.get("peerDependencies") or {})
+        return out
+
+    found: Set[str] = set()
+    stack = [start]
+    while stack:
+        key = stack.pop()
+        for name in deps_of(key):
+            resolved = _resolve_key(packages, key, name)
+            if resolved is None or resolved in found:
+                continue
+            entry = packages[resolved]
+            if entry.get("dev"):
+                continue
+            found.add(resolved)
+            # resolve a link's dependencies from its source directory, as node does through the real
+            # path: walking up from the link would find a hoisted copy the source nests
+            if entry.get("link") and entry.get("resolved"):
+                stack.append(entry["resolved"])
+            else:
+                stack.append(resolved)
+
+    # Keep only what is on disk, as npm ls does: an optional dependency excluded by os/cpu/libc is
+    # listed but not installed.
+    dirs = set()
+    for key in found:
+        directory = _entry_dir(project_root, key, packages[key])
+        if os.path.isdir(directory):
+            dirs.add(directory)
+        else:
+            LOG.debug("NODEJS lockfile lists %s but it is not installed; leaving it out", key)
+    # npm --parseable prints these too and the caller filters them, so emit them for parity
+    dirs.add(project_root)
+    dirs.add(os.path.realpath(install_dir))
+    return sorted(dirs)

tests/integration/workflows/nodejs_npm/test_nodejs_npm.py

@@ -13,6 +13,7 @@
 from aws_lambda_builders.builder import LambdaBuilder
 from aws_lambda_builders.exceptions import WorkflowFailedError
 from aws_lambda_builders.supported_runtimes import NODEJS_RUNTIMES
+from aws_lambda_builders.workflows.nodejs_npm.lockfile_closure import production_closure
 from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
 from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_NODEJS_MONOREPO, OSUtils
 from tests.testing_utils import read_link_without_junction_prefix
@@ -538,6 +539,58 @@ def test_build_in_source_in_workspaces_monorepo_links_the_hoisted_dependencies(s
         )
         self.assertEqual(require_handler.returncode, 0, require_handler.stderr)
 
+    def test_the_lockfile_closure_agrees_with_npm_ls_on_a_real_install(self):
+        # the property that lets the lockfile replace `npm ls`, and one a unit test cannot establish:
+        # it would compare this code against a fixture written from this code
+        monorepo_dir = os.path.join(self.temp_testdata_dir, "workspaces-monorepo")
+        npm = SubprocessNpm(OSUtils())
+        npm.run(["install", "--silent", "--no-audit", "--no-fund"], cwd=monorepo_dir)
+
+        for endpoint in ("fn", "other"):
+            install_dir = os.path.join(monorepo_dir, "endpoints", endpoint)
+            from_npm = npm.resolve_dependency_closure(install_dir)
+            self.assertIsNotNone(from_npm, "npm ls must answer, or this test proves nothing")
+            from_lockfile = production_closure(monorepo_dir, install_dir)
+            self.assertIsNotNone(from_lockfile, "npm's hidden lockfile must be readable by the resolver")
+
+            normalise = {os.path.normcase(os.path.realpath(p)) for p in from_npm}
+            self.assertEqual(
+                {os.path.normcase(os.path.realpath(p)) for p in from_lockfile},
+                normalise,
+                f"the two closure sources disagree for endpoints/{endpoint}",
+            )
+
+    def test_a_manifest_the_project_lockfile_has_not_recorded_still_reaches_the_artifacts(self):
+        # `npm install --no-save` installs a newly declared dependency without writing
+        # package-lock.json, so a closure read from that file would drop it
+        monorepo_dir = os.path.join(self.temp_testdata_dir, "workspaces-monorepo")
+        source_dir = os.path.join(monorepo_dir, "endpoints", "fn")
+        npm = SubprocessNpm(OSUtils())
+        npm.run(["install", "--silent", "--no-audit", "--no-fund"], cwd=monorepo_dir)
+
+        # declare a dependency the lockfile knows nothing about, exactly as editing package.json does
+        manifest_path = os.path.join(source_dir, "package.json")
+        with open(manifest_path) as manifest:
+            manifest_json = json.load(manifest)
+        manifest_json["dependencies"]["ms"] = "^2.1.3"
+        with open(manifest_path, "w") as manifest:
+            json.dump(manifest_json, manifest)
+        with open(os.path.join(monorepo_dir, "package-lock.json")) as lockfile:
+            self.assertNotIn("ms", json.load(lockfile)["packages"]["endpoints/fn"].get("dependencies", {}))
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            manifest_path,
+            # one runtime: this pins the closure source, which no runtime varies
+            runtime=NODEJS_RUNTIMES[-1],
+            build_in_source=True,
+            experimental_flags=[EXPERIMENTAL_FLAG_NODEJS_MONOREPO],
+        )
+
+        self.assertIn("ms", set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))))
+
     @parameterized.expand(SUPPORTED_RUNTIMES)
     def test_build_in_source_with_removed_dependencies(self, runtime):
         # run a build with default requirements and confirm dependencies are downloaded

tests/integration/workflows/nodejs_npm/testdata/workspaces-monorepo/packages/shared/package.json

@@ -3,5 +3,8 @@
   "version": "1.0.0",
   "license": "APACHE2.0",
   "main": "index.js",
-  "files": ["index.js"]
+  "files": ["index.js"],
+  "peerDependencies": {
+    "fake-http-request": "^1.4.0"
+  }
 }

tests/unit/workflows/nodejs_npm/test_actions.py

@@ -18,6 +18,7 @@
     NodejsNpmCIAction,
     NodejsNpmTestAction,
 )
+from aws_lambda_builders.workflows.nodejs_npm.lockfile_closure import hidden_lockfile_path
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError
 from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils
 
@@ -312,6 +313,16 @@ def _action(self):
             osutils=self.osutils,
         )
 
+    def _hidden_lockfile(self, version, packages):
+        # the only lockfile the action reads, and it must not be older than the tree
+        path = hidden_lockfile_path(self.root)
+        os.makedirs(os.path.dirname(path), exist_ok=True)
+        with open(path, "w") as handle:
+            json.dump({"lockfileVersion": version, "packages": packages}, handle)
+        tree = os.path.getmtime(os.path.join(self.root, "node_modules"))
+        os.utime(path, (tree + 1, tree + 1))
+        return path
+
     def _linked(self):
         node_modules = os.path.join(self.artifacts_dir, "node_modules")
         found = set()
@@ -322,6 +333,47 @@ def _linked(self):
                 found.add(entry)
         return found
 
+    def test_a_usable_hidden_lockfile_answers_the_closure_without_running_npm(self):
+        # the whole point of reading the lockfile: no npm process per function
+        self._package("endpoints/fn", "@mono/fn")
+        self._package("node_modules/lodash", "lodash")
+        self._hidden_lockfile(
+            3,
+            {
+                "": {"workspaces": ["endpoints/fn"]},
+                "endpoints/fn": {"dependencies": {"lodash": "^4.0.0"}},
+                "node_modules/lodash": {"version": "4.17.21"},
+            },
+        )
+
+        self._action().execute()
+
+        self.assertEqual(self._linked(), {"lodash"})
+        self.subprocess_npm.resolve_dependency_closure.assert_not_called()
+
+    def test_a_hidden_lockfile_that_cannot_answer_falls_back_to_npm(self):
+        # version 1 carries no per-path map, so the fallback is the only thing that can produce a closure
+        self._package("endpoints/fn", "@mono/fn")
+        mine = self._package("node_modules/lodash", "lodash")
+        self._hidden_lockfile(1, {})
+        self.subprocess_npm.resolve_dependency_closure.return_value = [self.root, self.install_dir, mine]
+
+        self._action().execute()
+
+        self.assertEqual(self._linked(), {"lodash"})
+        self.subprocess_npm.resolve_dependency_closure.assert_called_once_with(self.install_dir)
+
+    def test_no_hidden_lockfile_falls_back_to_npm(self):
+        # the pre-npm-7 and other-package-manager case: every existing closure test runs this path
+        self._package("endpoints/fn", "@mono/fn")
+        mine = self._package("node_modules/lodash", "lodash")
+        self.subprocess_npm.resolve_dependency_closure.return_value = [self.root, self.install_dir, mine]
+
+        self._action().execute()
+
+        self.assertEqual(self._linked(), {"lodash"})
+        self.subprocess_npm.resolve_dependency_closure.assert_called_once_with(self.install_dir)
+
     def test_links_only_this_function_s_dependencies(self):
         self._package("endpoints/fn", "@mono/fn")
         mine = self._package("node_modules/lodash", "lodash")