Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion aws_lambda_builders/workflows/python_uv/packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ def _build_from_lock_file(
"--no-emit-project", # Don't include the project itself, only dependencies
"--no-hashes", # Skip hashes for cleaner output (optional)
"--no-default-groups", # Exclude PEP 735 default groups (e.g. dev/test) from Lambda zips
"--no-editable", # Export editable dependencies as non-editable
"--output-file",
temp_requirements,
# We want to specify the version because `uv export` might default to using a different one
Expand All @@ -344,6 +345,15 @@ def _build_from_lock_file(
if rc != 0:
raise LockFileError(reason=f"Failed to export lock file: {stderr}")

# Get the workspace root (or project directory if no workspace is used)
# For packages in the workspace, exported paths are relative to the workspace root,
# regardless of where in the workspace uv export is called
workspace_args = ["workspace", "dir"]
rc, stdout, stderr = self._uv_runner._uv.run_uv_command(workspace_args, cwd=project_dir)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[GENERAL] This change introduces a second run_uv_command invocation inside _build_from_lock_file (for uv workspace dir), but the existing unit tests were not updated and will fail:

  • tests/unit/workflows/python_uv/test_packager.py::test_build_from_lock_file asserts self.mock_uv_runner._uv.run_uv_command.assert_called_once(), which now sees two calls (export + workspace dir).
  • tests/unit/workflows/python_uv/test_packager.py::test_build_dependencies_pyproject_with_uv_lock has the same assertion and will also break.

In addition, those tests set run_uv_command.return_value = (0, b"", b"") (bytes). The new line workspace_dir = stdout.strip() will yield b"" under this mock, which is then passed as cwd to install_requirements. Since install_requirements is mocked in the unit tests this does not fail there, but tests should be updated to use strings (matching production OSUtils.run_subprocess, which returns text=True output) and to explicitly cover the new uv workspace dir path — including the failure branch (rc != 0 raising LockFileError) and verifying that install_requirements is invoked with cwd=workspace_dir rather than project_dir. Without such coverage, regressions in the workspace‑resolution logic (which is the core of this fix) will go undetected.

Example update for test_build_from_lock_file:

def test_build_from_lock_file(self):
   # First call: export (returns empty stdout). Second call: workspace dir.
   self.mock_uv_runner._uv.run_uv_command.side_effect = [
       (0, "", ""),
       (0, "/workspace/root", ""),
   ]

   self.builder._build_from_lock_file(
       lock_path="/workspace/root/sam-app/uv.lock",
       target_dir="/target",
       scratch_dir="/scratch",
       python_version="3.9",
       architecture=X86_64,
       config=UvConfig(),
   )

   self.assertEqual(self.mock_uv_runner._uv.run_uv_command.call_count, 2)
   # export runs from project_dir, install runs from the workspace root
 , installkwargs = self.mock_uv_runner.install_requirements.call_args
   self.assertEqual(install_kwargs["cwd"], "/workspace/root")

if rc != 0:
raise LockFileError(reason=f"Failed to get workspace root: {stderr}")
workspace_dir = stdout.strip()

# Install with platform targeting
self._uv_runner.install_requirements(
requirements_path=temp_requirements,
Expand All @@ -352,7 +362,7 @@ def _build_from_lock_file(
config=config,
python_version=python_version,
platform="linux",
cwd=project_dir,
cwd=workspace_dir,
architecture=architecture,
)
except LockFileError:
Expand Down
52 changes: 33 additions & 19 deletions tests/unit/workflows/python_uv/test_packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,22 +174,30 @@ def test_extract_python_version(self):
self.assertIn("Runtime is required", str(context.exception))

def test_build_from_lock_file(self):
# Mock the uv command for export
self.mock_uv_runner._uv.run_uv_command.return_value = (0, b"", b"")

self.builder._build_from_lock_file(
lock_path="/path/to/uv.lock",
target_dir="/target",
scratch_dir="/scratch",
python_version="3.9",
architecture=X86_64,
config=UvConfig(),
)
# Mock the uv commands
self.mock_uv_runner._uv.run_uv_command.side_effect = [
(0, b"", b""), # export
(0, "/workspace\n", b""), # workspace dir
]

with patch("os.path.dirname", return_value="/path/to"):
self.builder._build_from_lock_file(
lock_path="/path/to/uv.lock",
target_dir="/target",
scratch_dir="/scratch",
python_version="3.9",
architecture=X86_64,
config=UvConfig(),
)

# Should call export then install_requirements
self.mock_uv_runner._uv.run_uv_command.assert_called_once()
# Should call export and workspace dir then install_requirements
assert self.mock_uv_runner._uv.run_uv_command.call_count == 2
self.mock_uv_runner.install_requirements.assert_called_once()

# Verify install_requirements is called from workspace root
assert self.mock_uv_runner._uv.run_uv_command.call_args.kwargs["cwd"] == "/path/to"
assert self.mock_uv_runner.install_requirements.call_args.kwargs["cwd"] == "/workspace"

def test_build_from_requirements(self):
self.builder._build_from_requirements(
requirements_path="/path/to/requirements.txt",
Expand Down Expand Up @@ -232,15 +240,17 @@ def test_build_dependencies_with_uv_lock_standalone_fails(self):

def test_build_dependencies_pyproject_with_uv_lock(self):
"""Test that pyproject.toml with uv.lock present uses lock-based build."""
# Mock the uv export command
self.mock_uv_runner._uv.run_uv_command.return_value = (0, b"", b"")
# Mock the uv commands
self.mock_uv_runner._uv.run_uv_command.side_effect = [
(0, b"", b""), # export
(0, "/workspace\n", b""), # workspace dir
]

with (
patch("os.path.basename", return_value="pyproject.toml"),
patch("os.path.dirname", return_value=os.path.join("path", "to")),
patch("os.path.exists") as mock_exists,
):

# Mock that uv.lock exists alongside pyproject.toml
mock_exists.return_value = True

Expand All @@ -251,16 +261,20 @@ def test_build_dependencies_pyproject_with_uv_lock(self):
architecture=X86_64,
)

# Should use export + install_requirements (for cross-platform support)
self.mock_uv_runner._uv.run_uv_command.assert_called_once() # export
# Should use export + workspace dir + install_requirements (for cross-platform support)
assert self.mock_uv_runner._uv.run_uv_command.call_count == 2
self.mock_uv_runner.install_requirements.assert_called_once()

# Verify install_requirements is called from workspace root
assert self.mock_uv_runner._uv.run_uv_command.call_args.kwargs["cwd"] == "path/to"
assert self.mock_uv_runner.install_requirements.call_args.kwargs["cwd"] == "/workspace"

# Verify it checked for uv.lock in the right location
mock_exists.assert_called_with(os.path.join("path", "to", "uv.lock"))

# Verify export excludes PEP 735 default dependency-groups (dev/test deps
# must not land in Lambda zips).
export_args = self.mock_uv_runner._uv.run_uv_command.call_args[0][0]
export_args = self.mock_uv_runner._uv.run_uv_command.call_args_list[-2][0][0]
self.assertIn("--no-default-groups", export_args)

def test_build_dependencies_pyproject_without_uv_lock(self):
Expand Down