Skip to content

Commit 456bea1

Browse files
ysheikh2claude
andcommitted
Check a module in the Python environment it is imported from
In file mode and the language server, each file gets the config of its location. A library installed editable into an app's environment, but with its own config and environment, then resolves its imports in its own environment, and the app sees two copies of the same package (#3556). Project mode does not have this problem, because it uses one config for every file. `SysInfo` now also carries the site-package path of the environment. It is part of the `Handle` key and every import passes it on, so a file reached from two environments becomes two modules. When a file lies in one of its handle's site-package directories and that environment differs from its location config's, `State::get_config` uses a cached copy of the config with the importer's site-package path, so its imports resolve to the same modules as the importer's. A file opened on its own keeps its own config. Bundled stubs never carry an environment, and the stdlib is still computed once per version and platform. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 61faa2a commit 456bea1

5 files changed

Lines changed: 101 additions & 18 deletions

File tree

‎crates/pyrefly_build/src/handle.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use dupe::Dupe;
99
use pyrefly_python::module_name::ModuleName;
1010
use pyrefly_python::module_name::ModuleNameWithKind;
1111
use pyrefly_python::module_path::ModulePath;
12+
use pyrefly_python::module_path::ModulePathDetails;
1213
use pyrefly_python::sys_info::SysInfo;
1314

1415
/// A handle to a Python module, containing its name, path, and system information.
@@ -22,11 +23,7 @@ pub struct Handle {
2223
impl Handle {
2324
/// Create a new handle with a guaranteed module name.
2425
pub fn new(module: ModuleName, path: ModulePath, sys_info: SysInfo) -> Self {
25-
Self {
26-
module: ModuleNameWithKind::guaranteed(module),
27-
path,
28-
sys_info,
29-
}
26+
Self::from_with_module_name_kind(ModuleNameWithKind::guaranteed(module), path, sys_info)
3027
}
3128

3229
/// Create a new handle from a ModuleNameWithKind.
@@ -35,6 +32,15 @@ impl Handle {
3532
path: ModulePath,
3633
sys_info: SysInfo,
3734
) -> Self {
35+
// Bundled modules are shared by every Python environment, so they never carry one.
36+
let sys_info = match path.details() {
37+
ModulePathDetails::BundledTypeshed(_)
38+
| ModulePathDetails::BundledTypeshedThirdParty(_)
39+
| ModulePathDetails::BundledThirdParty(_) => sys_info.without_site_package_path(),
40+
ModulePathDetails::FileSystem(_)
41+
| ModulePathDetails::Memory(_)
42+
| ModulePathDetails::Namespace(_) => sys_info,
43+
};
3844
Self {
3945
module,
4046
path,

‎crates/pyrefly_config/src/config.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,7 @@ impl ConfigFile {
11621162

11631163
pub fn get_sys_info(&self) -> SysInfo {
11641164
SysInfo::new(self.python_version(), self.python_platform().clone())
1165+
.with_site_package_path(&self.site_package_path().cloned().collect::<Vec<_>>())
11651166
}
11661167

11671168
pub fn errors(&self, path: &ModulePath) -> Cow<'_, ErrorDisplayConfig> {

‎crates/pyrefly_python/src/sys_info.rs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::fmt;
1111
use std::fmt::Display;
1212
use std::hash::Hash;
1313
use std::hash::Hasher;
14+
use std::path::PathBuf;
1415
use std::str::FromStr;
1516

1617
use dupe::Dupe;
@@ -406,9 +407,9 @@ impl<'de> Deserialize<'de> for PythonPlatform {
406407
static SYS_INFO_INTERNER: Interner<SysInfoInner> = Interner::new();
407408

408409
/// Information available from the Python library `sys`, namely
409-
/// `version` and `platform`.
410+
/// `version` and `platform`, plus the Python environment that imports resolve in.
410411
/// Interned so that cloning is a trivial pointer copy (no atomic refcount).
411-
/// There are very few distinct SysInfo values (typically 1 per run), so the
412+
/// There are very few distinct SysInfo values (typically 1 per Python environment), so the
412413
/// leaked memory from interning is negligible.
413414
#[derive(Clone, Dupe, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
414415
pub struct SysInfo(Intern<SysInfoInner>);
@@ -418,6 +419,7 @@ struct SysInfoInner {
418419
version: PythonVersion,
419420
platform: PythonPlatform,
420421
type_checking: bool,
422+
site_package_path: Option<Vec<PathBuf>>,
421423
}
422424

423425
impl Default for SysInfoInner {
@@ -426,6 +428,7 @@ impl Default for SysInfoInner {
426428
version: PythonVersion::default(),
427429
platform: PythonPlatform::default(),
428430
type_checking: true,
431+
site_package_path: None,
429432
}
430433
}
431434
}
@@ -442,6 +445,7 @@ impl SysInfo {
442445
version,
443446
platform,
444447
type_checking: true,
448+
site_package_path: None,
445449
}))
446450
}
447451

@@ -450,6 +454,31 @@ impl SysInfo {
450454
version,
451455
platform,
452456
type_checking: false,
457+
site_package_path: None,
458+
}))
459+
}
460+
461+
/// Records the directories that third-party imports are resolved from. A handle passes
462+
/// its `SysInfo` on to every module it imports, so a file imported from two environments
463+
/// becomes two modules. An empty path means the environment is unknown.
464+
pub fn with_site_package_path(self, path: &[PathBuf]) -> Self {
465+
Self(SYS_INFO_INTERNER.intern(SysInfoInner {
466+
site_package_path: (!path.is_empty()).then(|| path.to_vec()),
467+
..(*self.0).clone()
468+
}))
469+
}
470+
471+
/// The directories that third-party imports are resolved from, if known.
472+
pub fn site_package_path(&self) -> Option<&[PathBuf]> {
473+
self.0.site_package_path.as_deref()
474+
}
475+
476+
/// The same `SysInfo` without a Python environment, for anything shared across
477+
/// environments, such as the stdlib.
478+
pub fn without_site_package_path(self) -> Self {
479+
Self(SYS_INFO_INTERNER.intern(SysInfoInner {
480+
site_package_path: None,
481+
..(*self.0).clone()
453482
}))
454483
}
455484

