Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should uv sync sync the full workspace by default? #5727

Closed
charliermarsh opened this issue Aug 2, 2024 · 17 comments
Closed

Should uv sync sync the full workspace by default? #5727

charliermarsh opened this issue Aug 2, 2024 · 17 comments
Assignees
Labels
enhancement New feature or improvement to existing functionality

Comments

@charliermarsh
Copy link
Member

If you have a virtual workspace, we do this. But for non-virtual workspaces, we sync the workspace root. It's a little bit of a footgun (see, e.g., astral-sh/ruff#12622 (comment)).

@charliermarsh charliermarsh added the preview Experimental behavior label Aug 2, 2024
@charliermarsh
Copy link
Member Author

There are a few different cases to consider...

  • uv sync from the workspace root
  • uv sync -p foo from the workspace root
  • uv sync from a workspace member (e.g., ./foo)

@zanieb
Copy link
Member

zanieb commented Aug 2, 2024

Does this mean that all workspace requirements would no longer need to be explicit?

@charliermarsh
Copy link
Member Author

I think it'd be something like:

  • uv sync (and uv run by extension) always syncs the full workspace, regardless of whether you're in the top-level directory or a project.
  • uv sync -p always syncs the specified package (and it could be the workspace root).

@bluss
Copy link
Contributor

bluss commented Aug 2, 2024

I have a minor concern that un run would uninstall some other part of the workspace that an already running (concurrent) uv run is using - that's a drawback of the current behaviour, I mean.

@charliermarsh
Copy link
Member Author

In general uv run won't uninstall (unless your dependencies changed in some way) -- it just makes the changes necessary to get to a sufficient state, but could include some packages that you don't need.

@zanieb zanieb removed the preview Experimental behavior label Aug 20, 2024
@charliermarsh charliermarsh reopened this Sep 3, 2024
@charliermarsh
Copy link
Member Author

This has come up a few times so leaving it open for more discussion.

@jamesbraza
Copy link

I guess a good default behavior is to take a light touch, so maybe not syncing the full workspace by default makes sense.

Perhaps we can add a CLI/config arg to sync, something like --full-workspace, and provide decent docs on the decision

@vvuk
Copy link

vvuk commented Oct 30, 2024

I guess a good default behavior is to take a light touch, so maybe not syncing the full workspace by default makes sense.

Hmm.. I'd suggest the opposite -- that the good default behaviour is the least surprising one. If I've set up a workspace, I've explicitly declared "I'm going to be working with all of these members". It's very strange that uv sync sets up a venv, but doesn't install anything (unless my workspace root also happens to be a "real" project vs. just a workspace container) -- even though it does a full resolve and creates a lock file.

@charliermarsh
Copy link
Member Author

I would argue that it would also be surprising, though, if any package could import any other package regardless of whether it was declared as a dependency, which is what would be enabled if we synced the entire workspace by default. We would lose any sort of enforcement of module boundaries.

@vvuk
Copy link

vvuk commented Oct 30, 2024

But don't you end up in that state anyway, once you've synced multiple members of the workspace? e.g. from what you said above -- uv run won't uninstall, so you'll have things you don't need available in the environment.

@charliermarsh
Copy link
Member Author

uv run --exact will uninstall packages, as will uv sync, but yes uv run won't uninstall by default. That's a compromise, though, to make things work as expected with extras -- we don't want to remove extras between uv run --extra dev to uv run invocation, since that would be surprising to users.

@vvuk
Copy link

vvuk commented Oct 30, 2024

I think that makes workspaces much less useful. For example, the comfyui/plugins scenario can't be supported reasonably. Ideally, I'd want to specify extras as a global thing for the workspace. In other words, I wouldn't have the ability to run workspace member foo with extra thing and workspace member bar without extra thing. If I want that, why am I in a workspace to begin with? (Or if I really want that, uv run --isolated already exists.) Again, totally just my opinion, but to me:

  • inside a workspace, there should be one global set of packages (the full resolution), with any extra sets globally applied across all members
  • if I want to run a specific project with a different set of packages, then I should need to use --isolated

it shouldn't be up to uv to enforce perfect dependency declarations; that can be verified with --isolated, but otherwise it's a python ecosystem limitation that you can't really do this.

Edit: at the very least, an option to do this (sync everything) that can be set in the workspace root would be helpful. Then the actual default doesn't matter much.

@vvuk
Copy link

vvuk commented Oct 30, 2024

I completely missed the mention of "virtual workspace" vs. project in the initial post here. (There's also no mention of "virtual workspace" on https://docs.astral.sh/uv/concepts/workspaces/) If I just remove the [project] section (which was a dummy section -- I thought I needed one!) then I think I'm getting the behaviour that I want, but I'm currently fighting with pytorch still.

@charliermarsh
Copy link
Member Author

The “virtual workspace” concept isn’t present in the docs because they’re considered legacy. We removed them in favor of non-packaged projects (projects without a build system), which you can find in the docs.

@vvuk
Copy link

vvuk commented Oct 31, 2024

Ah ok. Please don't remove them even if legacy until there's another way to get the full-workspace sync behaviour -- it's the only way to make this work right now. non-packaged projects trigger the "sync just the project" behaviour.

@charliermarsh
Copy link
Member Author

You can now use uv sync --all-packages (same on uv run, uv export, etc.) to sync the full workspace.

@charliermarsh charliermarsh self-assigned this Nov 2, 2024
@charliermarsh charliermarsh added the enhancement New feature or improvement to existing functionality label Nov 2, 2024
@vvuk
Copy link

vvuk commented Nov 6, 2024

Great, thank you! Is there a way to set this in the workspace pyproject file, so that it doesn't need to be passed in as a command line argument?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants