From 04b4fdcd21c5b60e63ee5317800b7f0e426a06c8 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sat, 7 Oct 2023 14:48:34 -0400 Subject: [PATCH] Reload objects when changed externally Uses file modification timestamp polling for project config and objects to avoid unneeded complexity from the filesystem notification watcher. Allows disabling `build_base` as well for projects using an external build system. --- Cargo.lock | 1 + Cargo.toml | 1 + src/app.rs | 100 ++++++++++++++++++++++++++++---------------- src/config.rs | 65 +++++++++++++++++++--------- src/jobs/objdiff.rs | 63 ++++++++++++++++++++-------- src/obj/elf.rs | 7 +++- src/obj/mod.rs | 2 + src/views/config.rs | 27 +++++++++++- 8 files changed, 190 insertions(+), 76 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7afe5e4..59ac33b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2539,6 +2539,7 @@ dependencies = [ "egui", "egui_extras", "exec", + "filetime", "flagset", "globset", "log", diff --git a/Cargo.toml b/Cargo.toml index 9ed4efe..1935ea1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ dirs = "5.0.1" eframe = { version = "0.23.0", features = ["persistence"] } egui = "0.23.0" egui_extras = "0.23.0" +filetime = "0.2.22" flagset = "0.4.4" globset = { version = "0.4.13", features = ["serde1"] } log = "0.4.20" diff --git a/src/app.rs b/src/app.rs index f35d418..aa1a4f6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,5 +1,6 @@ use std::{ default::Default, + fs, path::{Path, PathBuf}, rc::Rc, sync::{ @@ -9,16 +10,18 @@ use std::{ time::Duration, }; -use globset::{Glob, GlobSet, GlobSetBuilder}; +use filetime::FileTime; +use globset::{Glob, GlobSet}; use notify::{RecursiveMode, Watcher}; use time::UtcOffset; use crate::{ app_config::{deserialize_config, AppConfigVersion}, - config::{ - build_globset, load_project_config, ProjectObject, ProjectObjectNode, CONFIG_FILENAMES, + config::{build_globset, load_project_config, ProjectObject, ProjectObjectNode}, + jobs::{ + objdiff::{start_build, ObjDiffConfig}, + Job, JobQueue, JobResult, JobStatus, }, - jobs::{objdiff::start_build, Job, JobQueue, JobResult, JobStatus}, views::{ appearance::{appearance_window, Appearance}, config::{config_ui, project_window, ConfigViewState, DEFAULT_WATCH_PATTERNS}, @@ -51,6 +54,12 @@ pub struct ObjectConfig { pub complete: Option, } +#[derive(Clone, Eq, PartialEq)] +pub struct ProjectConfigInfo { + pub path: PathBuf, + pub timestamp: FileTime, +} + #[inline] fn bool_true() -> bool { true } @@ -77,6 +86,8 @@ pub struct AppConfig { pub base_obj_dir: Option, #[serde(default)] pub selected_obj: Option, + #[serde(default = "bool_true")] + pub build_base: bool, #[serde(default)] pub build_target: bool, #[serde(default = "bool_true")] @@ -101,7 +112,9 @@ pub struct AppConfig { #[serde(skip)] pub queue_build: bool, #[serde(skip)] - pub project_config_loaded: bool, + pub queue_reload: bool, + #[serde(skip)] + pub project_config_info: Option, } impl Default for AppConfig { @@ -114,6 +127,7 @@ impl Default for AppConfig { target_obj_dir: None, base_obj_dir: None, selected_obj: None, + build_base: true, build_target: false, rebuild_on_changes: true, auto_update_check: true, @@ -125,7 +139,8 @@ impl Default for AppConfig { config_change: false, obj_change: false, queue_build: false, - project_config_loaded: false, + queue_reload: false, + project_config_info: None, } } } @@ -148,7 +163,7 @@ impl AppConfig { self.config_change = true; self.obj_change = true; self.queue_build = false; - self.project_config_loaded = false; + self.project_config_info = None; } pub fn set_target_obj_dir(&mut self, path: PathBuf) { @@ -180,7 +195,6 @@ pub struct App { view_state: ViewState, config: AppConfigRef, modified: Arc, - config_modified: Arc, watcher: Option, relaunch_path: Rc>>, should_relaunch: bool, @@ -286,8 +300,10 @@ impl App { }; let config = &mut *config; - if self.config_modified.swap(false, Ordering::Relaxed) { - config.config_change = true; + if let Some(info) = &config.project_config_info { + if file_modified(&info.path, info.timestamp) { + config.config_change = true; + } } if config.config_change { @@ -305,21 +321,14 @@ impl App { drop(self.watcher.take()); if let Some(project_dir) = &config.project_dir { - if !config.watch_patterns.is_empty() { - match build_globset(&config.watch_patterns) - .map_err(anyhow::Error::new) - .and_then(|globset| { - create_watcher( - self.modified.clone(), - self.config_modified.clone(), - project_dir, - globset, - ) + match build_globset(&config.watch_patterns).map_err(anyhow::Error::new).and_then( + |globset| { + create_watcher(self.modified.clone(), project_dir, globset) .map_err(anyhow::Error::new) - }) { - Ok(watcher) => self.watcher = Some(watcher), - Err(e) => log::error!("Failed to create watcher: {e}"), - } + }, + ) { + Ok(watcher) => self.watcher = Some(watcher), + Err(e) => log::error!("Failed to create watcher: {e}"), } config.watcher_change = false; } @@ -337,11 +346,32 @@ impl App { config.queue_build = true; } + if let Some(result) = &diff_state.build { + if let Some(obj) = &result.first_obj { + if file_modified(&obj.path, obj.timestamp) { + config.queue_reload = true; + } + } + if let Some(obj) = &result.second_obj { + if file_modified(&obj.path, obj.timestamp) { + config.queue_reload = true; + } + } + } + // Don't clear `queue_build` if a build is running. A file may have been modified during // the build, so we'll start another build after the current one finishes. if config.queue_build && config.selected_obj.is_some() && !jobs.is_running(Job::ObjDiff) { - jobs.push(start_build(self.config.clone())); + jobs.push(start_build(ObjDiffConfig::from_config(config))); config.queue_build = false; + config.queue_reload = false; + } else if config.queue_reload && !jobs.is_running(Job::ObjDiff) { + let mut diff_config = ObjDiffConfig::from_config(config); + // Don't build, just reload the current files + diff_config.build_base = false; + diff_config.build_target = false; + jobs.push(start_build(diff_config)); + config.queue_reload = false; } } } @@ -486,16 +516,9 @@ impl eframe::App for App { fn create_watcher( modified: Arc, - config_modified: Arc, project_dir: &Path, patterns: GlobSet, ) -> notify::Result { - let mut config_patterns = GlobSetBuilder::new(); - for filename in CONFIG_FILENAMES { - config_patterns.add(Glob::new(filename).unwrap()); - } - let config_patterns = config_patterns.build().unwrap(); - let base_dir = project_dir.to_owned(); let mut watcher = notify::recommended_watcher(move |res: notify::Result| match res { @@ -510,9 +533,7 @@ fn create_watcher( let Ok(path) = path.strip_prefix(&base_dir) else { continue; }; - if config_patterns.is_match(path) { - config_modified.store(true, Ordering::Relaxed); - } else if patterns.is_match(path) { + if patterns.is_match(path) { modified.store(true, Ordering::Relaxed); } } @@ -523,3 +544,12 @@ fn create_watcher( watcher.watch(project_dir, RecursiveMode::Recursive)?; Ok(watcher) } + +#[inline] +fn file_modified(path: &Path, last_ts: FileTime) -> bool { + if let Ok(metadata) = fs::metadata(path) { + FileTime::from_last_modification_time(&metadata) != last_ts + } else { + false + } +} diff --git a/src/config.rs b/src/config.rs index abbfb22..9a514a3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,33 +1,54 @@ use std::{ fs::File, + io::Read, path::{Component, Path, PathBuf}, }; -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; +use filetime::FileTime; use globset::{Glob, GlobSet, GlobSetBuilder}; -use crate::{app::AppConfig, views::config::DEFAULT_WATCH_PATTERNS}; +use crate::{ + app::{AppConfig, ProjectConfigInfo}, + views::config::DEFAULT_WATCH_PATTERNS, +}; + +#[inline] +fn bool_true() -> bool { true } #[derive(Default, Clone, serde::Deserialize)] -#[serde(default)] pub struct ProjectConfig { + #[serde(default)] pub min_version: Option, + #[serde(default)] pub custom_make: Option, + #[serde(default)] pub target_dir: Option, + #[serde(default)] pub base_dir: Option, + #[serde(default = "bool_true")] + pub build_base: bool, + #[serde(default)] pub build_target: bool, + #[serde(default)] pub watch_patterns: Option>, - #[serde(alias = "units")] + #[serde(default, alias = "units")] pub objects: Vec, } #[derive(Default, Clone, serde::Deserialize)] pub struct ProjectObject { + #[serde(default)] pub name: Option, + #[serde(default)] pub path: Option, + #[serde(default)] pub target_path: Option, + #[serde(default)] pub base_path: Option, + #[serde(default)] pub reverse_fn_order: Option, + #[serde(default)] pub complete: Option, } @@ -120,7 +141,7 @@ pub fn load_project_config(config: &mut AppConfig) -> Result<()> { let Some(project_dir) = &config.project_dir else { return Ok(()); }; - if let Some(result) = try_project_config(project_dir) { + if let Some((result, info)) = try_project_config(project_dir) { let project_config = result?; if let Some(min_version) = &project_config.min_version { let version_str = env!("CARGO_PKG_VERSION"); @@ -133,6 +154,7 @@ pub fn load_project_config(config: &mut AppConfig) -> Result<()> { config.custom_make = project_config.custom_make; config.target_obj_dir = project_config.target_dir.map(|p| project_dir.join(p)); config.base_obj_dir = project_config.base_dir.map(|p| project_dir.join(p)); + config.build_base = project_config.build_base; config.build_target = project_config.build_target; config.watch_patterns = project_config.watch_patterns.unwrap_or_else(|| { DEFAULT_WATCH_PATTERNS.iter().map(|s| Glob::new(s).unwrap()).collect() @@ -141,34 +163,39 @@ pub fn load_project_config(config: &mut AppConfig) -> Result<()> { config.objects = project_config.objects; config.object_nodes = build_nodes(&config.objects, project_dir, &config.target_obj_dir, &config.base_obj_dir); - config.project_config_loaded = true; + config.project_config_info = Some(info); } Ok(()) } -fn try_project_config(dir: &Path) -> Option> { +fn try_project_config(dir: &Path) -> Option<(Result, ProjectConfigInfo)> { for filename in CONFIG_FILENAMES.iter() { let config_path = dir.join(filename); - if config_path.is_file() { - return match filename.contains("json") { - true => Some(read_json_config(&config_path)), - false => Some(read_yml_config(&config_path)), + let Ok(mut file) = File::open(&config_path) else { + continue; + }; + let metadata = file.metadata(); + if let Ok(metadata) = metadata { + if !metadata.is_file() { + continue; + } + let ts = FileTime::from_last_modification_time(&metadata); + let config = match filename.contains("json") { + true => read_json_config(&mut file), + false => read_yml_config(&mut file), }; + return Some((config, ProjectConfigInfo { path: config_path, timestamp: ts })); } } None } -fn read_yml_config(config_path: &Path) -> Result { - let mut reader = File::open(config_path) - .with_context(|| format!("Failed to open config file '{}'", config_path.display()))?; - Ok(serde_yaml::from_reader(&mut reader)?) +fn read_yml_config(reader: &mut R) -> Result { + Ok(serde_yaml::from_reader(reader)?) } -fn read_json_config(config_path: &Path) -> Result { - let mut reader = File::open(config_path) - .with_context(|| format!("Failed to open config file '{}'", config_path.display()))?; - Ok(serde_json::from_reader(&mut reader)?) +fn read_json_config(reader: &mut R) -> Result { + Ok(serde_json::from_reader(reader)?) } pub fn build_globset(vec: &[Glob]) -> std::result::Result { diff --git a/src/jobs/objdiff.rs b/src/jobs/objdiff.rs index 54e3c36..39c4313 100644 --- a/src/jobs/objdiff.rs +++ b/src/jobs/objdiff.rs @@ -1,10 +1,15 @@ -use std::{path::Path, process::Command, str::from_utf8, sync::mpsc::Receiver}; +use std::{ + path::{Path, PathBuf}, + process::Command, + str::from_utf8, + sync::mpsc::Receiver, +}; use anyhow::{anyhow, Context, Error, Result}; use time::OffsetDateTime; use crate::{ - app::{AppConfig, AppConfigRef}, + app::{AppConfig, ObjectConfig}, diff::diff_objs, jobs::{start_job, update_status, Job, JobResult, JobState, JobStatusRef}, obj::{elf, ObjInfo}, @@ -15,6 +20,28 @@ pub struct BuildStatus { pub log: String, } +pub struct ObjDiffConfig { + pub build_base: bool, + pub build_target: bool, + pub custom_make: Option, + pub project_dir: Option, + pub selected_obj: Option, + pub selected_wsl_distro: Option, +} + +impl ObjDiffConfig { + pub(crate) fn from_config(config: &AppConfig) -> Self { + ObjDiffConfig { + build_base: config.build_base, + build_target: config.build_target, + custom_make: config.custom_make.clone(), + project_dir: config.project_dir.clone(), + selected_obj: config.selected_obj.clone(), + selected_wsl_distro: config.selected_wsl_distro.clone(), + } + } +} + pub struct ObjDiffResult { pub first_status: BuildStatus, pub second_status: BuildStatus, @@ -23,7 +50,7 @@ pub struct ObjDiffResult { pub time: OffsetDateTime, } -fn run_make(cwd: &Path, arg: &Path, config: &AppConfig) -> BuildStatus { +fn run_make(cwd: &Path, arg: &Path, config: &ObjDiffConfig) -> BuildStatus { match (|| -> Result { let make = config.custom_make.as_deref().unwrap_or("make"); #[cfg(not(windows))] @@ -73,9 +100,8 @@ fn run_make(cwd: &Path, arg: &Path, config: &AppConfig) -> BuildStatus { fn run_build( status: &JobStatusRef, cancel: Receiver<()>, - config: AppConfigRef, + config: ObjDiffConfig, ) -> Result> { - let config = config.read().map_err(|_| Error::msg("Failed to lock app config"))?.clone(); let obj_config = config.selected_obj.as_ref().ok_or_else(|| Error::msg("Missing obj path"))?; let project_dir = config.project_dir.as_ref().ok_or_else(|| Error::msg("Missing project dir"))?; @@ -106,7 +132,7 @@ fn run_build( if config.build_target && target_path_rel.is_some() { total += 1; } - if base_path_rel.is_some() { + if config.build_base && base_path_rel.is_some() { total += 1; } let first_status = match target_path_rel { @@ -123,17 +149,18 @@ fn run_build( _ => BuildStatus { success: true, log: String::new() }, }; - let second_status = if let Some(base_path_rel) = base_path_rel { - update_status( - status, - format!("Building base {}", base_path_rel.display()), - 1, - total, - &cancel, - )?; - run_make(project_dir, base_path_rel, &config) - } else { - BuildStatus { success: true, log: String::new() } + let second_status = match base_path_rel { + Some(base_path_rel) if config.build_base => { + update_status( + status, + format!("Building base {}", base_path_rel.display()), + 0, + total, + &cancel, + )?; + run_make(project_dir, base_path_rel, &config) + } + _ => BuildStatus { success: true, log: String::new() }, }; let time = OffsetDateTime::now_utc(); @@ -179,7 +206,7 @@ fn run_build( Ok(Box::new(ObjDiffResult { first_status, second_status, first_obj, second_obj, time })) } -pub fn start_build(config: AppConfigRef) -> JobState { +pub fn start_build(config: ObjDiffConfig) -> JobState { start_job("Object diff", Job::ObjDiff, move |status, cancel| { run_build(status, cancel, config).map(|result| JobResult::ObjDiff(Some(result))) }) diff --git a/src/obj/elf.rs b/src/obj/elf.rs index 11b205d..caedeea 100644 --- a/src/obj/elf.rs +++ b/src/obj/elf.rs @@ -3,6 +3,7 @@ use std::{collections::BTreeMap, fs, io::Cursor, path::Path}; use anyhow::{anyhow, bail, Context, Result}; use byteorder::{BigEndian, ReadBytesExt}; use cwdemangle::demangle; +use filetime::FileTime; use flagset::Flags; use object::{ elf, Architecture, File, Object, ObjectSection, ObjectSymbol, RelocationKind, RelocationTarget, @@ -304,9 +305,10 @@ fn line_info(obj_file: &File<'_>) -> Result>> { } pub fn read(obj_path: &Path) -> Result { - let data = { + let (data, timestamp) = { let file = fs::File::open(obj_path)?; - unsafe { memmap2::Mmap::map(&file) }? + let timestamp = FileTime::from_last_modification_time(&file.metadata()?); + (unsafe { memmap2::Mmap::map(&file) }?, timestamp) }; let obj_file = File::parse(&*data)?; let architecture = match obj_file.architecture() { @@ -322,6 +324,7 @@ pub fn read(obj_path: &Path) -> Result { let mut result = ObjInfo { architecture, path: obj_path.to_owned(), + timestamp, sections: filter_sections(&obj_file)?, common: common_symbols(&obj_file)?, line_info: line_info(&obj_file)?, diff --git a/src/obj/mod.rs b/src/obj/mod.rs index 9a658da..e64c416 100644 --- a/src/obj/mod.rs +++ b/src/obj/mod.rs @@ -4,6 +4,7 @@ pub mod ppc; use std::{collections::BTreeMap, path::PathBuf}; +use filetime::FileTime; use flagset::{flags, FlagSet}; #[derive(Debug, Eq, PartialEq, Copy, Clone)] @@ -144,6 +145,7 @@ pub enum ObjArchitecture { pub struct ObjInfo { pub architecture: ObjArchitecture, pub path: PathBuf, + pub timestamp: FileTime, pub sections: Vec, pub common: Vec, pub line_info: Option>, diff --git a/src/views/config.rs b/src/views/config.rs index 974315e..c4a1d91 100644 --- a/src/views/config.rs +++ b/src/views/config.rs @@ -616,7 +616,7 @@ fn split_obj_config_ui( ui.label(job); }, appearance, - !config.project_config_loaded, + config.project_config_info.is_none(), ); if response.clicked() { if let Some(path) = rfd::FileDialog::new().set_directory(&project_dir).pick_folder() { @@ -666,13 +666,36 @@ fn split_obj_config_ui( ui.label(job); }, appearance, - !config.project_config_loaded, + config.project_config_info.is_none(), ); if response.clicked() { if let Some(path) = rfd::FileDialog::new().set_directory(&project_dir).pick_folder() { config.set_base_obj_dir(path); } } + ui.checkbox(&mut config.build_base, "Build base objects").on_hover_ui(|ui| { + let mut job = LayoutJob::default(); + job.append( + "Tells the build system to produce the base object.\n", + 0.0, + text_format.clone(), + ); + job.append("For example, this would call ", 0.0, text_format.clone()); + job.append("make path/to/base.o", 0.0, code_format.clone()); + job.append(".\n\n", 0.0, text_format.clone()); + job.append( + "This can be disabled if you're running the build system\n", + 0.0, + text_format.clone(), + ); + job.append( + "externally, and just want objdiff to reload the files\n", + 0.0, + text_format.clone(), + ); + job.append("when they change.", 0.0, text_format.clone()); + ui.label(job); + }); ui.separator(); }