From 175a6e3e46570b63e6b85a61ca7359ee50a4147b Mon Sep 17 00:00:00 2001 From: "Calum H. (IMB11)" Date: Tue, 10 Nov 2026 15:40:09 +0000 Subject: [PATCH 1/6] refactor: app dir move logic --- Cargo.lock | 1 + Cargo.toml | 1 + .../src/components/ui/SplashScreen.vue | 30 +- .../app-frontend/src/locales/en-US/index.json | 3 + packages/app-lib/Cargo.toml | 3 + packages/app-lib/src/state/dirs.rs | 13 +- .../src/state/dirs/move_app_directory.rs | 571 +++++++++++++++--- .../move_app_directory/managed_content.rs | 60 +- 8 files changed, 522 insertions(+), 160 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 365cc0a37f8..ba6d84bff99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13805,6 +13805,7 @@ dependencies = [ "infer 0.22.0", "itertools 0.15.0", "json5", + "libc", "minecraft-meta-manifest", "modrinth-content-management", "notify", diff --git a/Cargo.toml b/Cargo.toml index a01116a0d68..a675a94ad06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -132,6 +132,7 @@ lettre = { version = "0.11.23", default-features = false, features = [ "tokio1", "tokio1-rustls", ] } +libc = "0.2.191" lingua = "=1.8.0" linkify = "0.11.0" lz4_flex = { version = "0.14.0", default-features = false, features = [ diff --git a/apps/app-frontend/src/components/ui/SplashScreen.vue b/apps/app-frontend/src/components/ui/SplashScreen.vue index c63421f80f6..1d79183bff5 100644 --- a/apps/app-frontend/src/components/ui/SplashScreen.vue +++ b/apps/app-frontend/src/components/ui/SplashScreen.vue @@ -67,8 +67,8 @@ - - {{ message }} + + {{ message }}
@@ -78,8 +78,8 @@ diff --git a/apps/app-frontend/src/locales/en-US/index.json b/apps/app-frontend/src/locales/en-US/index.json index 2cd19d9db8a..e117129f1e1 100644 --- a/apps/app-frontend/src/locales/en-US/index.json +++ b/apps/app-frontend/src/locales/en-US/index.json @@ -3521,6 +3521,9 @@ "app.skins.toggle-ears-features-on": { "message": "Toggle on" }, + "app.splash.moving-app-directory": { + "message": "Moving your app directory..." + }, "app.survey.body": { "message": "Would you mind answering a few questions about your experience with Modrinth App?" }, diff --git a/packages/app-lib/Cargo.toml b/packages/app-lib/Cargo.toml index 305f7854a0f..1d7a866fc10 100644 --- a/packages/app-lib/Cargo.toml +++ b/packages/app-lib/Cargo.toml @@ -128,6 +128,9 @@ zip = { workspace = true } dotenvy = { workspace = true } dunce = { workspace = true } +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } + [target.'cfg(target_os = "macos")'.dependencies] cidre = { workspace = true, features = ["blocks", "nw"] } diff --git a/packages/app-lib/src/state/dirs.rs b/packages/app-lib/src/state/dirs.rs index 8e348b5973e..99f5aaedcb9 100644 --- a/packages/app-lib/src/state/dirs.rs +++ b/packages/app-lib/src/state/dirs.rs @@ -1,9 +1,8 @@ //! Theseus directory information pub(crate) mod move_app_directory; -use crate::event::emit::{emit_loading, init_loading}; +use crate::ErrorKind; use crate::state::{Settings, State}; use crate::util::fetch::IoSemaphore; -use crate::{ErrorKind, LoadingBarType}; use anyhow::Context; use std::path::PathBuf; use tokio::fs; @@ -281,22 +280,12 @@ impl DirectoryInfo { if let Some(previous) = &previous && moving { - let loading = init_loading( - LoadingBarType::DirectoryMove { - old: previous.to_string_lossy().into_owned(), - new: destination.to_string_lossy().into_owned(), - }, - 100.0, - "Moving launcher directory", - ) - .await?; move_app_directory::move_app_directory( previous, &destination, pool, ) .await?; - emit_loading(&loading, 100.0, None)?; } if !moving { move_app_directory::resume_completed_move(&destination, pool) diff --git a/packages/app-lib/src/state/dirs/move_app_directory.rs b/packages/app-lib/src/state/dirs/move_app_directory.rs index a4efb818486..8b7551355f5 100644 --- a/packages/app-lib/src/state/dirs/move_app_directory.rs +++ b/packages/app-lib/src/state/dirs/move_app_directory.rs @@ -1,17 +1,69 @@ -use crate::ErrorKind; +//! 1. The user picks a new directory. The move runs on the next startup, before +//! the rest of the app state is created. +//! 2. The old directory is walked. Folders and links are created at the +//! destination, and every file is added to a list. Rebuildable caches are +//! left out; `caches/icons` is kept because instances point at it. +//! 3. Files are copied in parallel, without syncing each one to disk. A copy +//! keeps its source's modification time, so a file already at the +//! destination with the same time is skipped. +//! 4. Every copy is hashed and compared with its source. Mismatches are copied +//! again, up to three attempts, after which the move fails. +//! 5. Every new file and folder is synced to disk once. +//! 6. Paths in the database are rewritten and the move is marked as copied. +//! 7. The old directory is deleted. +//! +//! Until step 6, the old directory is untouched and remains the source of +//! truth. If the move is interrupted before then, the next startup runs it +//! again and step 4 catches any copy left damaged. Cancelling only resets +//! settings. +//! +//! After step 6, the new directory is the source of truth. If the app stops +//! during step 7, the next startup finishes deleting the old directory. + +use crate::event::emit::{emit_loading, init_loading}; use crate::state::content_store; +use crate::{ErrorKind, LoadingBarType}; mod managed_content; use crate::state::content_store::{ - hash_file, normalize, relative_link, sync_directory, + FileHashes, hash_file, normalize, relative_link, remove_instance_file, + sync_directory, }; use crate::state::instances::adapters::sqlite::instance_rows; use crate::state::{Instance, JavaVersion}; +use crate::util::content_hash::{copy_and_hash, hash_file_with_progress}; use anyhow::bail; +use futures::stream::{self, StreamExt, TryStreamExt}; use serde_json::Value; use sqlx::SqlitePool; +use std::collections::HashSet; use std::path::{Path, PathBuf}; +use std::time::Instant; use tokio::fs; +use tokio::io::AsyncWriteExt; + +const PARALLEL_FILES: usize = 8; +const COPY_ATTEMPTS: usize = 3; + +type ProgressFn<'a> = &'a (dyn Fn(u64) + Send + Sync); + +#[derive(Default)] +struct MovePlan { + files: Vec<(PathBuf, PathBuf)>, + directories: Vec, + bytes: u64, + required: u64, +} + +impl MovePlan { + async fn add_file(&mut self, source: PathBuf, target: PathBuf, len: u64) { + self.bytes = self.bytes.saturating_add(len); + if fs::symlink_metadata(&target).await.is_err() { + self.required = self.required.saturating_add(len); + } + self.files.push((source, target)); + } +} const MOVED_APP_DIRECTORIES: [&str; 6] = [ "store", @@ -44,24 +96,28 @@ pub(crate) async fn relocate_tree(from: &Path, to: &Path) -> crate::Result<()> { } return Ok(()); } - copy_tree( + let mut plan = MovePlan::default(); + plan_tree( from, to, &[(from.to_path_buf(), to.to_path_buf())], - &std::collections::HashSet::new(), + &HashSet::new(), + &mut plan, ) - .await + .await?; + copy_planned(&plan, to, &|_| {}).await } -async fn copy_tree( +async fn plan_tree( from: &Path, to: &Path, mappings: &[(PathBuf, PathBuf)], - managed_paths: &std::collections::HashSet, + skipped_paths: &HashSet, + plan: &mut MovePlan, ) -> crate::Result<()> { let mut pending = vec![(from.to_path_buf(), to.to_path_buf())]; while let Some((source, target)) = pending.pop() { - if managed_paths.contains(&source) { + if skipped_paths.contains(&source) { continue; } let metadata = fs::symlink_metadata(&source).await?; @@ -114,47 +170,215 @@ async fn copy_tree( while let Some(entry) = entries.next_entry().await? { pending.push((entry.path(), target.join(entry.file_name()))); } + plan.directories.push(target); } else if metadata.is_file() { - if let Ok(existing) = fs::symlink_metadata(&target).await { - if !existing.is_file() - || existing.file_type().is_symlink() - || hash_file(&source).await? != hash_file(&target).await? - { - bail!(ErrorKind::InputError(format!( - "migration would overwrite different data at {}", - target.display() - ))); - } - continue; - } - let parent = target.parent().ok_or(ErrorKind::InputError( - "invalid migration target".to_string(), - ))?; - fs::create_dir_all(parent).await?; - let temporary = parent - .join(format!(".modrinth-move-{}.tmp", uuid::Uuid::new_v4())); - crate::state::content_store::writable_copy(&source, &temporary) - .await?; - fs::File::options() - .write(true) - .open(&temporary) - .await? - .sync_all() - .await?; - fs::set_permissions(&temporary, metadata.permissions()).await?; - if hash_file(&source).await? != hash_file(&temporary).await? { - bail!(ErrorKind::InputError( - "file changed or failed verification during directory migration" - .to_string(), - )); - } - fs::rename(&temporary, &target).await?; - sync_directory(parent).await?; + plan.add_file(source, target, metadata.len()).await; } } Ok(()) } +async fn copy_planned( + plan: &MovePlan, + root: &Path, + on_progress: ProgressFn<'_>, +) -> crate::Result<()> { + let mut pending = (0..plan.files.len()).collect::>(); + for attempt in 0..COPY_ATTEMPTS { + let copied: Vec<_> = stream::iter(pending) + .map(|index| async move { + let (source, target) = &plan.files[index]; + copy_file(source, target, attempt > 0, on_progress) + .await + .map(|hashes| (index, hashes)) + }) + .buffer_unordered(PARALLEL_FILES) + .try_collect() + .await?; + let verified: Vec<_> = stream::iter(copied) + .map(|(index, hashes)| async move { + let (source, target) = &plan.files[index]; + verify_file(source, target, hashes, on_progress) + .await + .map(|matches| (index, matches)) + }) + .buffer_unordered(PARALLEL_FILES) + .try_collect() + .await?; + pending = verified + .into_iter() + .filter_map(|(index, matches)| (!matches).then_some(index)) + .collect(); + if pending.is_empty() { + break; + } + tracing::warn!( + files = pending.len(), + attempt, + "Files failed verification during app directory move" + ); + } + if let Some((source, _)) = pending.first().map(|&index| &plan.files[index]) + { + bail!(ErrorKind::InputError(format!( + "{} could not be copied after {COPY_ATTEMPTS} attempts", + source.display() + ))); + } + let paths = plan + .files + .iter() + .map(|(_, target)| target) + .chain(&plan.directories) + .chain(std::iter::once(&root.to_path_buf())) + .cloned() + .collect::>(); + stream::iter(paths) + .map(|path| async move { + Ok::<_, crate::Error>( + tokio::task::spawn_blocking(move || flush_path(&path)) + .await??, + ) + }) + .buffer_unordered(PARALLEL_FILES) + .try_collect::>() + .await?; + #[cfg(target_os = "macos")] + fs::File::open(root).await?.sync_all().await?; + Ok(()) +} + +/// `None` when the file was already copied. +async fn copy_file( + source: &Path, + target: &Path, + overwrite: bool, + on_progress: ProgressFn<'_>, +) -> crate::Result> { + let metadata = fs::symlink_metadata(source).await?; + if let Ok(existing) = fs::symlink_metadata(target).await { + if !existing.is_file() + || (!overwrite + && !is_copied_file(&metadata, &existing) + && hash_file(source).await? != hash_file(target).await?) + { + bail!(ErrorKind::InputError(format!( + "migration would overwrite different data at {}", + target.display() + ))); + } + if !overwrite { + on_progress(metadata.len()); + return Ok(None); + } + remove_instance_file(target).await?; + } + let parent = target.parent().ok_or(ErrorKind::InputError( + "invalid migration target".to_string(), + ))?; + let temporary = + parent.join(format!(".modrinth-move-{}.tmp", uuid::Uuid::new_v4())); + let modified = metadata.modified()?; + let mut output = fs::File::create(&temporary).await?; + let hashes = + copy_and_hash(fs::File::open(source).await?, &mut output, on_progress) + .await?; + output.flush().await?; + output.into_std().await.set_modified(modified)?; + fs::set_permissions(&temporary, metadata.permissions()).await?; + let after = fs::metadata(source).await?; + if after.len() != hashes.size || after.modified()? != modified { + bail!(ErrorKind::InputError(format!( + "{} changed during the app directory move", + source.display() + ))); + } + fs::rename(&temporary, target).await?; + Ok(Some(hashes)) +} + +async fn verify_file( + source: &Path, + target: &Path, + hashes: Option, + on_progress: ProgressFn<'_>, +) -> crate::Result { + let expected = match hashes { + Some(hashes) => hashes, + None => hash_file(source).await?, + }; + Ok(hash_file_with_progress(target, on_progress) + .await + .is_ok_and(|actual| actual == expected)) +} + +/// - Linux and other Unix: `fsync`, the same as `sync_all`. +/// - macOS: `fsync`, as `sync_all` is a slow `F_FULLFSYNC` that flushes the +/// drive's cache. `copy_planned` issues it once after every path is flushed. +/// - Windows: `sync_all` (`FlushFileBuffers`) on files. Directories can't be +/// flushed there and are skipped. +fn flush_path(path: &Path) -> std::io::Result<()> { + #[cfg(unix)] + { + use std::os::fd::AsRawFd; + let file = std::fs::File::open(path)?; + if unsafe { libc::fsync(file.as_raw_fd()) } != 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + } + #[cfg(windows)] + { + let metadata = std::fs::metadata(path)?; + if metadata.is_dir() { + return Ok(()); + } + let permissions = metadata.permissions(); + if permissions.readonly() { + let mut writable = permissions.clone(); + writable.set_readonly(false); + std::fs::set_permissions(path, writable)?; + } + let result = std::fs::OpenOptions::new() + .write(true) + .open(path) + .and_then(|file| file.sync_all()); + if permissions.readonly() { + std::fs::set_permissions(path, permissions)?; + } + result + } +} + +/// Everything in `caches` except `icons`, which instances reference by absolute path. +async fn rebuildable_cache_entries(root: &Path) -> crate::Result> { + let mut entries = Vec::new(); + let mut caches = match fs::read_dir(root.join("caches")).await { + Ok(caches) => caches, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(entries); + } + Err(error) => return Err(error.into()), + }; + while let Some(entry) = caches.next_entry().await? { + if entry.file_name() != "icons" { + entries.push(entry.path()); + } + } + Ok(entries) +} + +/// Copies keep the source's mtime, so a match means an earlier attempt wrote this file. +fn is_copied_file( + source: &std::fs::Metadata, + target: &std::fs::Metadata, +) -> bool { + matches!( + (source.modified(), target.modified()), + (Ok(source), Ok(target)) if source == target + ) +} + async fn create_link( link: &Path, target: &Path, @@ -401,12 +625,23 @@ pub(crate) async fn move_app_directory( } content_store::set_setting(pool, "store_directory_move", &checkpoint) .await?; + let started = Instant::now(); + let loading = init_loading( + LoadingBarType::DirectoryMove { + old: dunce::simplified(&from).to_string_lossy().into_owned(), + new: dunce::simplified(&to).to_string_lossy().into_owned(), + }, + 1.0, + "Moving app directory", + ) + .await?; let managed_files = managed_content::prepare(pool, &from, &to).await?; - let managed_paths = managed_files + let mut skipped_paths: HashSet = managed_files .iter() .map(|file| file.source.clone()) .collect(); - let mut required = 0_u64; + skipped_paths.extend(rebuildable_cache_entries(&from).await?); + let mut plan = MovePlan::default(); for directory in MOVED_APP_DIRECTORIES { let source = from.join(directory); match fs::symlink_metadata(&source).await { @@ -424,28 +659,35 @@ pub(crate) async fn move_app_directory( } Err(error) => return Err(error.into()), } - if fs::try_exists(&source).await? { - required = required.saturating_add( - required_copy_bytes( - &source, - &to.join(directory), - &managed_paths, - ) - .await?, - ); - } + plan_tree( + &source, + &to.join(directory), + &mappings, + &skipped_paths, + &mut plan, + ) + .await?; } - ensure_move_space(&to, required)?; - for directory in MOVED_APP_DIRECTORIES { - let source = from.join(directory); - if fs::try_exists(&source).await? { - copy_tree(&source, &to.join(directory), &mappings, &managed_paths) - .await?; - } + for (source, target, len) in + managed_content::checkpoint(pool, managed_files, &from, &to).await? + { + plan.add_file(source, target, len).await; } - managed_content::copy_and_checkpoint(pool, managed_files, &from, &to) - .await?; + ensure_move_space(&to, plan.required)?; + tracing::info!( + from = %from.display(), + to = %to.display(), + files = plan.files.len(), + bytes = plan.bytes, + "Moving app directory" + ); + let total = plan.bytes.saturating_mul(2).max(1) as f64; + let on_progress = |bytes: u64| { + let _ = emit_loading(&loading, bytes as f64 / total, None); + }; + copy_planned(&plan, &to, &on_progress).await?; rewrite_database_paths(pool, &mappings, &checkpoint).await?; + tracing::info!(elapsed = ?started.elapsed(), "Copied app directory"); Ok(()) } @@ -475,6 +717,8 @@ pub(crate) async fn finish_app_directory_move( .to_string(), )); } + tracing::info!(from = %from.display(), "Removing old app directory data"); + let started = Instant::now(); managed_content::commit(pool).await?; for directory in MOVED_APP_DIRECTORIES { let source = from.join(directory); @@ -487,6 +731,7 @@ pub(crate) async fn finish_app_directory_move( content_store::set_setting(pool, "store_directory_move", "").await?; content_store::set_setting(pool, "store_directory_move_copied", "").await?; managed_content::clear_checkpoint(pool).await?; + tracing::info!(elapsed = ?started.elapsed(), "Finished app directory move"); Ok(()) } @@ -499,32 +744,6 @@ fn ensure_move_space(destination: &Path, required: u64) -> crate::Result<()> { Ok(()) } -async fn required_copy_bytes( - from: &Path, - to: &Path, - managed_paths: &std::collections::HashSet, -) -> crate::Result { - let mut pending = vec![(from.to_path_buf(), to.to_path_buf())]; - let mut bytes = 0_u64; - while let Some((source, target)) = pending.pop() { - if managed_paths.contains(&source) { - continue; - } - let metadata = fs::symlink_metadata(&source).await?; - if metadata.is_dir() && !metadata.file_type().is_symlink() { - let mut entries = fs::read_dir(&source).await?; - while let Some(entry) = entries.next_entry().await? { - pending.push((entry.path(), target.join(entry.file_name()))); - } - } else if metadata.is_file() - && fs::symlink_metadata(&target).await.is_err() - { - bytes = bytes.saturating_add(metadata.len()); - } - } - Ok(bytes) -} - pub(crate) async fn resume_completed_move( destination: &Path, pool: &SqlitePool, @@ -535,6 +754,7 @@ pub(crate) async fn resume_completed_move( { let (from, to): (PathBuf, PathBuf) = serde_json::from_str(&checkpoint)?; if fs::canonicalize(destination).await? == to { + tracing::info!("Resuming interrupted app directory move cleanup"); finish_app_directory_move(&from, &to, pool).await?; } } @@ -607,3 +827,174 @@ pub(crate) async fn remove_migrated_tree(root: &Path) -> crate::Result<()> { fs::remove_dir_all(root).await?; Ok(()) } + +// dont usually need tests but cant fk this one up +#[cfg(test)] +mod tests { + use super::{MovePlan, copy_planned, plan_tree}; + use std::collections::HashSet; + use std::fs; + use std::path::Path; + use std::sync::atomic::{AtomicU64, Ordering}; + + async fn copy_tree(from: &Path, to: &Path) -> crate::Result { + let mut plan = MovePlan::default(); + plan_tree( + from, + to, + &[(from.to_path_buf(), to.to_path_buf())], + &HashSet::new(), + &mut plan, + ) + .await?; + let progress = AtomicU64::new(0); + copy_planned(&plan, to, &|bytes| { + progress.fetch_add(bytes, Ordering::Relaxed); + }) + .await?; + Ok(progress.into_inner()) + } + + fn modified(path: &Path) -> std::time::SystemTime { + fs::metadata(path).unwrap().modified().unwrap() + } + + #[tokio::test] + async fn copies_tree_with_metadata_and_full_progress() { + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + let large: Vec = (0..600_000).map(|i| (i % 251) as u8).collect(); + fs::create_dir_all(from.join("mods/nested")).unwrap(); + fs::create_dir_all(from.join("empty")).unwrap(); + fs::write(from.join("mods/large.jar"), &large).unwrap(); + fs::write(from.join("mods/nested/config.toml"), b"key = 1").unwrap(); + let mut permissions = + fs::metadata(from.join("mods/nested/config.toml")) + .unwrap() + .permissions(); + permissions.set_readonly(true); + fs::set_permissions(from.join("mods/nested/config.toml"), permissions) + .unwrap(); + + let progress = copy_tree(&from, &to).await.unwrap(); + + assert_eq!(fs::read(to.join("mods/large.jar")).unwrap(), large); + assert_eq!( + fs::read(to.join("mods/nested/config.toml")).unwrap(), + b"key = 1" + ); + assert!(to.join("empty").is_dir()); + assert_eq!( + modified(&to.join("mods/large.jar")), + modified(&from.join("mods/large.jar")) + ); + assert!( + fs::metadata(to.join("mods/nested/config.toml")) + .unwrap() + .permissions() + .readonly() + ); + assert_eq!(progress, 2 * (large.len() as u64 + 7)); + assert!(fs::read_dir(to.join("mods")).unwrap().all(|entry| { + !entry + .unwrap() + .file_name() + .to_string_lossy() + .ends_with(".tmp") + })); + } + + #[cfg(unix)] + #[tokio::test] + async fn second_run_skips_copied_files() { + use std::os::unix::fs::MetadataExt; + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + fs::create_dir_all(&from).unwrap(); + fs::write(from.join("file.bin"), b"contents").unwrap(); + copy_tree(&from, &to).await.unwrap(); + let inode = fs::metadata(to.join("file.bin")).unwrap().ino(); + + copy_tree(&from, &to).await.unwrap(); + + assert_eq!(fs::metadata(to.join("file.bin")).unwrap().ino(), inode); + } + + #[tokio::test] + async fn recopies_damaged_copy() { + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + fs::create_dir_all(&from).unwrap(); + fs::write(from.join("file.bin"), b"original contents").unwrap(); + copy_tree(&from, &to).await.unwrap(); + fs::write(to.join("file.bin"), b"damaged").unwrap(); + fs::File::options() + .write(true) + .open(to.join("file.bin")) + .unwrap() + .set_modified(modified(&from.join("file.bin"))) + .unwrap(); + + copy_tree(&from, &to).await.unwrap(); + + assert_eq!( + fs::read(to.join("file.bin")).unwrap(), + b"original contents" + ); + } + + #[tokio::test] + async fn refuses_to_overwrite_different_file() { + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + fs::create_dir_all(&from).unwrap(); + fs::create_dir_all(&to).unwrap(); + fs::write(from.join("file.bin"), b"source").unwrap(); + fs::write(to.join("file.bin"), b"someone else's data").unwrap(); + + assert!(copy_tree(&from, &to).await.is_err()); + assert_eq!( + fs::read(to.join("file.bin")).unwrap(), + b"someone else's data" + ); + } + + #[tokio::test] + async fn accepts_identical_existing_file() { + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + fs::create_dir_all(&from).unwrap(); + fs::create_dir_all(&to).unwrap(); + fs::write(from.join("file.bin"), b"same").unwrap(); + fs::write(to.join("file.bin"), b"same").unwrap(); + + copy_tree(&from, &to).await.unwrap(); + + assert_eq!(fs::read(to.join("file.bin")).unwrap(), b"same"); + } + + #[cfg(unix)] + #[tokio::test] + async fn remaps_links_into_the_new_directory() { + let root = tempfile::tempdir().unwrap(); + let from = root.path().join("from"); + let to = root.path().join("to"); + fs::create_dir_all(from.join("store")).unwrap(); + fs::write(from.join("store/blob"), b"blob").unwrap(); + std::os::unix::fs::symlink(from.join("store/blob"), from.join("link")) + .unwrap(); + + copy_tree(&from, &to).await.unwrap(); + + assert!(fs::read_link(to.join("link")).unwrap().is_relative()); + assert_eq!( + fs::canonicalize(to.join("link")).unwrap(), + fs::canonicalize(to.join("store/blob")).unwrap() + ); + } +} diff --git a/packages/app-lib/src/state/dirs/move_app_directory/managed_content.rs b/packages/app-lib/src/state/dirs/move_app_directory/managed_content.rs index 45993f6e1ba..6b0fd199e80 100644 --- a/packages/app-lib/src/state/dirs/move_app_directory/managed_content.rs +++ b/packages/app-lib/src/state/dirs/move_app_directory/managed_content.rs @@ -1,11 +1,8 @@ use crate::ErrorKind; use crate::state::content_store; use crate::state::content_store::{ - FileStorageKind, content_file_path, hash_file, sync_directory, - validate_instance_path, validate_relative, -}; -use crate::state::content_store::{ - remove_instance_file, validate_parent_directories, writable_copy, + FileStorageKind, content_file_path, validate_instance_path, + validate_parent_directories, validate_relative, }; use crate::state::instances::adapters::sqlite::{content_rows, instance_rows}; use anyhow::bail; @@ -81,12 +78,13 @@ pub(super) async fn prepare( Ok(moves) } -pub(super) async fn copy_and_checkpoint( +pub(super) async fn checkpoint( pool: &SqlitePool, files: Vec, from: &Path, to: &Path, -) -> crate::Result<()> { +) -> crate::Result> { + let mut copies = Vec::new(); let mut bindings = Vec::new(); for mut file in files { validate_parent_directories(from, &file.source).await?; @@ -105,61 +103,21 @@ pub(super) async fn copy_and_checkpoint( .to_string(), )); } - let hashes = hash_file(&file.source).await?; let parent = file.destination.parent().ok_or(ErrorKind::InputError( "invalid content destination".to_string(), ))?; fs::create_dir_all(parent).await?; - let temporary = - parent.join(format!(".modrinth-move-{}.tmp", uuid::Uuid::new_v4())); - let result = async { - super::ensure_move_space(parent, hashes.size)?; - writable_copy(&file.source, &temporary).await?; - if hash_file(&temporary).await? != hashes - || hash_file(&file.source).await? != hashes - { - bail!(ErrorKind::InputError( - "content changed during the app-directory move".to_string(), - )); - } - fs::File::options() - .write(true) - .open(&temporary) - .await? - .sync_all() - .await?; - match fs::symlink_metadata(&file.destination).await { - Ok(metadata) => { - if !metadata.is_file() - || hash_file(&file.destination).await? != hashes - { - bail!(ErrorKind::InputError( - "the content destination contains different data" - .to_string(), - )); - } - remove_instance_file(&file.destination).await?; - } - Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} - Err(error) => return Err(error.into()), - } - fs::rename(&temporary, &file.destination).await?; - sync_directory(parent).await?; - Ok(FileStorageKind::Copy) - } - .await; - if result.is_err() && fs::symlink_metadata(&temporary).await.is_ok() { - let _ = remove_instance_file(&temporary).await; - } - file.binding.storage_kind = result?; + file.binding.storage_kind = FileStorageKind::Copy; bindings.push(file.binding); + copies.push((file.source, file.destination, metadata.len())); } content_store::set_setting( pool, CHECKPOINT_KEY, &serde_json::to_string(&bindings)?, ) - .await + .await?; + Ok(copies) } /// Records how files are stored on the destination filesystem after the app switches to it. From 27876babff8b97fbda9e9f662af6002035389f56 Mon Sep 17 00:00:00 2001 From: "Calum H. (IMB11)" Date: Tue, 10 Nov 2026 16:11:23 +0000 Subject: [PATCH 2/6] feat: better app dir change ui flow --- .../app-directory/change-directory-modal.vue | 228 ++++++++++++++++++ .../app-directory/index.vue | 196 +++++++++++++++ .../app-directory/paths.ts | 21 ++ .../app-directory/use-app-directory.ts | 81 +++++++ .../app-directory/use-copy-path.ts | 38 +++ .../resource-management-settings/index.vue | 91 ++----- .../app-frontend/src/locales/en-US/index.json | 77 +++++- .../src/platform/app-lib/settings/commands.ts | 5 + apps/app-frontend/src/platform/dialogs.ts | 6 + apps/app/build.rs | 1 + apps/app/src/api/settings.rs | 8 +- packages/app-lib/src/api/settings.rs | 4 + 12 files changed, 681 insertions(+), 75 deletions(-) create mode 100644 apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/change-directory-modal.vue create mode 100644 apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/index.vue create mode 100644 apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/paths.ts create mode 100644 apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-app-directory.ts create mode 100644 apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-copy-path.ts create mode 100644 apps/app-frontend/src/platform/app-lib/settings/commands.ts create mode 100644 apps/app-frontend/src/platform/dialogs.ts diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/change-directory-modal.vue b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/change-directory-modal.vue new file mode 100644 index 00000000000..2dbae5984e9 --- /dev/null +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/change-directory-modal.vue @@ -0,0 +1,228 @@ + + + diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/index.vue b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/index.vue new file mode 100644 index 00000000000..09dbaa2c4e1 --- /dev/null +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/index.vue @@ -0,0 +1,196 @@ + + + diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/paths.ts b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/paths.ts new file mode 100644 index 00000000000..a6ac2be1684 --- /dev/null +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/paths.ts @@ -0,0 +1,21 @@ +export function normalizePath(path: string, caseInsensitive: boolean): string { + const normalized = path.replace(/\\/g, '/').replace(/\/+$/, '') + return caseInsensitive ? normalized.toLowerCase() : normalized +} + +export function isNestedPath(first: string, second: string): boolean { + return first.startsWith(`${second}/`) || second.startsWith(`${first}/`) +} + +const DRIVE_ROOTS = [ + /^$/, + /^[a-z]:$/i, + /^\/Volumes\/[^/]+$/, + /^\/mnt\/[^/]+$/, + /^(\/run)?\/media\/[^/]+\/[^/]+$/, +] + +export function isDriveRoot(path: string): boolean { + const normalized = normalizePath(path, false) + return DRIVE_ROOTS.some((root) => root.test(normalized)) +} diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-app-directory.ts b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-app-directory.ts new file mode 100644 index 00000000000..0e46f1aa6a1 --- /dev/null +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-app-directory.ts @@ -0,0 +1,81 @@ +import { injectNotificationManager } from '@modrinth/ui' +import { useMutation, useQuery, useQueryClient } from '@tanstack/vue-query' +import { computed, watch } from 'vue' + +import { + appSettingsKeys, + appSettingsQueryOptions, + cancel_directory_change, + get, + set, +} from '@/helpers/settings' +import { restartApp } from '@/helpers/utils.js' +import { getDefaultAppDirectory } from '@/platform/app-lib/settings/commands' + +export function useAppDirectory() { + const { handleError } = injectNotificationManager() + const queryClient = useQueryClient() + const settingsQuery = useQuery(appSettingsQueryOptions()) + const defaultDirQuery = useQuery({ + queryKey: ['app-directory', 'default'], + networkMode: 'always', + queryFn: getDefaultAppDirectory, + staleTime: Infinity, + }) + + watch([settingsQuery.error, defaultDirQuery.error], ([settingsError, defaultDirError]) => { + const error = settingsError ?? defaultDirError + if (error) handleError(error) + }) + + const defaultDir = computed(() => defaultDirQuery.data.value ?? null) + + const currentDir = computed(() => { + const settings = settingsQuery.data.value + return settings?.prev_custom_dir ?? settings?.custom_dir ?? null + }) + + const pendingDir = computed(() => { + const settings = settingsQuery.data.value + if (!settings?.prev_custom_dir || !settings.custom_dir) return null + return settings.custom_dir === settings.prev_custom_dir ? null : settings.custom_dir + }) + + const settingsMutationOptions = { + networkMode: 'always', + mutationKey: appSettingsKeys.update, + scope: { id: 'app-settings' }, + onMutate: () => queryClient.cancelQueries({ queryKey: appSettingsKeys.all }), + onError: handleError, + onSettled: () => queryClient.invalidateQueries({ queryKey: appSettingsKeys.all }), + } as const + + const changeMutation = useMutation({ + ...settingsMutationOptions, + mutationFn: async ({ directory, restartNow }: { directory: string; restartNow: boolean }) => { + await set({ ...(await get()), custom_dir: directory }) + if (restartNow) await restartApp() + }, + }) + + const cancelMutation = useMutation({ + ...settingsMutationOptions, + mutationFn: () => cancel_directory_change(), + }) + + const busy = computed(() => changeMutation.isPending.value || cancelMutation.isPending.value) + + function changeDirectory(directory: string, restartNow: boolean) { + changeMutation.mutate({ directory, restartNow }) + } + + function cancelMove() { + cancelMutation.mutate() + } + + function restartNow() { + restartApp().catch(handleError) + } + + return { currentDir, defaultDir, pendingDir, busy, changeDirectory, cancelMove, restartNow } +} diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-copy-path.ts b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-copy-path.ts new file mode 100644 index 00000000000..061a72e5d92 --- /dev/null +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/app-directory/use-copy-path.ts @@ -0,0 +1,38 @@ +import { defineMessages, injectNotificationManager, useVIntl } from '@modrinth/ui' + +const messages = defineMessages({ + copiedTitle: { + id: 'app.settings.resource-management.app-directory.path-copied-title', + defaultMessage: 'Path copied', + }, + copiedText: { + id: 'app.settings.resource-management.app-directory.path-copied-text', + defaultMessage: 'The app directory path has been copied to your clipboard.', + }, + copyFailedTitle: { + id: 'app.settings.resource-management.app-directory.path-copy-failed-title', + defaultMessage: 'Failed to copy path', + }, +}) + +export function useCopyPath() { + const { formatMessage } = useVIntl() + const { addNotification } = injectNotificationManager() + + return async function copyPath(path: string) { + try { + await navigator.clipboard.writeText(path) + addNotification({ + type: 'success', + title: formatMessage(messages.copiedTitle), + text: formatMessage(messages.copiedText), + }) + } catch (error) { + addNotification({ + type: 'error', + title: formatMessage(messages.copyFailedTitle), + text: error instanceof Error ? error.message : String(error), + }) + } + } +} diff --git a/apps/app-frontend/src/features/settings/system/resource-management-settings/index.vue b/apps/app-frontend/src/features/settings/system/resource-management-settings/index.vue index eb12af28eb1..ae1fefeed1a 100644 --- a/apps/app-frontend/src/features/settings/system/resource-management-settings/index.vue +++ b/apps/app-frontend/src/features/settings/system/resource-management-settings/index.vue @@ -1,34 +1,19 @@