Skip to content

fix(python-uv): take into account dependencies on other workspace members - #887

Open
noritada wants to merge 1 commit into
aws:developfrom
noritada:working
Open

fix(python-uv): take into account dependencies on other workspace members#887
noritada wants to merge 1 commit into
aws:developfrom
noritada:working

Conversation

@noritada

@noritada noritada commented Jul 9, 2026

Copy link
Copy Markdown

This PR fixes an issue where an application created as a member of a
uv workspace would fail to build if they depended on other workspace
members.

The following shows the problematic workspace structure and the error
message.

workspace root
├── lib
│   ├── pyproject.toml
│   └── src
├── pyproject.toml
├── sam-app
│   ├── __init__.py
│   ├── app.py
│   ├── pyproject.toml
│   ├── samconfig.toml
│   └── template.yaml
└── uv.lock
Build Failed
Error: PythonUvBuilder:ResolveDependencies - UV package build failed: Failed to build from pyproject.toml: Lock file operation failed: Failed to install dependencies using uv: UV pip install failed: Using CPython 3.13.0 interpreter at: /path/to/workspace/.venv/bin/python3
error: Distribution not found at: file:///path/to/workspace/sam-app/lib

Even though lib and sam-app are in the same directory level in the
workspace, the workflow attempts to install lib under sam-app.

In the workflow, uv export outputs a list of dependency packages,
which is then passed to uv pip install for installation. When doing
so, uv export outputs relative paths from the workspace root.
Therefore, uv pip install must be run from the workspace root, not
from the application directory.

Additionally, dependencies on other packages within the workspace are
exported as editable installations (e.g., -e ./lib) by default.
When this is passed to uv pip install, only the .pth (path
configuration) file for the package will be installed without the
package body. To prevent this, the --no-editable option needs to be
used.

Closes #892.

Commands to reproduce the build failure

uv init --bare
uv init --lib lib
sam init --name sam-app --runtime python3.14 --architecture arm64 \
    --dependency-manager pip --package-type Zip \
    --app-template hello-world
cd sam-app
uv init
uv add lib@../lib
# remove requirements.txt and edit the app
# edit template.yaml to configure `BuildMethod: python-uv` and `CodeUri: .`
sam build --beta-features

@noritada
noritada requested a review from a team as a code owner July 9, 2026 17:03
@github-actions github-actions Bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Jul 9, 2026

@aws-sam-tooling-bot aws-sam-tooling-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Results

Reviewed: 37cd1d1..e96d649
Files: 1
Comments: 1

# 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")

…bers

This commit fixes an issue where an application created as a member of a
uv workspace would fail to build if they depended on other workspace
members.

The following shows the problematic workspace structure and the error
message.

```
workspace root
├── lib
│   ├── pyproject.toml
│   └── src
├── pyproject.toml
├── sam-app
│   ├── __init__.py
│   ├── app.py
│   ├── pyproject.toml
│   ├── samconfig.toml
│   └── template.yaml
└── uv.lock
```

```
Build Failed
Error: PythonUvBuilder:ResolveDependencies - UV package build failed: Failed to build from pyproject.toml: Lock file operation failed: Failed to install dependencies using uv: UV pip install failed: Using CPython 3.13.0 interpreter at: /path/to/workspace/.venv/bin/python3
error: Distribution not found at: file:///path/to/workspace/sam-app/lib
```

Even though `lib` and `sam-app` are in the same directory level in the
workspace, the workflow attempts to install `lib` under `sam-app`.

In the workflow, `uv export` outputs a list of dependency packages,
which is then passed to `uv pip install` for installation. When doing
so, [`uv export` outputs relative paths from the workspace root][1].
Therefore, `uv pip install` must be run from the workspace root, not
from the application directory.

Additionally, dependencies on other packages within the workspace are
exported as editable installations (e.g., `-e ./lib`) by default.
When this is passed to `uv pip install`, only the `.pth` (path
configuration) file for the package will be installed without the
package body. To prevent this, the `--no-editable` option needs to be
used.

[1]: astral-sh/uv#20238
@roger-zhangg

Copy link
Copy Markdown
Member

Thanks for the fix and the unusually clear write-up, @noritada — and apologies for the long silence on this one.

I reproduced this end to end and your diagnosis is correct. Details below, including one thing I'd like changed before merge.

Yes, this fixes #892

I built the exact workspace from your repro (bare root + lib + an app member depending on lib@../lib) and drove the real PythonUvDependencyBuilder.build_dependencies() against it:

So #892 is closable when this merges.