‎pyrefly/lib/report/pysa/module.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,12 @@ impl ModuleIds {
5050
pub fn new(handles: &[Handle]) -> ModuleIds {
5151
let step = StepLogger::start("Building unique module ids", "Built unique module ids");
5252

53-
// Collect unique SysInfo values from project handles.
54-
let sys_infos: HashSet<SysInfo> = handles.iter().map(|handle| *handle.sys_info()).collect();
53+
// Collect unique SysInfo values from project handles. Typeshed handles carry no
54+
// Python environment, so drop it to avoid duplicate typeshed handles.
55+
let sys_infos: HashSet<SysInfo> = handles
56+
.iter()
57+
.map(|handle| handle.sys_info().without_site_package_path())
58+
.collect();
5559

5660
// Build sorted typeshed handles for each SysInfo variant.
5761
let typeshed = typeshed().expect("Failed to load typeshed");

‎pyrefly/lib/state/state.rs‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,7 +2043,11 @@ impl<'a> Transaction<'a> {
20432043
return self.data.stdlib.first().unwrap().1.dupe();
20442044
}
20452045

2046-
self.data.stdlib.get(handle.sys_info()).unwrap().dupe()
2046+
self.data
2047+
.stdlib
2048+
.get(&handle.sys_info().without_site_package_path())
2049+
.unwrap()
2050+
.dupe()
20472051
}
20482052

20492053
/// Compute the `Stdlib` for each requested `SysInfo`.
@@ -2056,7 +2060,10 @@ impl<'a> Transaction<'a> {
20562060
/// Returns `true` if all entries were already cached (no work done).
20572061
fn compute_stdlib(&mut self, handles: &[Handle]) -> bool {
20582062
// Filter out SysInfos that already have a computed stdlib.
2059-
let sys_infos: SmallSet<SysInfo> = handles.iter().map(|h| h.sys_info().dupe()).collect();
2063+
let sys_infos: SmallSet<SysInfo> = handles
2064+
.iter()
2065+
.map(|h| h.sys_info().without_site_package_path())
2066+
.collect();
20602067
let missing: SmallSet<SysInfo> = sys_infos
20612068
.into_iter()
20622069
.filter(|k| !self.data.stdlib.contains_key(k))
@@ -2067,13 +2074,14 @@ impl<'a> Transaction<'a> {
20672074
// Use defaults (disabled) for stdlib - depth limiting is for user code
20682075
let thread_state = ThreadState::new(None);
20692076
for k in missing.into_iter_hashed() {
2070-
// The stdlib is cached per `SysInfo`, so every handle sharing this `SysInfo`
2071-
// must resolve to the same `typeshed_path`; otherwise the cached stdlib would
2072-
// depend on which handle happened to be seen first. Enforce that invariant
2073-
// rather than silently loading the stdlib from an arbitrary handle's typeshed.
2077+
// The stdlib is cached per `SysInfo` without its Python environment, so every
2078+
// handle sharing this key must resolve to the same `typeshed_path`; otherwise the
2079+
// cached stdlib would depend on which handle happened to be seen first. Enforce
2080+
// that invariant rather than silently loading the stdlib from an arbitrary
2081+
// handle's typeshed.
20742082
let config = handles
20752083
.iter()
2076-
.filter(|h| h.sys_info() == &*k)
2084+
.filter(|h| h.sys_info().without_site_package_path() == *k)
20772085
.map(|h| self.data.state.get_config(h))
20782086
.reduce(|a, b| {
20792087
assert_eq!(
@@ -2500,6 +2508,7 @@ impl<'a> Transaction<'a> {
25002508
// We clear the global config cache, rather than making a dedicated copy.
25012509
// This is reasonable, because we will cache the result on ModuleData.
25022510
self.data.state.config_finder.clear();
2511+
self.data.state.environment_configs.lock().clear();
25032512

25042513
// Wipe the copy of ConfigFile on each module that has changed.
25052514
// If they change, set find to dirty.
@@ -3510,6 +3519,10 @@ pub struct State {
35103519
threads: ThreadPool,
35113520
uniques: UniqueFactory,
35123521
config_finder: ConfigFinder,
3522+
/// The configs of modules checked in a Python environment other than their own, by
3523+
/// the config they derive from and the environment. Kept so that a module's config stays
3524+
/// the same object, which its import resolution cache is keyed by.
3525+
environment_configs: Mutex<SmallMap<(ArcId<ConfigFile>, SysInfo), ArcId<ConfigFile>>>,
35133526
state: RwLock<StateData>,
35143527
run_count: AtomicUsize,
35153528
committing_transaction_lock: Mutex<()>,
@@ -3545,6 +3558,7 @@ impl State {
35453558
threads: ThreadPool::new(thread_count),
35463559
uniques: UniqueFactory::new(),
35473560
config_finder,
3561+
environment_configs: Mutex::new(SmallMap::new()),
35483562
state: RwLock::new(StateData::new()),
35493563
run_count: AtomicUsize::new(0),
35503564
committing_transaction_lock: Mutex::new(()),
@@ -3584,8 +3598,37 @@ impl State {
35843598
) {
35853599
BundledTypeshedStdlib::config()
35863600
} else {
3587-
self.config_finder
3588-
.python_file(handle.module_kind(), handle.path())
3601+
let config = self
3602+
.config_finder
3603+
.python_file(handle.module_kind(), handle.path());
3604+
// A module found through the site-package path of an importer in another Python
3605+
// environment is checked in the importer's environment, so that its own imports
3606+
// resolve to the same modules as the importer's.
3607+
match handle.sys_info().site_package_path() {
3608+
Some(site_package_path)
3609+
if site_package_path
3610+
.iter()
3611+
.any(|dir| handle.path().as_path().starts_with(dir))
3612+
&& !config.site_package_path().eq(site_package_path) =>
3613+
{
3614+
self.environment_configs
3615+
.lock()
3616+
.entry((config.dupe(), *handle.sys_info()))
3617+
.or_insert_with(|| {
3618+
let mut config = ConfigFile::clone(&config);
3619+
config.python_environment.site_package_path =
3620+
Some(site_package_path.to_vec());
3621+
config
3622+
.python_environment
3623+
.interpreter_site_package_path
3624+
.clear();
3625+
config.python_environment.interpreter_editable_path.clear();
3626+
ArcId::new(config)
3627+
})
3628+
.dupe()
3629+
}
3630+
_ => config,
3631+
}
35893632
}
35903633
}
35913634

0 commit comments

Comments
 (0)