From 9702dae19dcba08d2b8f32d8028af747aefc4a13 Mon Sep 17 00:00:00 2001 From: Carter Date: Fri, 5 Jan 2024 11:00:08 -0800 Subject: [PATCH 1/2] Switch from stdout log to latest log MOD-595 (#964) * Switch from stdout log to latest log * remove std capture * Remove unused functions --- theseus/src/api/logs.rs | 8 -- theseus/src/launcher/mod.rs | 6 +- theseus/src/state/children.rs | 165 +----------------------- theseus_gui/src-tauri/src/api/logs.rs | 10 -- theseus_gui/src/helpers/logs.js | 4 - theseus_gui/src/pages/instance/Logs.vue | 30 +---- 6 files changed, 11 insertions(+), 212 deletions(-) diff --git a/theseus/src/api/logs.rs b/theseus/src/api/logs.rs index c3ab4cc92..6a1aa492c 100644 --- a/theseus/src/api/logs.rs +++ b/theseus/src/api/logs.rs @@ -241,14 +241,6 @@ pub async fn get_latest_log_cursor( get_generic_live_log_cursor(profile_path, "latest.log", cursor).await } -#[tracing::instrument] -pub async fn get_std_log_cursor( - profile_path: ProfilePathId, - cursor: u64, // 0 to start at beginning of file -) -> crate::Result { - get_generic_live_log_cursor(profile_path, "latest_stdout.log", cursor).await -} - #[tracing::instrument] pub async fn get_generic_live_log_cursor( profile_path: ProfilePathId, diff --git a/theseus/src/launcher/mod.rs b/theseus/src/launcher/mod.rs index 103caa845..1d670ba11 100644 --- a/theseus/src/launcher/mod.rs +++ b/theseus/src/launcher/mod.rs @@ -16,7 +16,7 @@ use daedalus as d; use daedalus::minecraft::{RuleAction, VersionInfo}; use st::Profile; use std::collections::HashMap; -use std::{process::Stdio, sync::Arc}; +use std::sync::Arc; use tokio::process::Command; use uuid::Uuid; @@ -511,9 +511,7 @@ pub async fn launch_minecraft( .into_iter() .collect::>(), ) - .current_dir(instance_path.clone()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); + .current_dir(instance_path.clone()); // CARGO-set DYLD_LIBRARY_PATH breaks Minecraft on macOS during testing on playground #[cfg(target_os = "macos")] diff --git a/theseus/src/state/children.rs b/theseus/src/state/children.rs index 0b474567f..76344987f 100644 --- a/theseus/src/state/children.rs +++ b/theseus/src/state/children.rs @@ -1,21 +1,12 @@ -use super::DirectoryInfo; use super::{Profile, ProfilePathId}; use chrono::{DateTime, Utc}; use serde::Deserialize; use serde::Serialize; -use std::path::Path; use std::{collections::HashMap, sync::Arc}; use sysinfo::PidExt; -use tokio::fs::File; -use tokio::io::AsyncBufReadExt; -use tokio::io::AsyncWriteExt; -use tokio::io::BufReader; use tokio::process::Child; -use tokio::process::ChildStderr; -use tokio::process::ChildStdout; use tokio::process::Command; use tokio::sync::RwLock; -use tracing::error; use crate::event::emit::emit_process; use crate::event::ProcessPayloadType; @@ -201,7 +192,6 @@ impl ChildType { pub struct MinecraftChild { pub uuid: Uuid, pub profile_relative_path: ProfilePathId, - pub output: Option, pub manager: Option>>, // None when future has completed and been handled pub current_child: Arc>, pub last_updated_playtime: DateTime, // The last time we updated the playtime for the associated profile @@ -281,44 +271,9 @@ impl Children { censor_strings: HashMap, ) -> crate::Result>> { // Takes the first element of the commands vector and spawns it - let mut child = mc_command.spawn().map_err(IOError::from)?; + let mc_proc = mc_command.spawn().map_err(IOError::from)?; - // Create std watcher threads for stdout and stderr - let log_path = DirectoryInfo::profile_logs_dir(&profile_relative_path) - .await? - .join("latest_stdout.log"); - let shared_output = - SharedOutput::build(&log_path, censor_strings).await?; - if let Some(child_stdout) = child.stdout.take() { - let stdout_clone = shared_output.clone(); - tokio::spawn(async move { - if let Err(e) = stdout_clone.read_stdout(child_stdout).await { - error!("Stdout process died with error: {}", e); - let _ = stdout_clone - .push_line(format!( - "Stdout process died with error: {}", - e - )) - .await; - } - }); - } - if let Some(child_stderr) = child.stderr.take() { - let stderr_clone = shared_output.clone(); - tokio::spawn(async move { - if let Err(e) = stderr_clone.read_stderr(child_stderr).await { - error!("Stderr process died with error: {}", e); - let _ = stderr_clone - .push_line(format!( - "Stderr process died with error: {}", - e - )) - .await; - } - }); - } - - let child = ChildType::TokioChild(child); + let child = ChildType::TokioChild(mc_proc); // Slots child into manager let pid = child.id().ok_or_else(|| { @@ -358,7 +313,6 @@ impl Children { let mchild = MinecraftChild { uuid, profile_relative_path, - output: Some(shared_output), current_child, manager, last_updated_playtime, @@ -449,7 +403,6 @@ impl Children { let mchild = MinecraftChild { uuid: cached_process.uuid, profile_relative_path: cached_process.profile_relative_path, - output: None, // No output for cached/rescued processes current_child, manager, last_updated_playtime, @@ -758,117 +711,3 @@ impl Default for Children { Self::new() } } - -// SharedOutput, a wrapper around a String that can be read from and written to concurrently -// Designed to be used with ChildStdout and ChildStderr in a tokio thread to have a simple String storage for the output of a child process -#[derive(Debug, Clone)] -pub struct SharedOutput { - log_file: Arc>, - censor_strings: HashMap, -} - -impl SharedOutput { - #[tracing::instrument(skip(censor_strings))] - async fn build( - log_file_path: &Path, - censor_strings: HashMap, - ) -> crate::Result { - // create log_file_path parent if it doesn't exist - let parent_folder = log_file_path.parent().ok_or_else(|| { - crate::ErrorKind::LauncherError(format!( - "Could not get parent folder of {:?}", - log_file_path - )) - })?; - tokio::fs::create_dir_all(parent_folder) - .await - .map_err(|e| IOError::with_path(e, parent_folder))?; - - Ok(SharedOutput { - log_file: Arc::new(RwLock::new( - File::create(log_file_path) - .await - .map_err(|e| IOError::with_path(e, log_file_path))?, - )), - censor_strings, - }) - } - - async fn read_stdout( - &self, - child_stdout: ChildStdout, - ) -> crate::Result<()> { - let mut buf_reader = BufReader::new(child_stdout); - let mut buf = Vec::new(); - - while buf_reader - .read_until(b'\n', &mut buf) - .await - .map_err(IOError::from)? - > 0 - { - let line = String::from_utf8_lossy(&buf).into_owned(); - let val_line = self.censor_log(line.clone()); - { - let mut log_file = self.log_file.write().await; - log_file - .write_all(val_line.as_bytes()) - .await - .map_err(IOError::from)?; - } - - buf.clear(); - } - Ok(()) - } - - async fn read_stderr( - &self, - child_stderr: ChildStderr, - ) -> crate::Result<()> { - let mut buf_reader = BufReader::new(child_stderr); - let mut buf = Vec::new(); - - // TODO: these can be asbtracted into noe function - while buf_reader - .read_until(b'\n', &mut buf) - .await - .map_err(IOError::from)? - > 0 - { - let line = String::from_utf8_lossy(&buf).into_owned(); - let val_line = self.censor_log(line.clone()); - { - let mut log_file = self.log_file.write().await; - log_file - .write_all(val_line.as_bytes()) - .await - .map_err(IOError::from)?; - } - - buf.clear(); - } - Ok(()) - } - - async fn push_line(&self, line: String) -> crate::Result<()> { - let val_line = self.censor_log(line.clone()); - { - let mut log_file = self.log_file.write().await; - log_file - .write_all(val_line.as_bytes()) - .await - .map_err(IOError::from)?; - } - - Ok(()) - } - - fn censor_log(&self, mut val: String) -> String { - for (find, replace) in &self.censor_strings { - val = val.replace(find, replace); - } - - val - } -} diff --git a/theseus_gui/src-tauri/src/api/logs.rs b/theseus_gui/src-tauri/src/api/logs.rs index cfea2efb3..d328aac4e 100644 --- a/theseus_gui/src-tauri/src/api/logs.rs +++ b/theseus_gui/src-tauri/src/api/logs.rs @@ -23,7 +23,6 @@ pub fn init() -> tauri::plugin::TauriPlugin { logs_delete_logs, logs_delete_logs_by_filename, logs_get_latest_log_cursor, - logs_get_std_log_cursor, ]) .build() } @@ -91,12 +90,3 @@ pub async fn logs_get_latest_log_cursor( ) -> Result { Ok(logs::get_latest_log_cursor(profile_path, cursor).await?) } - -/// Get live stdout log from a cursor -#[tauri::command] -pub async fn logs_get_std_log_cursor( - profile_path: ProfilePathId, - cursor: u64, // 0 to start at beginning of file -) -> Result { - Ok(logs::get_std_log_cursor(profile_path, cursor).await?) -} diff --git a/theseus_gui/src/helpers/logs.js b/theseus_gui/src/helpers/logs.js index f325a9f20..cd9fa70a0 100644 --- a/theseus_gui/src/helpers/logs.js +++ b/theseus_gui/src/helpers/logs.js @@ -54,7 +54,3 @@ export async function delete_logs(profilePath) { export async function get_latest_log_cursor(profilePath, cursor) { return await invoke('plugin:logs|logs_get_latest_log_cursor', { profilePath, cursor }) } -// For std log (from modrinth app written latest_stdout.log, contains stdout and stderr) -export async function get_std_log_cursor(profilePath, cursor) { - return await invoke('plugin:logs|logs_get_std_log_cursor', { profilePath, cursor }) -} diff --git a/theseus_gui/src/pages/instance/Logs.vue b/theseus_gui/src/pages/instance/Logs.vue index d39405fbc..3129e20cb 100644 --- a/theseus_gui/src/pages/instance/Logs.vue +++ b/theseus_gui/src/pages/instance/Logs.vue @@ -102,7 +102,7 @@ import { delete_logs_by_filename, get_logs, get_output_by_filename, - get_std_log_cursor, + get_latest_log_cursor, } from '@/helpers/logs.js' import { computed, nextTick, onBeforeUnmount, onMounted, onUnmounted, ref, watch } from 'vue' import dayjs from 'dayjs' @@ -139,7 +139,7 @@ const props = defineProps({ const currentLiveLog = ref(null) const currentLiveLogCursor = ref(0) -const emptyText = ['No live game detected.', 'Start your game to proceed'] +const emptyText = ['No live game detected.', 'Start your game to proceed.'] const logs = ref([]) await setLogs() @@ -223,7 +223,7 @@ async function getLiveStdLog() { if (uuids.length === 0) { returnValue = emptyText.join('\n') } else { - const logCursor = await get_std_log_cursor( + const logCursor = await get_latest_log_cursor( props.instance.path, currentLiveLogCursor.value ).catch(handleError) @@ -243,31 +243,15 @@ async function getLogs() { return (await get_logs(props.instance.path, true).catch(handleError)) .reverse() .filter( + // filter out latest_stdout.log or anything without .log in it (log) => log.filename !== 'latest_stdout.log' && log.filename !== 'latest_stdout' && - log.stdout !== '' + log.stdout !== '' && + log.filename.includes('.log') ) .map((log) => { - if (log.filename == 'latest.log') { - log.name = 'Latest Log' - } else { - let filename = log.filename.split('.')[0] - let day = dayjs(filename.slice(0, 10)) - if (day.isValid()) { - if (day.isToday()) { - log.name = 'Today' - } else if (day.isYesterday()) { - log.name = 'Yesterday' - } else { - log.name = day.format('MMMM D, YYYY') - } - // Displays as "Today-1", "Today-2", etc, matching minecraft log naming but with the date - log.name = log.name + filename.slice(10) - } else { - log.name = filename - } - } + log.name = log.filename || 'Unknown' log.stdout = 'Loading...' return log }) From 0d3f007dd4ab5b2fb6dadb09f073c28095b33e33 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Fri, 5 Jan 2024 11:00:48 -0800 Subject: [PATCH 2/2] Config transfer (#951) * fixed config dir issue * jackson's sync write --- theseus/src/api/pack/import/mod.rs | 2 +- theseus/src/api/settings.rs | 115 +++++++++++++++++++---------- theseus/src/state/dirs.rs | 6 +- theseus/src/util/io.rs | 39 ++++++++-- theseus_gui/src/pages/Settings.vue | 2 +- 5 files changed, 116 insertions(+), 48 deletions(-) diff --git a/theseus/src/api/pack/import/mod.rs b/theseus/src/api/pack/import/mod.rs index 1d3c71605..f4a384c9f 100644 --- a/theseus/src/api/pack/import/mod.rs +++ b/theseus/src/api/pack/import/mod.rs @@ -301,7 +301,7 @@ pub async fn copy_dotminecraft( #[theseus_macros::debug_pin] #[async_recursion::async_recursion] #[tracing::instrument] -async fn get_all_subfiles(src: &Path) -> crate::Result> { +pub async fn get_all_subfiles(src: &Path) -> crate::Result> { if !src.is_dir() { return Ok(vec![src.to_path_buf()]); } diff --git a/theseus/src/api/settings.rs b/theseus/src/api/settings.rs index 270a01680..89107ee17 100644 --- a/theseus/src/api/settings.rs +++ b/theseus/src/api/settings.rs @@ -1,6 +1,6 @@ //! Theseus profile management interface -use std::path::PathBuf; +use std::path::{PathBuf, Path}; use tokio::fs; use io::IOError; @@ -10,7 +10,7 @@ use crate::{ event::emit::{emit_loading, init_loading}, prelude::DirectoryInfo, state::{self, Profiles}, - util::io, + util::{io, fetch}, }; pub use crate::{ state::{ @@ -77,6 +77,7 @@ pub async fn set(settings: Settings) -> crate::Result<()> { /// Sets the new config dir, the location of all Theseus data except for the settings.json and caches /// Takes control of the entire state and blocks until completion pub async fn set_config_dir(new_config_dir: PathBuf) -> crate::Result<()> { + tracing::trace!("Changing config dir to: {}", new_config_dir.display()); if !new_config_dir.is_dir() { return Err(crate::ErrorKind::FSError(format!( "New config dir is not a folder: {}", @@ -85,6 +86,14 @@ pub async fn set_config_dir(new_config_dir: PathBuf) -> crate::Result<()> { .as_error()); } + if !is_dir_writeable(new_config_dir.clone()).await? { + return Err(crate::ErrorKind::FSError(format!( + "New config dir is not writeable: {}", + new_config_dir.display() + )) + .as_error()); + } + let loading_bar = init_loading( crate::LoadingBarType::ConfigChange { new_path: new_config_dir.clone(), @@ -100,6 +109,52 @@ pub async fn set_config_dir(new_config_dir: PathBuf) -> crate::Result<()> { let old_config_dir = state_write.directories.config_dir.read().await.clone(); + + // Reset file watcher + tracing::trace!("Reset file watcher"); + let file_watcher = state::init_watcher().await?; + state_write.file_watcher = RwLock::new(file_watcher); + + // Getting files to be moved + let mut config_entries = io::read_dir(&old_config_dir).await?; + let across_drives = is_different_drive(&old_config_dir, &new_config_dir); + let mut entries = vec![]; + let mut deletable_entries = vec![]; + while let Some(entry) = config_entries + .next_entry() + .await + .map_err(|e| IOError::with_path(e, &old_config_dir))? + { + + let entry_path = entry.path(); + if let Some(file_name) = entry_path.file_name() { + // We are only moving the profiles and metadata folders + if file_name == state::PROFILES_FOLDER_NAME || file_name == state::METADATA_FOLDER_NAME { + if across_drives { + entries.extend(crate::pack::import::get_all_subfiles(&entry_path).await?); + deletable_entries.push(entry_path.clone()); + } else { + entries.push(entry_path.clone()); + } + } + } + } + + tracing::trace!("Moving files"); + let semaphore = &state_write.io_semaphore; + let num_entries = entries.len() as f64; + for entry_path in entries { + let relative_path = entry_path.strip_prefix(&old_config_dir)?; + let new_path = new_config_dir.join(relative_path); + if across_drives { + fetch::copy(&entry_path, &new_path, semaphore).await?; + } else { + io::rename(entry_path.clone(), new_path.clone()).await?; + } + emit_loading(&loading_bar, 80.0 * (1.0 / num_entries), None) + .await?; + } + tracing::trace!("Setting configuration setting"); // Set load config dir setting let settings = { @@ -132,41 +187,19 @@ pub async fn set_config_dir(new_config_dir: PathBuf) -> crate::Result<()> { tracing::trace!("Reinitializing directory"); // Set new state information state_write.directories = DirectoryInfo::init(&settings)?; - let total_entries = std::fs::read_dir(&old_config_dir) - .map_err(|e| IOError::with_path(e, &old_config_dir))? - .count() as f64; - // Move all files over from state_write.directories.config_dir to new_config_dir - tracing::trace!("Renaming folder structure"); - let mut i = 0.0; - let mut entries = io::read_dir(&old_config_dir).await?; - while let Some(entry) = entries - .next_entry() - .await - .map_err(|e| IOError::with_path(e, &old_config_dir))? - { - let entry_path = entry.path(); - if let Some(file_name) = entry_path.file_name() { - // Ignore settings.json - if file_name == state::SETTINGS_FILE_NAME { - continue; - } - // Ignore caches folder - if file_name == state::CACHES_FOLDER_NAME { - continue; - } - // Ignore modrinth_logs folder - if file_name == state::LAUNCHER_LOGS_FOLDER_NAME { - continue; - } - - let new_path = new_config_dir.join(file_name); - io::rename(entry_path, new_path).await?; - - i += 1.0; - emit_loading(&loading_bar, 90.0 * (i / total_entries), None) - .await?; - } + // Delete entries that were from a different drive + let deletable_entries_len = deletable_entries.len(); + if deletable_entries_len > 0 { + tracing::trace!("Deleting old files"); + } + for entry in deletable_entries { + io::remove_dir_all(entry).await?; + emit_loading( + &loading_bar, + 10.0 * (1.0 / deletable_entries_len as f64), + None, + ).await?; } // Reset file watcher @@ -181,15 +214,21 @@ pub async fn set_config_dir(new_config_dir: PathBuf) -> crate::Result<()> { emit_loading(&loading_bar, 10.0, None).await?; - // TODO: need to be able to safely error out of this function, reverting the changes tracing::info!( "Successfully switched config folder to: {}", new_config_dir.display() ); - Ok(()) } +// Function to check if two paths are on different drives/roots +fn is_different_drive(path1: &Path, path2: &Path) -> bool { + let root1 = path1.components().next(); + let root2 = path2.components().next(); + root1 != root2 +} + + pub async fn is_dir_writeable(new_config_dir: PathBuf) -> crate::Result { let temp_path = new_config_dir.join(".tmp"); match fs::write(temp_path.clone(), "test").await { diff --git a/theseus/src/state/dirs.rs b/theseus/src/state/dirs.rs index 4d3a64608..365638cfe 100644 --- a/theseus/src/state/dirs.rs +++ b/theseus/src/state/dirs.rs @@ -9,6 +9,8 @@ use super::{ProfilePathId, Settings}; pub const SETTINGS_FILE_NAME: &str = "settings.json"; pub const CACHES_FOLDER_NAME: &str = "caches"; pub const LAUNCHER_LOGS_FOLDER_NAME: &str = "launcher_logs"; +pub const PROFILES_FOLDER_NAME: &str = "profiles"; +pub const METADATA_FOLDER_NAME: &str = "meta"; #[derive(Debug)] pub struct DirectoryInfo { @@ -75,7 +77,7 @@ impl DirectoryInfo { /// Get the Minecraft instance metadata directory #[inline] pub async fn metadata_dir(&self) -> PathBuf { - self.config_dir.read().await.join("meta") + self.config_dir.read().await.join(METADATA_FOLDER_NAME) } /// Get the Minecraft java versions metadata directory @@ -153,7 +155,7 @@ impl DirectoryInfo { /// Get the profiles directory for created profiles #[inline] pub async fn profiles_dir(&self) -> PathBuf { - self.config_dir.read().await.join("profiles") + self.config_dir.read().await.join(PROFILES_FOLDER_NAME) } /// Gets the logs dir for a given profile diff --git a/theseus/src/util/io.rs b/theseus/src/util/io.rs index 840130159..03484e987 100644 --- a/theseus/src/util/io.rs +++ b/theseus/src/util/io.rs @@ -1,7 +1,10 @@ // IO error // A wrapper around the tokio IO functions that adds the path to the error message, instead of the uninformative std::io::Error. -use std::path::Path; +use std::{path::Path, io::Write}; + +use tempfile::NamedTempFile; +use tauri::async_runtime::spawn_blocking; #[derive(Debug, thiserror::Error)] pub enum IOError { @@ -113,15 +116,39 @@ pub async fn write( path: impl AsRef, data: impl AsRef<[u8]>, ) -> Result<(), IOError> { - let path = path.as_ref(); - tokio::fs::write(path, data) - .await - .map_err(|e| IOError::IOPathError { + let path = path.as_ref().to_owned(); + let data = data.as_ref().to_owned(); + spawn_blocking(move || { + let cloned_path = path.clone(); + sync_write(data, path).map_err(|e| IOError::IOPathError { source: e, - path: path.to_string_lossy().to_string(), + path: cloned_path.to_string_lossy().to_string(), }) + }) + .await + .map_err(|_| { + std::io::Error::new(std::io::ErrorKind::Other, "background task failed") + })??; + + Ok(()) } +fn sync_write( + data: impl AsRef<[u8]>, + path: impl AsRef, +) -> Result<(), std::io::Error> { + let mut tempfile = NamedTempFile::new_in(path.as_ref().parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::Other, + "could not get parent directory for temporary file", + ) + })?)?; + tempfile.write_all(data.as_ref())?; + let tmp_path = tempfile.into_temp_path(); + let path = path.as_ref(); + tmp_path.persist(path)?; + std::io::Result::Ok(()) +} // rename pub async fn rename( from: impl AsRef, diff --git a/theseus_gui/src/pages/Settings.vue b/theseus_gui/src/pages/Settings.vue index 59ff2376b..0faeae113 100644 --- a/theseus_gui/src/pages/Settings.vue +++ b/theseus_gui/src/pages/Settings.vue @@ -139,7 +139,7 @@ async function findLauncherDir() { } async function refreshDir() { - await change_config_dir(settingsDir.value) + await change_config_dir(settingsDir.value).catch(handleError) settings.value = await accessSettings().catch(handleError) settingsDir.value = settings.value.loaded_config_dir }