I also confirmed that both halves of the change are load-bearing, which wasn't obvious to me from the diff alone. With only the cwd change (packager.py:365) and without --no-editable (packager.py:335), uv pip install succeeds but writes only lib.pth + lib-0.1.0.dist-info into the target — a Lambda zip that would ModuleNotFoundError at runtime. Worth stating that in the comment on line 335, since a future reader might otherwise take --no-editable for a cosmetic tidy-up and drop it.

And your claim about export paths (astral-sh/uv#20238) checks out: exporting from the member dir emits -e ./lib, i.e. relative to the workspace root, not ../lib relative to the member.

Your approach also looks like the only reliable one here. Because uv writes uv.lock at the workspace root, a workspace member never takes the os.path.exists(uv_lock_path) branch at packager.py:274-276; it goes through _build_from_pyproject, and the lock_path assembled at packager.py:398 does not actually exist on disk. That path works today only because _build_from_lock_file uses it purely for os.path.dirname (packager.py:324). So there is no local uv.lock to walk up from, and asking uv directly is the right call.

One change requested: fall back when uv workspace dir is unavailable

uv workspace dir (packager.py:351-355) is recent — added in astral-sh/uv#16678, first released in uv 0.9.9 (2025-11-12), stabilized in 0.10.0. (I tested 0.9.9 specifically: it works there with no --preview flag and no stderr warning, despite the workspace-dir preview gate mentioned in that PR.)

The issue is that rc != 0 is fatal, and every pyproject.toml build funnels through _build_from_lock_file — workspace or not. On uv 0.8.17 against a plain single-project pyproject.toml, with no workspace involved at all:

  • base 37cd1d1: succeeds
  • this PR: LockFileError: Lock file operation failed: Failed to get workspace root: error: unrecognized subcommand 'workspace'

aws-lambda-builders neither pins nor checks a minimum uv version (the only reference is DESIGN.md:298, "Minimum UV version: 0.1.0 (to be determined)"), and SAM CLI doesn't vendor uv — users bring their own, frequently pinned in CI images. As written, this breaks builds that work today for users who have no workspace and no need for the fix.

Since uv workspace dir returns the project directory itself for a non-workspace project (I verified this), degrading to the previous behaviour is exact rather than approximate:

rc, stdout, stderr = self._uv_runner._uv.run_uv_command(workspace_args, cwd=project_dir)
if rc == 0:
    workspace_dir = stdout.strip()
else:
    # `uv workspace dir` requires uv >= 0.9.9. Fall back to the project directory,
    # which is what that command returns for any non-workspace project anyway.
    LOG.debug("Could not determine workspace root, assuming no workspace: %s", stderr)
    workspace_dir = project_dir

Workspace users on uv < 0.9.9 then get today's error instead of a confusing new one, and everyone on 0.9.9+ gets the fix. A version check would also work, but a fallback seems lower-risk than parsing version strings.

Test results

  • pytest tests/unit/workflows/python_uv/ on this PR: 65 passed. Base 37cd1d1 is also 65, so nothing was dropped.
  • The PR merges cleanly into current develop (587257c); on the merge result: 68 passed (develop has added 3 tests in the meantime).
  • Full pytest tests/unit: 831 passed, 6 subtests passed.
  • black --check clean on both changed files.

Minor, non-blocking

  1. The unit tests mock run_uv_command, so they cannot catch the class of bug this PR fixes — the new assertions would pass just as happily against a wrong-but-consistent cwd string. Since tests/integration/workflows/python_uv/testdata/ already exists, a small workspace fixture there is where this behaviour could actually be pinned down. Noting it more as follow-up work for us than as a request to you.
  2. test_packager.py:277call_args_list[-2][0][0] will silently point at the wrong call if a third uv invocation is ever added ahead of the install. Matching on the call whose first arg starts with "export" would survive that.
  3. The comment at packager.py:139-141 states "UV runs with cwd set to the project directory", which is no longer strictly true. The os.path.abspath(target_dir) on line 142 remains correct (it resolves against the builder's cwd, not uv's), but the stated rationale is now slightly off and could mislead later.
  4. One thing I checked and did not find a problem with: moving cwd up to the workspace root does not appear to change uv's config discovery in a way that matters. A member-level [tool.uv.pip] index-url was not applied from either directory, so I observed no behavioural difference.

Overall the approach and the analysis behind it are right, and the end-to-end result is verified working. With the fallback above added, this looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: PythonUvBuilder fails to build app with dependencies on editable installs in the workspace

2 participants