Skip to content

Conversation

@zanieb
Copy link
Member

@zanieb zanieb commented Nov 20, 2025

No description provided.

@zanieb zanieb added the documentation Improvements or additions to documentation label Nov 20, 2025
@zanieb zanieb changed the title Add documentation for intermediate layers with a workspace Add documentation for intermediate Docker layers in a workspace Nov 20, 2025
@zanieb zanieb requested a review from charliermarsh November 20, 2025 17:28
ADD . /app

RUN --mount=type=cache,target=/root/.cache/uv \
uv sync --locked
Copy link
Member

Choose a reason for hiding this comment

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

I think I would recommend --frozen here, isn't it much simpler? That's what we tend to do in our own Dockerfiles.

Copy link
Member Author

Choose a reason for hiding this comment

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

In what sense? --frozen here doesn't change anything because the whole workspace has been added already, it'd be up above where using --frozen instead of --locked makes things simpler.

I don't think we should recommend --frozen in general though, because I think --locked asserting that your uv.lock is not stale is meaningful and helpful.

I think it's fine to recommend --frozen above and rely on this subsequent --locked to catch a stale lock, but I worry that will also be confusing to people (i.e., I recommended exactly this in the issue and the user opted to do the additional mounts instead)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant to comment on the line above. I personally find it unnecessary to enumerate all of the pyproject.toml files in the uv sync --no-install-workspace when you're doing a locked install here anyway? (But -- again personally -- I wouldn't use --locked in a Dockerfile, I think that validation should be done in CI.)

In pyx at least, we instead use --frozen for this install phase, but enumerate all the workspace members when we do the ADD (to minimize invalidation).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah... I'll change it.

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants