From 55aedda37934ee2153312130de5efebbd23efebb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 15 May 2024 12:29:39 -0400 Subject: [PATCH] Separate cache construction from initialization (#3607) ## Summary Ensures that we only initialize the cache for commands that require it. Closes https://github.com/astral-sh/uv/issues/3539. --- crates/bench/benches/uv.rs | 2 +- crates/uv-cache/src/cli.rs | 7 ---- crates/uv-cache/src/lib.rs | 19 +++++----- crates/uv-client/tests/remote_metadata.rs | 2 +- crates/uv-client/tests/user_agent_version.rs | 4 +-- crates/uv-dev/src/build.rs | 2 +- crates/uv-dev/src/compile.rs | 2 +- crates/uv-dev/src/wheel_metadata.rs | 5 ++- crates/uv-interpreter/src/find_python.rs | 21 ++++++----- crates/uv-interpreter/src/interpreter.rs | 2 +- crates/uv-interpreter/src/py_launcher.rs | 12 +++---- crates/uv-resolver/tests/resolver.rs | 10 +++--- crates/uv/src/main.rs | 37 ++++++++++++++++++-- crates/uv/tests/common/mod.rs | 2 +- 14 files changed, 78 insertions(+), 49 deletions(-) diff --git a/crates/bench/benches/uv.rs b/crates/bench/benches/uv.rs index 5328a0a07..8816dbaaa 100644 --- a/crates/bench/benches/uv.rs +++ b/crates/bench/benches/uv.rs @@ -13,7 +13,7 @@ fn resolve_warm_jupyter(c: &mut Criterion) { .build() .unwrap(); - let cache = &Cache::from_path(".cache").unwrap(); + let cache = &Cache::from_path(".cache").unwrap().init().unwrap(); let manifest = &Manifest::simple(vec![Requirement::from_pep508( pep508_rs::Requirement::from_str("jupyter").unwrap(), ) diff --git a/crates/uv-cache/src/cli.rs b/crates/uv-cache/src/cli.rs index 2d9bac089..e027120f3 100644 --- a/crates/uv-cache/src/cli.rs +++ b/crates/uv-cache/src/cli.rs @@ -51,13 +51,6 @@ impl Cache { impl TryFrom for Cache { type Error = io::Error; - /// Prefer, in order: - /// 1. A temporary cache directory, if the user requested `--no-cache`. - /// 2. The specific cache directory specified by the user via `--cache-dir` or `UV_CACHE_DIR`. - /// 3. The system-appropriate cache directory. - /// 4. A `.uv_cache` directory in the current working directory. - /// - /// Returns an absolute cache dir. fn try_from(value: CacheArgs) -> Result { Cache::from_settings(value.no_cache, value.cache_dir) } diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index fd080016f..49775811f 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -128,7 +128,7 @@ impl Cache { /// A persistent cache directory at `root`. pub fn from_path(root: impl Into) -> Result { Ok(Self { - root: Self::init(root)?, + root: root.into(), refresh: Refresh::None, _temp_dir_drop: None, }) @@ -138,7 +138,7 @@ impl Cache { pub fn temp() -> Result { let temp_dir = tempdir()?; Ok(Self { - root: Self::init(temp_dir.path())?, + root: temp_dir.path().to_path_buf(), refresh: Refresh::None, _temp_dir_drop: Some(Arc::new(temp_dir)), }) @@ -243,15 +243,15 @@ impl Cache { Ok(id) } - /// Initialize a directory for use as a cache. - fn init(root: impl Into) -> Result { - let root = root.into(); + /// Initialize the cache. + pub fn init(self) -> Result { + let root = &self.root; // Create the cache directory, if it doesn't exist. - fs::create_dir_all(&root)?; + fs::create_dir_all(root)?; // Add the CACHEDIR.TAG. - cachedir::ensure_tag(&root)?; + cachedir::ensure_tag(root)?; // Add the .gitignore. match fs::OpenOptions::new() @@ -289,7 +289,10 @@ impl Cache { .write(true) .open(root.join(CacheBucket::BuiltWheels.to_str()).join(".git"))?; - fs::canonicalize(root) + Ok(Self { + root: fs::canonicalize(root)?, + ..self + }) } /// Clear the cache, removing all entries. diff --git a/crates/uv-client/tests/remote_metadata.rs b/crates/uv-client/tests/remote_metadata.rs index ffa6765a1..daac77d09 100644 --- a/crates/uv-client/tests/remote_metadata.rs +++ b/crates/uv-client/tests/remote_metadata.rs @@ -11,7 +11,7 @@ use uv_client::RegistryClientBuilder; #[tokio::test] async fn remote_metadata_with_and_without_cache() -> Result<()> { - let cache = Cache::temp()?; + let cache = Cache::temp()?.init()?; let client = RegistryClientBuilder::new(cache).build(); // The first run is without cache (the tempdir is empty), the second has the cache from the diff --git a/crates/uv-client/tests/user_agent_version.rs b/crates/uv-client/tests/user_agent_version.rs index b9cd59c4a..1e63a5555 100644 --- a/crates/uv-client/tests/user_agent_version.rs +++ b/crates/uv-client/tests/user_agent_version.rs @@ -48,7 +48,7 @@ async fn test_user_agent_has_version() -> Result<()> { }); // Initialize uv-client - let cache = Cache::temp()?; + let cache = Cache::temp()?.init()?; let client = RegistryClientBuilder::new(cache).build(); // Send request to our dummy server @@ -122,7 +122,7 @@ async fn test_user_agent_has_linehaul() -> Result<()> { .unwrap(); // Initialize uv-client - let cache = Cache::temp()?; + let cache = Cache::temp()?.init()?; let mut builder = RegistryClientBuilder::new(cache).markers(&markers); let linux = Platform::new( diff --git a/crates/uv-dev/src/build.rs b/crates/uv-dev/src/build.rs index 79be5cc58..965f3f056 100644 --- a/crates/uv-dev/src/build.rs +++ b/crates/uv-dev/src/build.rs @@ -53,7 +53,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result { BuildKind::Wheel }; - let cache = Cache::try_from(args.cache_args)?; + let cache = Cache::try_from(args.cache_args)?.init()?; let venv = PythonEnvironment::from_virtualenv(&cache)?; let client = RegistryClientBuilder::new(cache.clone()).build(); diff --git a/crates/uv-dev/src/compile.rs b/crates/uv-dev/src/compile.rs index 86421a55c..d996ba17b 100644 --- a/crates/uv-dev/src/compile.rs +++ b/crates/uv-dev/src/compile.rs @@ -15,7 +15,7 @@ pub(crate) struct CompileArgs { } pub(crate) async fn compile(args: CompileArgs) -> anyhow::Result<()> { - let cache = Cache::try_from(args.cache_args)?; + let cache = Cache::try_from(args.cache_args)?.init()?; let interpreter = if let Some(python) = args.python { python diff --git a/crates/uv-dev/src/wheel_metadata.rs b/crates/uv-dev/src/wheel_metadata.rs index 9b6788634..beb915488 100644 --- a/crates/uv-dev/src/wheel_metadata.rs +++ b/crates/uv-dev/src/wheel_metadata.rs @@ -18,9 +18,8 @@ pub(crate) struct WheelMetadataArgs { } pub(crate) async fn wheel_metadata(args: WheelMetadataArgs) -> Result<()> { - let cache_dir = Cache::try_from(args.cache_args)?; - - let client = RegistryClientBuilder::new(cache_dir.clone()).build(); + let cache = Cache::try_from(args.cache_args)?.init()?; + let client = RegistryClientBuilder::new(cache).build(); let filename = WheelFilename::from_str( args.url diff --git a/crates/uv-interpreter/src/find_python.rs b/crates/uv-interpreter/src/find_python.rs index fa9145dba..be1f9e252 100644 --- a/crates/uv-interpreter/src/find_python.rs +++ b/crates/uv-interpreter/src/find_python.rs @@ -727,12 +727,12 @@ mod windows { #[test] #[cfg_attr(not(windows), ignore)] fn no_such_python_path() { - let result = - find_requested_python(r"C:\does\not\exists\python3.12", &Cache::temp().unwrap()) - .unwrap() - .ok_or(Error::RequestedPythonNotFound( - r"C:\does\not\exists\python3.12".to_string(), - )); + let cache = Cache::temp().unwrap().init().unwrap(); + let result = find_requested_python(r"C:\does\not\exists\python3.12", &cache) + .unwrap() + .ok_or(Error::RequestedPythonNotFound( + r"C:\does\not\exists\python3.12".to_string(), + )); assert_snapshot!( format_err(result), @"Failed to locate Python interpreter at: `C:\\does\\not\\exists\\python3.12`" @@ -760,8 +760,9 @@ mod tests { #[test] #[cfg_attr(not(unix), ignore)] fn no_such_python_version() { + let cache = Cache::temp().unwrap().init().unwrap(); let request = "3.1000"; - let result = find_requested_python(request, &Cache::temp().unwrap()) + let result = find_requested_python(request, &cache) .unwrap() .ok_or(Error::NoSuchPython(request.to_string())); assert_snapshot!( @@ -773,8 +774,9 @@ mod tests { #[test] #[cfg_attr(not(unix), ignore)] fn no_such_python_binary() { + let cache = Cache::temp().unwrap().init().unwrap(); let request = "python3.1000"; - let result = find_requested_python(request, &Cache::temp().unwrap()) + let result = find_requested_python(request, &cache) .unwrap() .ok_or(Error::NoSuchPython(request.to_string())); assert_snapshot!( @@ -786,7 +788,8 @@ mod tests { #[test] #[cfg_attr(not(unix), ignore)] fn no_such_python_path() { - let result = find_requested_python("/does/not/exists/python3.12", &Cache::temp().unwrap()) + let cache = Cache::temp().unwrap().init().unwrap(); + let result = find_requested_python("/does/not/exists/python3.12", &cache) .unwrap() .ok_or(Error::RequestedPythonNotFound( "/does/not/exists/python3.12".to_string(), diff --git a/crates/uv-interpreter/src/interpreter.rs b/crates/uv-interpreter/src/interpreter.rs index 66e67c1fc..2d52b10fb 100644 --- a/crates/uv-interpreter/src/interpreter.rs +++ b/crates/uv-interpreter/src/interpreter.rs @@ -677,7 +677,7 @@ mod tests { } "##}; - let cache = Cache::temp().unwrap(); + let cache = Cache::temp().unwrap().init().unwrap(); fs::write( &mocked_interpreter, diff --git a/crates/uv-interpreter/src/py_launcher.rs b/crates/uv-interpreter/src/py_launcher.rs index d29e26963..483f29f4e 100644 --- a/crates/uv-interpreter/src/py_launcher.rs +++ b/crates/uv-interpreter/src/py_launcher.rs @@ -112,12 +112,12 @@ mod tests { #[test] #[cfg_attr(not(windows), ignore)] fn no_such_python_path() { - let result = - find_requested_python(r"C:\does\not\exists\python3.12", &Cache::temp().unwrap()) - .unwrap() - .ok_or(Error::RequestedPythonNotFound( - r"C:\does\not\exists\python3.12".to_string(), - )); + let cache = Cache::temp().unwrap().init().unwrap(); + let result = find_requested_python(r"C:\does\not\exists\python3.12", &cache) + .unwrap() + .ok_or(Error::RequestedPythonNotFound( + r"C:\does\not\exists\python3.12".to_string(), + )); assert_snapshot!( format_err(result), @"Failed to locate Python interpreter at: `C:\\does\\not\\exists\\python3.12`" diff --git a/crates/uv-resolver/tests/resolver.rs b/crates/uv-resolver/tests/resolver.rs index 1a9f00361..5eca33aa0 100644 --- a/crates/uv-resolver/tests/resolver.rs +++ b/crates/uv-resolver/tests/resolver.rs @@ -123,15 +123,15 @@ async fn resolve( markers: &'static MarkerEnvironment, tags: &Tags, ) -> Result { - let client = RegistryClientBuilder::new(Cache::temp()?).build(); + let cache = Cache::temp().unwrap().init().unwrap(); + let real_interpreter = find_default_python(&cache).expect("Expected a python to be installed"); + let client = RegistryClientBuilder::new(cache).build(); let flat_index = FlatIndex::default(); let index = InMemoryIndex::default(); - // TODO(konstin): Should we also use the bootstrapped pythons here? - let real_interpreter = - find_default_python(&Cache::temp().unwrap()).expect("Expected a python to be installed"); let interpreter = Interpreter::artificial(real_interpreter.platform().clone(), markers.clone()); let python_requirement = PythonRequirement::from_marker_environment(&interpreter, markers); - let build_context = DummyContext::new(Cache::temp()?, interpreter.clone()); + let cache = Cache::temp().unwrap().init().unwrap(); + let build_context = DummyContext::new(cache, interpreter.clone()); let hashes = HashStrategy::None; let installed_packages = EmptyInstalledPackages; let concurrency = Concurrency::default(); diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index 5821af0ee..e523b1f56 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -177,7 +177,8 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipCompileSettings::resolve(args, workspace); - let cache = cache.with_refresh(args.refresh); + // Initialize the cache. + let cache = cache.init()?.with_refresh(args.refresh); let requirements = args .src_file .into_iter() @@ -247,7 +248,8 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipSyncSettings::resolve(args, workspace); - let cache = cache.with_refresh(args.refresh); + // Initialize the cache. + let cache = cache.init()?.with_refresh(args.refresh); let sources = args .src_file .into_iter() @@ -288,10 +290,12 @@ async fn run() -> Result { command: PipCommand::Install(args), }) => { args.compat_args.validate()?; + // Resolve the settings from the command-line arguments and workspace configuration. let args = PipInstallSettings::resolve(args, workspace); - let cache = cache.with_refresh(args.refresh); + // Initialize the cache. + let cache = cache.init()?.with_refresh(args.refresh); let requirements = args .package .into_iter() @@ -360,6 +364,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipUninstallSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + let sources = args .package .into_iter() @@ -391,6 +398,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipFreezeSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::pip_freeze( args.exclude_editable, args.shared.strict, @@ -408,6 +418,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipListSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::pip_list( args.editable, args.exclude_editable, @@ -426,6 +439,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipShowSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::pip_show( args.package, args.shared.strict, @@ -441,6 +457,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = PipCheckSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::pip_check( args.shared.python.as_deref(), args.shared.system, @@ -467,6 +486,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = settings::VenvSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + // Since we use ".venv" as the default name, we use "." as the default prompt. let prompt = args.prompt.or_else(|| { if args.name == PathBuf::from(".venv") { @@ -499,6 +521,9 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let args = settings::RunSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + let requirements = args .with .into_iter() @@ -535,12 +560,18 @@ async fn run() -> Result { // Resolve the settings from the command-line arguments and workspace configuration. let _args = settings::SyncSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::sync(globals.preview, &cache, printer).await } Commands::Lock(args) => { // Resolve the settings from the command-line arguments and workspace configuration. let _args = settings::LockSettings::resolve(args, workspace); + // Initialize the cache. + let cache = cache.init()?; + commands::lock(globals.preview, &cache, printer).await } #[cfg(feature = "self-update")] diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index fb6cbbcd8..d6bf4f066 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -374,7 +374,7 @@ pub fn python_path_with_versions( temp_dir: &assert_fs::TempDir, python_versions: &[&str], ) -> anyhow::Result { - let cache = Cache::from_path(temp_dir.child("cache").to_path_buf())?; + let cache = Cache::from_path(temp_dir.child("cache").to_path_buf())?.init()?; let selected_pythons = python_versions .iter() .flat_map(|python_version| {