Skip to content

perf(python): symlink layer dependencies instead of copying them - #932

Open
bnusunny wants to merge 4 commits into
aws:developfrom
bnusunny:perf/link-layer-dependencies
Open

bnusunny wants to merge 4 commits into
aws:developfrom
bnusunny:perf/link-layer-dependencies

fix: clear a leftover destination link before the copy fallback

d0f5f6a
Select commit
Loading
Failed to load commit list.
aws-sam-tooling-bot / lambda-tooling pr reviewer failed Sep 28, 2026 in 8m 34s

2 warnings found

Code Review Results

Reviewed: 587257c..d0f5f6a
Files: 6
Comments: 2


Comments on lines outside the diff:

[aws_lambda_builders/actions.py:143] [BUG] execute() iterates over source_files only, so destination entries that no longer exist in the source are never reconciled. Switching layers from copying to linking changes the consequence of that from benign to fatal.

Concretely, for the python_pip layer path the workflow is CleanUpAction(dependencies_dir) → pip install → LinkSourceAction(dependencies_dir, artifacts_dir) → CopySourceAction(source_dir, artifacts_dir). CleanUpAction is only ever applied to dependencies_dir — nothing in the workflow cleans artifacts_dir, and the PR's own test_replaces_a_real_directory_left_by_an_earlier_copying_build is premised on that directory surviving into the next build. So if a package is dropped from requirements.txt:

  • before this PR: artifacts_dir/requests/ is a stale real directory — wrong content, but it still packages and runs.
  • after this PR: artifacts_dir/requests is a symlink into a target that CleanUpAction just deleted and pip did not recreate, and LinkSourceAction never visits the name because it is absent from source_files. It stays dangling.

A dangling symlink is not a directory, so os.walk yields it in the files list and both zipfile.ZipFile.write and tarfile.add(..., dereference=True) raise FileNotFoundError on it. The dereferencing tarball this PR relies on for sam local is exactly one of those consumers, so the failure lands on the path the PR is optimizing.

Reconciling the destination after linking keeps this narrow enough to be safe for the nodejs_npm_esbuild consumer, which links into a scratch_dir that already holds copied source — only links that point into self._source_dir and no longer resolve are removed:

for source_file in source_files:
            ...

        # A dependency dropped from the manifest is absent from source_files, so the loop above
        # never visits its stale link. Left in place it dangles, and packing the artifacts
        # (tar with dereference, or zip) fails on a broken link rather than skipping it.
        for stale in set(os.listdir(self._dest_dir)) - source_files:
            stale_path = Path(self._dest_dir, stale)
            if stale_path.is_symlink() and not stale_path.exists():
                if _is_within_directory(self._source_dir, os.path.realpath(stale_path)):
                    LOG.debug("Removing dangling symlink %s left by an earlier build", stale_path)
                    os.remove(stale_path)

_is_within_directory already exists in aws_lambda_builders/utils.py. Worth a test alongside test_replaces_a_dangling_symlink: link two dependencies, remove one from the source directory, re-run, and assert the destination no longer contains a broken link.