-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Fix server version #19284
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
[ty] Fix server version #19284
Conversation
|
|
0f58920 to
474d4ac
Compare
474d4ac to
a82b6ff
Compare
|
Hmm... isn't showing the ruff version in the ty server even more confusing than showing 0.0.0? Sure, the commit hash is useful for us, but most users won't know that the ty codebase lives inside the ruff repo? |
We only use the ruff commit version when you build ty from within the Ruff repository. Only developers should really be doing this. The builds that we release are all built from within the ty repository and, therefore, they use the version from the |
Should we use the |
| }: Args, | ||
| ) -> Result<ExitStatus> { | ||
| { | ||
| ruff_db::set_program_version(crate::version::version().to_string()).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this required? Isn't this the entrypoint for the Ruff CLI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more a precaution that we don't introduce a panic in ruff when we add a new path accessing version (Both the diagnostics and ruff analzye use the ruff_db crate).
I think it's important that the server version matches the output of |
* dcreager/merge-arguments: (223 commits) fix docs Combine CallArguments and CallArgumentTypes [ty] Sync vendored typeshed stubs (#19334) [`refurb`] Make example error out-of-the-box (`FURB122`) (#19297) [refurb] Make example error out-of-the-box (FURB177) (#19309) [ty] ignore errors when reformatting codemodded typeshed (#19332) [ty] Provide docstrings for stdlib APIs when hovering over them in an IDE (#19311) [ty] Add virtual files to the only project database (#19322) Add t-string fixtures for rules that do not need to be modified (#19146) [ty] Remove `FileLookupError` (#19323) [ty] Fix handling of metaclasses in `object.<CURSOR>` completions [ty] Use an interval map for scopes by expression (#19025) [ty] List all `enum` members (#19283) [ty] Handle configuration errors in LSP more gracefully (#19262) [ty] Use python version and path from Python extension (#19012) [`pep8_naming`] Avoid false positives on standard library functions with uppercase names (`N802`) (#18907) Update Rust crate toml to 0.9.0 (#19320) [ty] Fix server version (#19284) Update NPM Development dependencies (#19319) Update taiki-e/install-action action to v2.56.13 (#19317) ...
Summary
Closes astral-sh/ty#814
cargo-dist only sets the version of the ty crate. That means, the
CARGO_PKG_VERSIONis always 0.0.0 for the ty_server.This PR works around this by adding the
program_versiontoruff_dbwhich gets initialized by the ty crate (where the version is correct)and the server can read from. This has the nice side effect that the same version can be used in the type checking panic handler (in
ty_project).An alternative would have been to pass the version to
ty_serverbut we then couldn't use it in the catch handler inty_project.I made some changes to ty's build script to take the commit from the ruff repository when not built inside the ty repository.
This gives us at least some meaningful version identifier for debug builds.
Test Plan
Version in
initializeresponse:Version logged during startup
Version in panic handler