Refactor `PythonVersionFile` global loading by zanieb · Pull Request #14107 · astral-sh/uv

zanieb · 2025-06-17T13:57:02Z

I was looking into uv tool not supporting version files, and noticed this implementation was confusing and skipped handling like a tracing log if --no-config excludes selection a file. I've refactored it in preparation for the next change.

zanieb · 2025-06-17T16:56:11Z

crates/uv-python/src/version_files.rs

-        }
-        Ok(None)
+    fn find_global(options: &DiscoveryOptions<'_>) -> Option<PathBuf> {
+        let user_config_dir = user_uv_config_dir()?;


I'm not sure why this was passed in previously.

zanieb · 2025-06-17T16:56:32Z

crates/uv-python/src/version_files.rs

-        user_config_working_directory: impl AsRef<Path>,
-        options: &DiscoveryOptions<'_>,
-    ) -> Result<Option<Self>, std::io::Error> {
-        if !options.no_config {


We'll respect the no_config logic above instead, which has a message for when it's applied.

zanieb · 2025-06-17T16:57:35Z

crates/uv-python/src/version_files.rs

+            // First, try to find a local version file.
+            let local = Self::find_nearest(&working_directory, options);
+            if local.is_none() {
+                // Log where we searched for the file, if not found


These messages are just indented, we don't want to show them when no_local is enabled.

What do you mean by just indented?

The code is literally indented, there are no changes otherwise.

zanieb · 2025-06-18T23:43:03Z

Example usage in the tool install change at #14112

uv/crates/uv/src/commands/tool/install.rs

Lines 77 to 95 in c421cb9

    
           let (python_request, explicit_python_request) = if let Some(request) = python.as_deref() { 
        
               (Some(PythonRequest::parse(request)), true) 
        
           } else { 
        
               // Discover a global Python version pin, if no request was made 
        
               ( 
        
                   PythonVersionFile::discover( 
        
                       // TODO(zanieb): We don't use the directory, should we expose another interface? 
        
                       // Should `no_local` be implied by `None` here? 
        
                       &*CWD, 
        
                       &VersionFileDiscoveryOptions::default() 
        
                           // TODO(zanieb): Propagate `no_config` from the global options to here 
        
                           .with_no_config(false) 
        
                           .with_no_local(true), 
        
                   ) 
        
                   .await? 
        
                   .and_then(PythonVersionFile::into_version), 
        
                   false, 
        
               ) 
        
           };

oconnor663 · 2025-06-19T00:01:02Z

crates/uv-python/src/version_files.rs

+        let user_config_dir = user_uv_config_dir()?;
+        Self::find_in_directory(&user_config_dir, options)
+            .into_iter()
+            .find(|path| path.is_file())


Is this last is_file check redundant with the same check done inside find_in_directory?

Sure seems like it, not sure why that was there — I'll remove it.

oconnor663 · 2025-06-19T00:08:59Z

crates/uv/src/commands/python/pin.rs

        }
    };

-    let version_file = if global {


It seems like the previous behavior was that a global pin would only be updated if --global is set, but with the changes in this PR, it gets updated by default if no local pin is found. Is that intended?

Hm, good point. Confusingly, python_pin_global_use_local_if_available should provide test coverage for this and is passing?

This is because we only read from this file. For construction of a new file, we use this

uv/crates/uv/src/commands/python/pin.rs

Lines 188 to 200 in 6c5b1e8

// TODO(zanieb): Allow updating the discovered version file with an `--update` flag.

let new = if global {

let Some(config_dir) = user_uv_config_dir() else {

return Err(anyhow::anyhow!("No user-level config directory found."));

};

fs_err::tokio::create_dir_all(&config_dir).await?;

PythonVersionFile::new(config_dir.join(PYTHON_VERSION_FILENAME))

.with_versions(vec![request])

} else {

PythonVersionFile::new(project_dir.join(PYTHON_VERSION_FILENAME))

.with_versions(vec![request])

};

The previous code was just superfluous

Ah never mind, I was misunderstanding the change to PythonVersionFile::discover. I thought the "fall back to global if nothing local is found" behavior was new, but it's not new.

I was looking into `uv tool` not supporting version files, and noticed this implementation was confusing and skipped handling like a tracing log if `--no-config` excludes selection a file. I've refactored it in preparation for the next change.

@oconnor663