From b559fdf02cbe3c8b0691b803571b7a05b02ba8a8 Mon Sep 17 00:00:00 2001 From: dcabib Date: Mon, 9 Feb 2026 12:29:31 -0300 Subject: [PATCH 1/2] fix(build): share dependencies folder for functions with same manifest (#6732) --- samcli/lib/build/build_graph.py | 34 +++++++- samcli/lib/build/build_strategy.py | 19 ++++- .../unit/lib/build_module/test_build_graph.py | 84 +++++++++++++++++++ .../lib/build_module/test_build_strategy.py | 52 ++++++++++++ 4 files changed, 184 insertions(+), 5 deletions(-) diff --git a/samcli/lib/build/build_graph.py b/samcli/lib/build/build_graph.py index c31ee03943..7699a4c55a 100644 --- a/samcli/lib/build/build_graph.py +++ b/samcli/lib/build/build_graph.py @@ -23,6 +23,7 @@ SAM_RESOURCE_ID_KEY, ) from samcli.lib.utils.architecture import X86_64 +from samcli.lib.utils.hash import str_checksum from samcli.lib.utils.packagetype import ZIP LOG = logging.getLogger(__name__) @@ -501,7 +502,22 @@ def __init__( @property def dependencies_dir(self) -> str: - return str(os.path.join(DEFAULT_DEPENDENCIES_DIR, self.uuid)) + """ + Returns the dependencies directory path. + Uses a deterministic hash-based key when manifest_hash is available, + allowing functions with the same manifest to share dependencies. + Falls back to UUID for backward compatibility. + """ + deps_key = self._get_dependencies_key() if self.manifest_hash else self.uuid + return str(os.path.join(DEFAULT_DEPENDENCIES_DIR, deps_key)) + + @abstractmethod + def _get_dependencies_key(self) -> str: + """ + Returns a deterministic key for the dependencies directory based on + manifest_hash, runtime/build_method, and architecture. + This allows functions/layers with identical manifests to share dependencies. + """ @property def env_vars(self) -> Dict: @@ -537,6 +553,14 @@ def __init__( # this and move "layer" out of LayerBuildDefinition to take advantage of type check. self.layer: LayerVersion = None # type: ignore + def _get_dependencies_key(self) -> str: + """ + Returns a deterministic key for the dependencies directory based on + manifest_hash, build_method, and architecture. + """ + key_string = f"{self.manifest_hash}:{self.build_method or ''}:{self.architecture}" + return str_checksum(key_string)[:16] + def get_resource_full_paths(self) -> str: if not self.layer: LOG.debug("LayerBuildDefinition with uuid (%s) doesn't have a layer assigned to it", self.uuid) @@ -609,6 +633,14 @@ def __init__( self.functions: List[Function] = [] + def _get_dependencies_key(self) -> str: + """ + Returns a deterministic key for the dependencies directory based on + manifest_hash, runtime, and architecture. + """ + key_string = f"{self.manifest_hash}:{self.runtime or ''}:{self.architecture}" + return str_checksum(key_string)[:16] + def add_function(self, function: Function) -> None: self.functions.append(function) diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 87b412b41c..3c8cc17440 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -504,9 +504,12 @@ def _check_whether_manifest_is_changed( is_dependencies_dir_missing = True if manifest_hash: is_manifest_changed = manifest_hash != build_definition.manifest_hash + # FIX for issue #6732: Update manifest_hash BEFORE checking dependencies_dir + # so the hash-based deterministic path is used correctly + if is_manifest_changed: + build_definition.manifest_hash = manifest_hash is_dependencies_dir_missing = not os.path.exists(build_definition.dependencies_dir) if is_manifest_changed or is_dependencies_dir_missing: - build_definition.manifest_hash = manifest_hash LOG.info( "Manifest file is changed (new hash: %s) or dependency folder (%s) is missing for (%s), " "downloading dependencies and copying/building source", @@ -525,10 +528,18 @@ def _check_whether_manifest_is_changed( def _clean_redundant_dependencies(self) -> None: """ Update build definitions with possible new manifest hash information and clean the redundant dependencies folder + FIX for issue #6732: Use hash-based folder names instead of UUIDs """ - uuids = {bd.uuid for bd in self._build_graph.get_function_build_definitions()} - uuids.update({ld.uuid for ld in self._build_graph.get_layer_build_definitions()}) - clean_redundant_folders(DEFAULT_DEPENDENCIES_DIR, uuids) + deps_keys: Set[str] = set() + for bd in self._build_graph.get_function_build_definitions(): + deps_dir = bd.dependencies_dir + if deps_dir and deps_dir != DEFAULT_DEPENDENCIES_DIR: + deps_keys.add(pathlib.Path(deps_dir).name) + for ld in self._build_graph.get_layer_build_definitions(): + deps_dir = ld.dependencies_dir + if deps_dir and deps_dir != DEFAULT_DEPENDENCIES_DIR: + deps_keys.add(pathlib.Path(deps_dir).name) + clean_redundant_folders(DEFAULT_DEPENDENCIES_DIR, deps_keys) class CachedOrIncrementalBuildStrategyWrapper(BuildStrategy): diff --git a/tests/unit/lib/build_module/test_build_graph.py b/tests/unit/lib/build_module/test_build_graph.py index 898e7a965d..ea16b8d3b3 100644 --- a/tests/unit/lib/build_module/test_build_graph.py +++ b/tests/unit/lib/build_module/test_build_graph.py @@ -1031,3 +1031,87 @@ def test_go_runtime_different_handlers_are_not_equal(self): self.assertEqual(len(build_definitions), 2) self.assertEqual(len(build_definition1.functions), 1) self.assertEqual(len(build_definition2.functions), 1) + + def test_dependencies_dir_uses_uuid_when_no_manifest_hash(self): + """Test that dependencies_dir falls back to UUID when manifest_hash is empty""" + build_definition = FunctionBuildDefinition( + "runtime", "codeuri", None, ZIP, X86_64, {}, "handler", "", "" # empty manifest_hash + ) + # Should use UUID when no manifest_hash + self.assertIn(build_definition.uuid, build_definition.dependencies_dir) + + def test_dependencies_dir_uses_hash_key_when_manifest_hash_present(self): + """Test that dependencies_dir uses deterministic hash when manifest_hash is set""" + build_definition = FunctionBuildDefinition( + "nodejs18.x", "codeuri", None, ZIP, X86_64, {}, "handler", "", "fa8267680f5dd92c2b4679c19c34d739" + ) + # Should NOT use UUID when manifest_hash is present + self.assertNotIn(build_definition.uuid, build_definition.dependencies_dir) + # Should be deterministic + self.assertIn("deps", build_definition.dependencies_dir) + + def test_dependencies_dir_is_deterministic_for_same_manifest_runtime_arch(self): + """Test that two definitions with same manifest+runtime+arch get same dependencies_dir""" + manifest_hash = "fa8267680f5dd92c2b4679c19c34d739" + runtime = "nodejs18.x" + arch = X86_64 + + build_definition1 = FunctionBuildDefinition( + runtime, "codeuri1", None, ZIP, arch, {}, "handler1", "", manifest_hash + ) + build_definition2 = FunctionBuildDefinition( + runtime, "codeuri2", None, ZIP, arch, {}, "handler2", "", manifest_hash + ) + + # Both should have the same dependencies_dir (shared deps) + self.assertEqual(build_definition1.dependencies_dir, build_definition2.dependencies_dir) + + def test_dependencies_dir_differs_for_different_runtime(self): + """Test that definitions with different runtime get different dependencies_dir""" + manifest_hash = "fa8267680f5dd92c2b4679c19c34d739" + + build_definition1 = FunctionBuildDefinition( + "nodejs18.x", "codeuri", None, ZIP, X86_64, {}, "handler", "", manifest_hash + ) + build_definition2 = FunctionBuildDefinition( + "python3.9", "codeuri", None, ZIP, X86_64, {}, "handler", "", manifest_hash + ) + + self.assertNotEqual(build_definition1.dependencies_dir, build_definition2.dependencies_dir) + + def test_dependencies_dir_differs_for_different_architecture(self): + """Test that definitions with different architecture get different dependencies_dir""" + manifest_hash = "fa8267680f5dd92c2b4679c19c34d739" + + build_definition1 = FunctionBuildDefinition( + "nodejs18.x", "codeuri", None, ZIP, X86_64, {}, "handler", "", manifest_hash + ) + build_definition2 = FunctionBuildDefinition( + "nodejs18.x", "codeuri", None, ZIP, ARM64, {}, "handler", "", manifest_hash + ) + + self.assertNotEqual(build_definition1.dependencies_dir, build_definition2.dependencies_dir) + + def test_layer_dependencies_dir_uses_hash_key_when_manifest_hash_present(self): + """Test that LayerBuildDefinition dependencies_dir uses deterministic hash""" + layer_definition = LayerBuildDefinition( + "layer1", "codeuri", "nodejs18.x", ["nodejs18.x"], X86_64, "", "fa8267680f5dd92c2b4679c19c34d739" + ) + # Should NOT use UUID when manifest_hash is present + self.assertNotIn(layer_definition.uuid, layer_definition.dependencies_dir) + + def test_layer_dependencies_dir_is_deterministic_for_same_manifest_method_arch(self): + """Test that two layer definitions with same manifest+build_method+arch get same dependencies_dir""" + manifest_hash = "fa8267680f5dd92c2b4679c19c34d739" + build_method = "nodejs18.x" + arch = X86_64 + + layer_definition1 = LayerBuildDefinition( + "layer1", "codeuri1", build_method, ["nodejs18.x"], arch, "", manifest_hash + ) + layer_definition2 = LayerBuildDefinition( + "layer2", "codeuri2", build_method, ["nodejs18.x"], arch, "", manifest_hash + ) + + # Both should have the same dependencies_dir (shared deps) + self.assertEqual(layer_definition1.dependencies_dir, layer_definition2.dependencies_dir) diff --git a/tests/unit/lib/build_module/test_build_strategy.py b/tests/unit/lib/build_module/test_build_strategy.py index c0ecedb125..c44b2a31b7 100644 --- a/tests/unit/lib/build_module/test_build_strategy.py +++ b/tests/unit/lib/build_module/test_build_strategy.py @@ -720,6 +720,58 @@ def test_assert_incremental_build_layer(self, patched_manifest_hash, patched_os, ) +class TestIncrementalBuildStrategyCleanRedundantDependencies(TestCase): + """Tests for _clean_redundant_dependencies using hash-based folder names""" + + @patch("samcli.lib.build.build_strategy.clean_redundant_folders") + def test_clean_redundant_dependencies_uses_hash_based_keys(self, mock_clean_folders): + """Test that _clean_redundant_dependencies collects hash-based folder names""" + mock_build_graph = Mock() + + # Create mock function build definitions with manifest_hash set + mock_func_bd1 = Mock() + mock_func_bd1.dependencies_dir = ".aws-sam/deps/abc123def456" + mock_func_bd2 = Mock() + mock_func_bd2.dependencies_dir = ".aws-sam/deps/abc123def456" # Same as bd1 (shared) + + # Create mock layer build definition with manifest_hash set + mock_layer_bd = Mock() + mock_layer_bd.dependencies_dir = ".aws-sam/deps/xyz789layer00" + + mock_build_graph.get_function_build_definitions.return_value = [mock_func_bd1, mock_func_bd2] + mock_build_graph.get_layer_build_definitions.return_value = [mock_layer_bd] + + build_strategy = IncrementalBuildStrategy(mock_build_graph, Mock(), "base_dir", None) + + build_strategy._clean_redundant_dependencies() + + # Should be called with the hash-based folder names, not UUIDs + mock_clean_folders.assert_called_once() + call_args = mock_clean_folders.call_args + deps_keys = call_args[0][1] + + # Should have 2 unique keys (abc123def456 and xyz789layer00) + self.assertEqual(len(deps_keys), 2) + self.assertIn("abc123def456", deps_keys) + self.assertIn("xyz789layer00", deps_keys) + + @patch("samcli.lib.build.build_strategy.clean_redundant_folders") + def test_clean_redundant_dependencies_handles_empty_definitions(self, mock_clean_folders): + """Test that _clean_redundant_dependencies handles empty build definitions""" + mock_build_graph = Mock() + mock_build_graph.get_function_build_definitions.return_value = [] + mock_build_graph.get_layer_build_definitions.return_value = [] + + build_strategy = IncrementalBuildStrategy(mock_build_graph, Mock(), "base_dir", None) + + build_strategy._clean_redundant_dependencies() + + mock_clean_folders.assert_called_once() + call_args = mock_clean_folders.call_args + deps_keys = call_args[0][1] + self.assertEqual(len(deps_keys), 0) + + @patch("samcli.lib.build.build_graph.BuildGraph._write") @patch("samcli.lib.build.build_graph.BuildGraph._read") class TestCachedOrIncrementalBuildStrategyWrapper(TestCase): From f8824be7d3a691d3a1aff559204bb3a7d70ba0da Mon Sep 17 00:00:00 2001 From: dcabib Date: Wed, 11 Feb 2026 10:28:00 -0300 Subject: [PATCH 2/2] fix(build): add -L and --fail flags to curl for zlib download This fixes the PyInstaller build failure where zlib download might fail due to HTTP redirects or server errors not being handled properly. --- installer/pyinstaller/build-linux.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/installer/pyinstaller/build-linux.sh b/installer/pyinstaller/build-linux.sh index 9a992a9821..6d11c4c5e2 100755 --- a/installer/pyinstaller/build-linux.sh +++ b/installer/pyinstaller/build-linux.sh @@ -58,8 +58,8 @@ ln -sf /opt/openssl/lib64 /opt/openssl/lib cd ../../ echo "Building zlib" -curl https://www.zlib.net/zlib-${zlib_version}.tar.gz --output zlib.tar.gz -tar xvf zlib.tar.gz +curl -L --fail https://www.zlib.net/zlib-${zlib_version}.tar.gz --output zlib.tar.gz +tar xzf zlib.tar.gz cd zlib-${zlib_version} ./configure && make -j8 && make -j8 install cd ../