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

[bnusunny]

Issue #, if available: aws/aws-sam-cli#4828

Description of changes

sam build --cached re-copies every Python dependency file on every build. For a project with a large dependency layer that is most of the build time — measured on a 185 MB / ~6,200-file layer, a warm no-change rebuild spends 0.96 s of 1.88 s re-copying the layer.

This workflow can already symlink dependencies instead of copying them, but the branch is dead code: workflow.py:119 reads if False and is_experimental_build_improvements_enabled(...). #391 disabled it in 2022 because symlinked dependencies broke sam local.

That reason holds for functions but not for layers, because the two reach the local invoke container by different routes:

• A function's artifacts are bind-mounted at /var/task, so a symlink pointing outside the mount dangles inside the container. sam local invoke --mount-symlinks opts into mounting the targets, but sam local start-api and start-lambda have no such option — linking function dependencies really would break them.
• A layer's artifacts are packed into a tarball that becomes the local invoke image, and the tar dereferences symlinks. By the time the container sees the layer they are ordinary files.

So this gates the existing LinkSourceAction on is_building_layer, which BaseWorkflow already carries and which java_gradle / java_maven already branch on. Functions keep copying, and the layer path stays behind SAM_CLI_BETA_BUILD_PERFORMANCE. With SAM CLI 1.166.2 the warm rebuild goes 1.88 s -> 0.93 s and the layer build directory 184.5 MB -> ~20 KB.

LinkSourceAction also had no unit tests and two latent bugs that this second consumer hits: os.remove() raises IsADirectoryError on a real directory left behind by an earlier copying build, and a dangling symlink is not exists(), so it was left in place for os.symlink to fail on and silently fall back to a full copy. Both are fixed here.

Description of how you validated changes

Unit tests: tests/unit passes (861 tests). Coverage is 94%, meeting the --cov-fail-under 94 gate. ruff check and black --check are clean.

New tests cover the layer/function contrast on otherwise-identical inputs, and LinkSourceAction linking into an empty destination, replacing a real directory, replacing a dangling symlink, and being idempotent. Both changes are mutation-checked: reverting the is_building_layer gate fails 6 tests, and reverting the LinkSourceAction fix fails 2.

End to end with SAM CLI against a template with 10 python3.12 functions and one AWS::Serverless::LayerVersion:

• sam local invoke with no flags — pandas and cryptography import from /opt/python. Repeated after docker rmi of the built image, to rule out a stale image.
• sam local start-lambda — returns StatusCode 200, no FunctionError.
• A real sam deploy — the uploaded layer is CodeSize: 61936488, no zip entry is stored as a symlink, and the deployed function imports pandas from /opt/python in the Lambda runtime. The layer zip's inventory (6,158 entries, 61.9 MB) is identical to a copying build's zip of the same tree.
• Functions still get real copies (.aws-sam/build/<Fn>/requests/ is a directory, not a link), and plain sam build without --cached has no dependencies_dir, so it is unaffected.

Note for reviewers: the symlinks point at absolute paths under .aws-sam/deps/<uuid>, so CI that tars .aws-sam/build to hand to a later stage needs tar -h. That is part of why this stays behind the experimental flag.

Two pre-existing failures in this environment, unrelated to the change and reproduced on a clean origin/develop checkout: 13 errors in tests/functional/workflows/python_uv (uv not on PATH) and 6 failures in tests/functional/test_cli.py::TestCliWithHelloWorkflow.

Checklist

• Review the generative AI contribution guidelines

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 587257c..0c0df09
Files: 4
Comments: 3

---

Comments on lines outside the diff:

[aws_lambda_builders/actions.py:151] [BUG] The copy fallback described in the PR description does not actually work for file entries, and a Python dependencies directory always has them. create_symlink_or_copy falls back to copytree(source, destination) when os.symlink raises (the common case being WinError 1314, privilege not held), and copytree assumes source is a directory:

if not os.path.exists(destination):
   os.makedirs(destination)          # creates a directory named six.py
...
names = os.listdir(source)            # NotADirectoryError on a regular file

