diff --git a/samcli/lib/build/build_graph.py b/samcli/lib/build/build_graph.py index ceda957ffb..f2d328be02 100644 --- a/samcli/lib/build/build_graph.py +++ b/samcli/lib/build/build_graph.py @@ -3,6 +3,7 @@ """ import logging +from copy import deepcopy from pathlib import Path from typing import Tuple, List, Any, Optional, Dict, cast from uuid import uuid4 @@ -323,9 +324,14 @@ class AbstractBuildDefinition: Build definition holds information about each unique build """ - def __init__(self, source_md5: str) -> None: + def __init__(self, source_md5: str, env_vars: Optional[Dict] = None) -> None: self.uuid = str(uuid4()) self.source_md5 = source_md5 + self._env_vars = env_vars if env_vars else {} + + @property + def env_vars(self) -> Dict: + return deepcopy(self._env_vars) class LayerBuildDefinition(AbstractBuildDefinition): @@ -342,12 +348,11 @@ def __init__( source_md5: str = "", env_vars: Optional[Dict] = None, ): - super().__init__(source_md5) + super().__init__(source_md5, env_vars) self.name = name self.codeuri = codeuri self.build_method = build_method self.compatible_runtimes = compatible_runtimes - self.env_vars = env_vars if env_vars else {} # Note(xinhol): In our code, we assume "layer" is never None. We should refactor # this and move "layer" out of LayerBuildDefinition to take advantage of type check. self.layer: LayerVersion = None # type: ignore @@ -398,12 +403,11 @@ def __init__( source_md5: str = "", env_vars: Optional[Dict] = None, ) -> None: - super().__init__(source_md5) + super().__init__(source_md5, env_vars) self.runtime = runtime self.codeuri = codeuri self.packagetype = packagetype self.metadata = metadata if metadata else {} - self.env_vars = env_vars if env_vars else {} self.functions: List[Function] = [] def add_function(self, function: Function) -> None: diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 258101ba2d..ecded3a743 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -5,7 +5,6 @@ import pathlib import shutil from abc import abstractmethod, ABC -from copy import deepcopy from typing import Callable, Dict, List, Any, Optional, cast from samcli.commands.build.exceptions import MissingBuildMethodException @@ -115,10 +114,6 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini LOG.debug("Building to following folder %s", single_build_dir) - # we should create a copy and pass it down, otherwise additional env vars like LAMBDA_BUILDERS_LOG_LEVEL - # will make cache invalid all the time - container_env_vars = deepcopy(build_definition.env_vars) - # when a function is passed here, it is ZIP function, codeuri and runtime are not None result = self._build_function( build_definition.get_function_name(), @@ -128,7 +123,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini build_definition.get_handler_name(), single_build_dir, build_definition.metadata, - container_env_vars, + build_definition.env_vars, ) function_build_results[single_full_path] = result diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index f0980ad7ba..09860e0212 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -1349,35 +1349,50 @@ def test_cache_build(self, use_container, code_uri, function1_handler, function2 expected_messages, command_result, self._make_parameter_override_arg(overrides) ) + +@skipIf( + ((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE), + "Skip build tests on windows when running in CI unless overridden", +) +class TestRepeatedBuildHitsCache(BuildIntegBase): + # Use template containing both functions and layers + template = "layers-functions-template.yaml" + + @parameterized.expand([(True,), (False,)]) @skipIf(SKIP_DOCKER_TESTS, SKIP_DOCKER_MESSAGE) - def test_cached_build_with_env_vars(self): + def test_repeated_cached_build_hits_cache(self, use_container): """ Build 2 times to verify that second time hits the cached build """ - overrides = { - "FunctionCodeUri": "Python", - "Function1Handler": "main.first_function_handler", - "Function2Handler": "main.second_function_handler", - "FunctionRuntime": "python3.8", + + parameter_overrides = { + "LayerContentUri": "PyLayer", + "LayerBuildMethod": "python3.7", + "CodeUri": "Python", + "Handler": "main.handler", + "Runtime": "python3.7", + "LayerMakeContentUri": "PyLayerMake", } + cmdlist = self.get_command_list( - use_container=True, parameter_overrides=overrides, cached=True, container_env_var="FOO=BAR" + use_container=use_container, + parameter_overrides=parameter_overrides, + cached=True, + container_env_var="FOO=BAR" if use_container else None, ) + cache_invalid_output = "Cache is invalid, running build and copying resources to " + cache_valid_output = "Valid cache found, copying previously built resources from " + LOG.info("Running Command (cache should be invalid): %s", cmdlist) - command_result = run_command(cmdlist, cwd=self.working_dir) - self.assertTrue( - "Cache is invalid, running build and copying resources to function build definition" - in command_result.stderr.decode("utf-8") - ) + command_result = run_command(cmdlist, cwd=self.working_dir).stderr.decode("utf-8") + self.assertTrue(cache_invalid_output in command_result) + self.assertFalse(cache_valid_output in command_result) LOG.info("Re-Running Command (valid cache should exist): %s", cmdlist) - command_result_with_cache = run_command(cmdlist, cwd=self.working_dir) - - self.assertTrue( - "Valid cache found, copying previously built resources from function build definition" - in command_result_with_cache.stderr.decode("utf-8") - ) + command_result = run_command(cmdlist, cwd=self.working_dir).stderr.decode("utf-8") + self.assertFalse(cache_invalid_output in command_result) + self.assertTrue(cache_valid_output in command_result) @skipIf( diff --git a/tests/unit/lib/build_module/test_build_strategy.py b/tests/unit/lib/build_module/test_build_strategy.py index 1fae5b7962..f0e7ab3e7d 100644 --- a/tests/unit/lib/build_module/test_build_strategy.py +++ b/tests/unit/lib/build_module/test_build_strategy.py @@ -1,4 +1,3 @@ -from copy import deepcopy from unittest import TestCase from unittest.mock import Mock, patch, MagicMock, call, ANY @@ -223,11 +222,7 @@ def test_build_single_function_definition_image_functions_with_same_metadata(sel # since they have the same metadata, they are put into the same build_definition. build_definition.functions = [function1, function2] - with patch("samcli.lib.build.build_strategy.deepcopy", wraps=deepcopy) as patched_deepcopy: - result = default_build_strategy.build_single_function_definition(build_definition) - - patched_deepcopy.assert_called_with(build_definition.env_vars) - + result = default_build_strategy.build_single_function_definition(build_definition) # both of the function name should show up in results self.assertEqual(result, {"Function": built_image, "Function2": built_image})