[ty] Fall back to python3 found in $PATH if no environment is found#22843
Merged
carljm merged 13 commits intoastral-sh:mainfrom Feb 10, 2026
Merged
[ty] Fall back to python3 found in $PATH if no environment is found#22843carljm merged 13 commits intoastral-sh:mainfrom
carljm merged 13 commits intoastral-sh:mainfrom
Conversation
figsoda
requested review from
AlexWaygood,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
Typing conformance resultsNo changes detected ✅ |
|
|
figsoda
force-pushed
the
path-env
branch
2 times, most recently
from
e1be8c6 to
2d6c9c8
Compare
AlexWaygood
reviewed
Jan 25, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Thanks. I think I'm in favour of this -- this is a simple fallback that is also used by pyright and pyrefly. We've talked in the past about using uv to determine where system Python environments are located, but I'm sceptical that using uv would work as well as this because:
- not all ty users will have uv installed, and we can only use uv's system-Python detection if uv is also installed
- if they do have uv installed, they're likely a uv user, and most uv users will anyway be using virtual environments. We already handle Python environment detection just fine if a virtual environment is either activated or placed in a conventional location.
If we want to improve our system environment detection, I think it has to work well for non-uv users, since most people using system Python environments will likely not be uv users.
Contributor
Author
|
Thanks for the review! I'm not familar with the code base, and I'm not sure what the test failures entail. I'm guessing they make different assumptions about the python environment found in CI? |
Member
|
When I run Which implies that:
With regards to (1), the technique we use elsewhere is to extend the ruff/crates/ruff_db/src/system/test.rs Lines 49 to 55 in bd4e6f3 ruff/crates/ty_server/tests/e2e/main.rs Lines 226 to 235 in bd4e6f3 I don't know what the solution to (2) is |
Contributor
Author
|
Thanks for the pointer on (1), I was able to pass all the e2e tests with a fake python. Note that this does unset $PATH in the tests, so stuff like uv would also be gone if we were to stop using diff --git a/Cargo.lock b/Cargo.lock
index 317e73b662..ed011de866 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4660,6 +4660,7 @@ dependencies = [
"ty_module_resolver",
"ty_project",
"ty_python_semantic",
+ "which",
]
[[package]]
diff --git a/crates/ty_server/Cargo.toml b/crates/ty_server/Cargo.toml
index d3597df57c..a1d48298ba 100644
--- a/crates/ty_server/Cargo.toml
+++ b/crates/ty_server/Cargo.toml
@@ -39,6 +39,7 @@ shellexpand = { workspace = true }
thiserror = { workspace = true }
tracing = { workspace = true }
tracing-subscriber = { workspace = true, features = ["chrono"] }
+which = { workspace = true }
[target.'cfg(target_vendor = "apple")'.dependencies]
libc = { workspace = true }
diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs
index 0eb422dd16..2949b9d588 100644
--- a/crates/ty_server/tests/e2e/main.rs
+++ b/crates/ty_server/tests/e2e/main.rs
@@ -1138,6 +1138,20 @@ impl TestServerBuilder {
self
}
+ pub(crate) fn with_binaries(mut self, binaries: &[&str]) -> Self {
+ let bin_dir = self.test_context._temp_dir.path().join("bin");
+ std::fs::create_dir_all(&bin_dir).unwrap();
+ for binary in binaries {
+ let src = which::which(binary).unwrap();
+ std::os::unix::fs::symlink(src, bin_dir.join(binary)).unwrap();
+ }
+ self.env_vars.push((
+ "PATH".into(),
+ Some(bin_dir.into_os_string().into_string().unwrap()),
+ ));
+ self
+ }
+
/// Add a workspace to the test server with the given root path and options.
///
/// This option will be used to respond to the `workspace/configuration` request that theThe file_watching tests are still failing at the moment since they are not using ruff/crates/ty/tests/file_watching.rs Line 395 in 8acd52d As for (2), the operations in |
Contributor
Author
|
looks like the shim check was enough for ci to be passing :) |
carljm
reviewed
Feb 3, 2026
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Thanks for working on this!
Contributor
|
Ideally we would add a test for this functionality -- but AFAICS |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
prefect
sphinx
trio
|
Member
|
I confirmed locally that all tests in the |
Member
|
I think you just need to rebase this on |
carljm
approved these changes
Feb 10, 2026
Member
|
We need to document this fallback here: ruff/crates/ty_project/src/metadata/options.rs Lines 666 to 689 in 4d39967 there is also documentation about our environment discovery here, but I'd be inclined not to add any more information here, since it's good for the CLI help text to be concise: Lines 70 to 79 in 38fa994 @figsoda or @BurntSushi, would one of you be able to pick that up? |
BurntSushi
added a commit
that referenced
this pull request
Feb 11, 2026
BurntSushi
added a commit
to astral-sh/ty
that referenced
this pull request
Feb 11, 2026
Member
|
@AlexWaygood Ah nice catch! I put up #23229 and astral-sh/ty#2787 |
BurntSushi
added a commit
that referenced
this pull request
Feb 11, 2026
BurntSushi
added a commit
to astral-sh/ty
that referenced
this pull request
Feb 11, 2026
BurntSushi
added a commit
that referenced
this pull request
Feb 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I have a python environment setup using Nixpkgs'
python3.withPackages, which, if I understand correctly, basically wraps the regular python binary and uses site.addsitedir to introduce the site packages.This didn't work with ty out of the box, and using
ty check --python $(which python3)or settingenvironment.pythonto the wrapped python binary fixes it for me.This pull request makes it so that
PythonEnvironment::discoverfalls back to$(which python3). I'm not sure if this is the right way to do this, feel free to turn this into an issue if it's not.Test Plan
I have only tested it with my own setup with
python3.withPackages.