The copytree call is inside the except OSError handler, so the resulting NotADirectoryError is not caught and the build fails. Until now the only LinkSourceAction consumer was nodejs_npm_esbuild, whose dependencies dir contains just node_modules/, so the bug stayed dormant. A pip-installed target dir reliably has top-level files (six.py, typing_extensions.py, *.pth), so enabling this for Python layers turns "fall back to copying" into a hard failure on any host where symlink creation is not permitted.

Either handle files in create_symlink_or_copy (shutil.copy2 when Path(source).is_file()) or pick the copy primitive in LinkSourceAction based on source_path.is_dir(). A test that forces os.symlink to raise OSError and asserts a file dependency still lands as a file would lock this down — the new tests all assert is_symlink(), so none of them exercise the fallback.

[aws_lambda_builders/workflows/python_pip/workflow.py:127] [BUG] The final CopySourceAction(source_dir, artifacts_dir) queued below runs after the dependency links exist, and copytree recurses into a destination directory without checking whether it is a symlink:

if not os.path.exists(destination):   # True for a symlink to a directory -> skipped
   os.makedirs(destination)
...
elif os.path.isdir(new_source):
   copytree(new_source, new_destination)

So if a top-level name in source_dir collides with a top-level name in dependencies_dir, the source files are written through the symlink into the shared dependencies directory instead of into the artifacts directory. With copying, that merge stayed inside artifacts_dir and was discarded on the next build; with links it mutates the cache that --cached reuses across builds, and CleanUpAction only wipes it when download_dependencies is true, so the pollution survives and gets published into later builds.

The collision requires a matching name, so it is not the common case, but the blast radius is a corrupted shared cache outside the build directory. Worth either replacing a colliding link with a real copy or asserting the artifacts-dir boundary before the source copy runs.

[aws-sam-tooling-bot]

[BUG] os.remove() cannot delete a directory symlink on Windows — DeleteFileW fails with ERROR_ACCESS_DENIED for a reparse point that is a directory, which surfaces as PermissionError. Since is_symlink() is now the first branch, every rebuild of an already-linked dependency directory takes it, so the idempotent second build that test_is_idempotent covers on POSIX will fail on Windows for package directories (requests/, certifi/, …) whenever symlink creation succeeded on the first build (developer mode / elevated shell). The exception is not an ActionFailedError, so it propagates as a hard workflow failure rather than degrading to a copy.

The directory symlink must be removed with os.rmdir() on Windows (it unlinks the link, it does not recurse):

if destination_path.is_symlink():
   if sys.platform == "win32" and destination_path.is_dir():
       # os.remove() cannot delete a directory symlink/junction on Windows
       os.rmdir(destination_path)
   else:
       os.remove(destination_path)
elif destination_path.is_file():
   os.remove(destination_path)
elif destination_path.is_dir():
   shutil.rmtree(destination_path)
else:
   os.makedirs(destination_path.parent, exist_ok=True)

Note this also pre-dates the PR (the old exists() branch called os.remove on the same input), but this PR is what makes the path reachable for Python layer builds, and it is the branch being rewritten here.

[bnusunny]

Checked this against Windows rather than reasoning about it, and it does not reproduce: all four TestLinkSourceAction tests pass on windows-latest, including test_is_idempotent, which is exactly the scenario described here.

test_is_idempotent links somepkg (a real directory), then runs the action again. The second run takes the is_symlink() branch and calls os.remove() on a directory symlink. _assert_linked then asserts destination.is_symlink() is True for that entry, so a silent fallback to copying would fail the test too — the symlink really was created and really was removed.

From windows-latest / 3.10 / unit-functional on this head (job 108558335756):

tests/unit/test_actions.py::TestLinkSourceAction::test_is_idempotent PASSED
tests/unit/test_actions.py::TestLinkSourceAction::test_links_into_an_empty_destination PASSED
tests/unit/test_actions.py::TestLinkSourceAction::test_replaces_a_dangling_symlink PASSED
tests/unit/test_actions.py::TestLinkSourceAction::test_replaces_a_real_directory_left_by_an_earlier_copying_build PASSED

Same result on the 3.11, 3.12 and 3.13 unit-functional jobs, so it is not a single-version quirk.

