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

Add uv sync --check flag #12342

Merged
merged 5 commits into from
Mar 21, 2025
Merged

Add uv sync --check flag #12342

merged 5 commits into from
Mar 21, 2025

Conversation

blueraft
Copy link
Contributor

Summary

Closes #12338

Test Plan

cargo test

// Running `uv sync --check` should fail.
uv_snapshot!(context.filters(), context.sync().arg("--check"), @r###"
success: false
exit_code: 2
Copy link
Member

Choose a reason for hiding this comment

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

Should this be exit code 2 or 1? I think we probably should use 1.

Would download 1 package
Would install 1 package
+ iniconfig==2.0.0
error: The environment is out of sync and requires updates. Run `uv sync` to install or update packages.
Copy link
Member

@zanieb zanieb Mar 20, 2025

Choose a reason for hiding this comment

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

I think we don't want error: here, modeling after Ruff

❯ uvx ruff format --check
Installed 1 package in 2ms
51 files already formatted
❯ echo "import  bar" > test.py
❯ uvx ruff format --check
Would reformat: test.py
1 file would be reformatted, 51 files already formatted
❯ echo $?
1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uv sync --locked already uses error: (exit code 2). Should we change both, or is it too risky due to potential breakage for users relying on the --locked exit code?

❯ uv sync --locked
Resolved 14 packages in 4ms
error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`.

Copy link
Member

Choose a reason for hiding this comment

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

Hm.. I'm not sure.

I think of the difference as: --check is asking uv to "check if my sync would do anything" whereas the other is asking uv to "sync but fail if the lockfile is outdated". In the first, it's not an error if sync would do something — that's the intent of the invocation. Whereas in the second, it's a failure to sync?

Unfortunately uv lock --check destroys my carefully constructed logic:

❯ uv lock --check
Using CPython 3.13.0
Resolved 4 packages in 86ms
error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`.
❯ echo $?
2

I'd honestly be partial to changing that one specifically (in a separate PR), I think it's wrong. I think it's probably breaking to change though.

Copy link
Member

Choose a reason for hiding this comment

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

I could be convinced to keep what you have here and change them both together in a future release.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah makes sense, I'm okay with either option, lmk. Happy to make a follow up PR fixing that!

Copy link
Member

Choose a reason for hiding this comment

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

Let's stick with consistency to uv lock for now and open an issue to track a broader change.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah seems fine to ship this as-is and re-consider in a future release.

@zanieb zanieb added the enhancement New feature or improvement to existing functionality label Mar 20, 2025
blueraft and others added 2 commits March 21, 2025 16:37

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Zanie Blue <[email protected]>
@zanieb zanieb enabled auto-merge (squash) March 21, 2025 15:46
@zanieb zanieb merged commit a80353d into astral-sh:main Mar 21, 2025
76 checks passed
@zanieb
Copy link
Member

zanieb commented Mar 21, 2025

If you're ever interested in some guidance on places we could use your help, feel free to reach out on Discord. Always appreciate your contributions.

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
None yet
Development

Successfully merging this pull request may close these issues.

Add a uv sync --check flag
3 participants