fix: include lock files in dependency hash for cache invalidation

[dcabib]

Summary

Fixes #8242

When using sam build --cached, changes to lock files (package-lock.json, Gemfile.lock, etc.) were not triggering cache invalidation. This caused the build to skip dependency installation even when lock files specified different dependency versions, leaving functions with outdated or vulnerable dependencies.

Changes

Added a mapping of dependency managers to their lock file names in DependencyHashGenerator. When a lock file exists, the cache hash now includes both the manifest and lock file, ensuring proper cache invalidation when lock files change.

Supported lock files:

• npm: package-lock.json
• npm-esbuild: package-lock.json
• bundler: Gemfile.lock
• gradle: gradle.lockfile
• cli-package (dotnet): packages.lock.json
• modules (go): go.sum
• cargo (rust): Cargo.lock
• uv (python): uv.lock
• poetry (python): poetry.lock

Implementation Details

The fix follows the approach suggested by @seshubaws to add a small mapping in DependencyHashGenerator rather than modifying the CONFIG namedtuple, keeping the change isolated and minimizing impact.

The implementation:

• Checks if the dependency manager has a corresponding lock file
• If the lock file exists, combines both manifest and lock file hashes
• Returns a single combined hash for cache validation
• Is backward compatible - lock files are optional

Testing

• Manual testing with Node.js project confirmed cache invalidation works when package-lock.json changes
• Existing unit tests pass (4/4)
• Cache still works correctly when no changes are made

Test plan

• Run existing unit tests: pytest tests/unit/lib/build_module/test_dependency_hash_generator.py
• Test with Node.js project: modify package-lock.json and verify sam build --cached invalidates cache
• Test with Ruby project: modify Gemfile.lock and verify cache invalidation
• Verify cache still works when no changes are made

[seshubaws]

do we not need to add yarn.lock and pnpm-lock.yaml?

[seshubaws]

why are we not wanting to add the config to the hash if there is a different manifest path? Can the lock file not still be resolved relative to the override path's directory?

[seshubaws]

Could you add tests for if there is a lock file present and not present, and if there is an unknown dep manager

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 56510b1..0d47042
Files: 1
Comments: 4

[aws-sam-tooling-bot]

[BUG] Setting config = None in the manifest_path_override branch guarantees the lock-file logic below (if config and config.dependency_manager in LOCK_FILE_MAPPING) is skipped entirely. This was raised by @seshubaws and has not been addressed: the lock file can still be resolved relative to self._code_dir even when the manifest path is overridden, and skipping it defeats the whole point of the PR for users with overrides.

There are two further consequences of this design:

• PYTHON_UV_CONFIG in samcli/lib/build/workflows.py has manifest_name=None, so uv projects can only produce a hash via manifest_path_override. Combined with this branch, the new "uv": "uv.lock" mapping is unreachable.
• There is no poetry entry in ALL_CONFIGS, so config.dependency_manager is never "poetry" and "poetry": "poetry.lock" is also unreachable.

Consider resolving the workflow config (or at least the dependency manager) even when an override is present, e.g.:

if self._manifest_path_override:
   manifest_file = self._manifest_path_override
else:
   manifest_file = None
config = get_workflow_config(self._runtime, self._code_dir, self._base_dir)
if manifest_file is None:
   manifest_file = config.manifest_name

so the lock file is honored in every case where a dependency manager is known.

[aws-sam-tooling-bot]

[BUG] LOCK_FILE_MAPPING only maps npm/npm-esbuild to package-lock.json, but SAM classifies every Node.js project under dependency_manager="npm" regardless of whether the user actually uses npm, yarn, or pnpm (there is no separate yarn/pnpm workflow in samcli/lib/build/workflows.py). A project with a yarn.lock or pnpm-lock.yaml and no package-lock.json will not get cache invalidation when the lock file changes — exactly the bug this PR is trying to fix. @seshubaws raised this and it has not been addressed.

Since only one lock file name is stored per manager, consider either supporting a list of candidate lock files (checking each and hashing whichever exists) or falling back through the known Node lock file names. For example:

LOCK_FILE_MAPPING = {
   "npm": ["package-lock.json", "yarn.lock", "pnpm-lock.yaml"],
   "npm-esbuild": ["package-lock.json", "yarn.lock", "pnpm-lock.yaml"],
   ...
}

and hash whichever candidate exists.

[aws-sam-tooling-bot]

[BUG] hasher = self._hash_generator() if self._hash_generator else hashlib.md5() treats _hash_generator as a factory/class, but everywhere else in the codebase it is an already-constructed hasher instance:

• The class docstring documents hash_generator as hashlib.md5()/hashlib.sha256() (i.e. instances).
• file_checksum in samcli/lib/utils/hash.py calls hash_generator.update(buf) directly on the value, confirming it is an instance.

Hash objects returned by hashlib.md5() are not callable, so if any caller ever passes a non-None hash_generator, this line raises TypeError: '_hashlib.HASH' object is not callable.

There is a related latent bug on lines 90/99: file_checksum does not reset or copy the hasher it receives; it calls update() on it in place. So when self._hash_generator is non-None, the second file_checksum call inherits the manifest state and lock_file_hash ends up being a hash of manifest+lock rather than lock alone.

Suggested fix — always construct fresh hashers locally (and use the codebase's _get_md5 helper for consistency):

manifest_hash = file_checksum(str(manifest_path), hash_generator=self._hash_generator)

if config and config.dependency_manager in LOCK_FILE_MAPPING:
   lock_file_path = pathlib.Path(self._code_dir, LOCK_FILE_MAPPING[config.dependency_manager]).resolve()
   if lock_file_path.is_file():
       lock_file_hash = file_checksum(str(lock_file_path))  # fresh hasher
       hasher = hashlib.md5(usedforsecurity=False)
       hasher.update(f"{manifest_hash}:{lock_file_hash}".encode("utf-8"))
       return hasher.hexdigest()

[aws-sam-tooling-bot]

[GENERAL] @seshubaws explicitly asked for unit tests covering "if there is a lock file present and not present, and if there is an unknown dep manager." The diff does not modify tests/unit/lib/build_module/test_dependency_hash_generator.py, and the PR's own test-plan checkbox for running/adding unit tests is unchecked.

Given the number of code paths introduced (lock file present, lock file absent, dep manager not in LOCK_FILE_MAPPING, manifest_path_override with and without lock file, and the combined-hash construction), regressions here are easy to miss. Please add tests that:

• Exercise the case where the manifest exists but no lock file exists (should return the manifest hash unchanged, preserving previous caches).
• Exercise the case where both manifest and lock file exist (verify a distinct combined hash and that both files are read).
• Exercise a runtime whose dependency_manager is not in LOCK_FILE_MAPPING (e.g. maven) — no lock lookup should occur.
• Exercise the manifest_path_override path (see comment Bundling a new version of GoFormation. #1 — behavior here should be defined and tested).