The Win32 distinction you cite is real — DeleteFile's own docs say "To remove an empty directory, use the RemoveDirectory function" — but os.remove is not a thin wrapper over DeleteFileW, and empirically CPython handles the directory reparse point on all four supported versions. Adding a sys.platform == "win32" branch would therefore be an untested path guarding a condition that does not occur, so I would rather not carry it.

Happy to reconsider with a failing case on a Windows configuration the CI matrix does not cover.

Your other two comments were both real and are fixed — replies on those separately.

[bnusunny]

Re aws_lambda_builders/actions.py:151 — the copy fallback breaks on file entries. Confirmed and fixed.

Reproduced it exactly as you described, by forcing os.symlink to raise against a dependencies directory holding one package and one top-level file:

#2 fallback CRASHED: NotADirectoryError: [Errno 20] Not a directory: '.../deps/six.py'

So the fallback this PR advertises was a hard failure on any host where symlink creation is not permitted, and you are right that it stayed dormant because nodejs_npm_esbuild only ever had node_modules/ at the top level.

create_symlink_or_copy now picks the copy primitive from the source:

if os.path.isdir(source):
    copytree(source, destination)
else:
    os.makedirs(os.path.dirname(destination), exist_ok=True)
    shutil.copy2(source, destination)

After the fix, the same probe gives six.py copied as a FILE: True with its contents intact, and pkgdir copied as a DIR: True.

You were also right that none of my tests exercised the fallback, since they all assert is_symlink(). Added two that force OSError from os.symlink on a real filesystem — test_falls_back_to_copying_a_top_level_file and test_falls_back_to_copying_a_package_directory — so the file and directory shapes are both pinned.

One thing I found while adding them, worth flagging separately: test_must_copy_if_symlink_fails was defined twice in tests/unit/test_utils.py, so the second definition shadowed the first and the fallback assertion never ran. Renaming the second to test_must_not_copy_when_symlink_succeeds revived the first, which then failed — patching aws_lambda_builders.utils.Path wholesale makes the already-a-symlink branch truthy, so the function returned before reaching os.symlink. It now sets exists.return_value = False and asserts what its name claims.

[bnusunny]

Re aws_lambda_builders/workflows/python_pip/workflow.py:127 — the source copy writing through a dependency symlink. Confirmed and fixed. This was the most serious of the three, and it reproduced on the first try.

A dependency requests/ linked into the artifacts directory, then a source tree containing its own requests/my_helper.py copied over it:

#3 art/requests is symlink: True
#3 user file leaked into the shared deps cache: True -> .../deps/requests/my_helper.py

Exactly as you described: the file lands outside the artifacts directory, in the cache that later --cached builds reuse.

Fixed in copytree rather than in the workflow, because the hole is in the shared layer — CopySourceAction is not the only caller that can be handed a symlinked destination, and a fix in the Python workflow would leave nodejs_npm_esbuild exposed to the same thing. A new _materialize_symlinked_destination replaces a symlinked destination with a real copy of what it points at before anything is written into it:

if os.path.isdir(link_target):
    copytree(link_target, destination)
elif os.path.isfile(link_target):
    os.makedirs(os.path.dirname(destination), exist_ok=True)
    shutil.copy2(link_target, destination)

That keeps the semantics a copying build had — the collision merges inside the artifacts directory and is discarded on the next build — rather than just refusing the copy. After the fix:

#3 leaked into shared cache: False
#3 artifacts still have the dep:  REAL DEP
#3 artifacts have the user file: USER CODE
#3 colliding entry is now a real dir: True

Pinned by Test_copytree::test_does_not_write_through_a_symlinked_destination, which asserts all four of those: nothing in the dependencies directory, the destination is no longer a symlink, and both the dependency and the source file are readable in the artifacts directory.

I took the shared-layer route over "assert the artifacts-dir boundary" because the boundary check would turn a name collision into a build failure, and collisions were previously legal and harmless.

Full gates on the amended commit: ruff clean, black --check clean, 865 unit tests passing, 961 with functional. Coverage is unchanged against develop measured with identical exclusions (201 missed lines here versus 210 on the base).