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

@@ -2539,6 +2539,7 @@ dependencies = [
  "egui",
  "egui_extras",
  "exec",
+ "filetime",
  "flagset",
  "globset",
  "log",

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"

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::{
+    config::{build_globset, load_project_config, ProjectObject, ProjectObjectNode},
-        build_globset, load_project_config, ProjectObject, ProjectObjectNode, CONFIG_FILENAMES,
+    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<bool>,
 }
 
+#[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<PathBuf>,
     #[serde(default)]
     pub selected_obj: Option<ObjectConfig>,
+    #[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<ProjectConfigInfo>,
 }
 
 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<AtomicBool>,
-    config_modified: Arc<AtomicBool>,
     watcher: Option<notify::RecommendedWatcher>,
     relaunch_path: Rc<Mutex<Option<PathBuf>>>,
     should_relaunch: bool,
@@ -286,9 +300,11 @@ impl App {
         };
         let config = &mut *config;
 
-        if self.config_modified.swap(false, Ordering::Relaxed) {
+        if let Some(info) = &config.project_config_info {
+            if file_modified(&info.path, info.timestamp) {
                 config.config_change = true;
             }
+        }
 
         if config.config_change {
             config.config_change = false;
@@ -305,22 +321,15 @@ 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(
-                    match build_globset(&config.watch_patterns)
+                    |globset| {
+                        create_watcher(self.modified.clone(), project_dir, globset)
                             .map_err(anyhow::Error::new)
-                        .and_then(|globset| {
+                    },
-                            create_watcher(
+                ) {
-                                self.modified.clone(),
-                                self.config_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}"),
                 }
-                }
                 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<AtomicBool>,
-    config_modified: Arc<AtomicBool>,
     project_dir: &Path,
     patterns: GlobSet,
 ) -> notify::Result<notify::RecommendedWatcher> {
-    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<notify::Event>| 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) {
+                        if patterns.is_match(path) {
-                            config_modified.store(true, Ordering::Relaxed);
-                        } else 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
+    }
+}

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<String>,
+    #[serde(default)]
     pub custom_make: Option<String>,
+    #[serde(default)]
     pub target_dir: Option<PathBuf>,
+    #[serde(default)]
     pub base_dir: Option<PathBuf>,
+    #[serde(default = "bool_true")]
+    pub build_base: bool,
+    #[serde(default)]
     pub build_target: bool,
+    #[serde(default)]
     pub watch_patterns: Option<Vec<Glob>>,
-    #[serde(alias = "units")]
+    #[serde(default, alias = "units")]
     pub objects: Vec<ProjectObject>,
 }
 
 #[derive(Default, Clone, serde::Deserialize)]
 pub struct ProjectObject {
+    #[serde(default)]
     pub name: Option<String>,
+    #[serde(default)]
     pub path: Option<PathBuf>,
+    #[serde(default)]
     pub target_path: Option<PathBuf>,
+    #[serde(default)]
     pub base_path: Option<PathBuf>,
+    #[serde(default)]
     pub reverse_fn_order: Option<bool>,
+    #[serde(default)]
     pub complete: Option<bool>,
 }
 
@@ -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<Result<ProjectConfig>> {
+fn try_project_config(dir: &Path) -> Option<(Result<ProjectConfig>, ProjectConfigInfo)> {
     for filename in CONFIG_FILENAMES.iter() {
         let config_path = dir.join(filename);
-        if config_path.is_file() {
+        let Ok(mut file) = File::open(&config_path) else {
-            return match filename.contains("json") {
+            continue;
-                true => Some(read_json_config(&config_path)),
-                false => Some(read_yml_config(&config_path)),
         };
+        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<ProjectConfig> {
+fn read_yml_config<R: Read>(reader: &mut R) -> Result<ProjectConfig> {
-    let mut reader = File::open(config_path)
+    Ok(serde_yaml::from_reader(reader)?)
-        .with_context(|| format!("Failed to open config file '{}'", config_path.display()))?;
-    Ok(serde_yaml::from_reader(&mut reader)?)
 }
 
-fn read_json_config(config_path: &Path) -> Result<ProjectConfig> {
+fn read_json_config<R: Read>(reader: &mut R) -> Result<ProjectConfig> {
-    let mut reader = File::open(config_path)
+    Ok(serde_json::from_reader(reader)?)
-        .with_context(|| format!("Failed to open config file '{}'", config_path.display()))?;
-    Ok(serde_json::from_reader(&mut reader)?)
 }
 
 pub fn build_globset(vec: &[Glob]) -> std::result::Result<GlobSet, globset::Error> {

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<String>,
+    pub project_dir: Option<PathBuf>,
+    pub selected_obj: Option<ObjectConfig>,
+    pub selected_wsl_distro: Option<String>,
+}
+
+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<BuildStatus> {
         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<Box<ObjDiffResult>> {
-    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 {
+    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()),
-            1,
+                0,
                 total,
                 &cancel,
             )?;
             run_make(project_dir, base_path_rel, &config)
-    } else {
+        }
-        BuildStatus { success: true, log: String::new() }
+        _ => 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)))
     })

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<Option<BTreeMap<u32, u32>>> {
 }
 
 pub fn read(obj_path: &Path) -> Result<ObjInfo> {
-    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<ObjInfo> {
     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)?,

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<ObjSection>,
     pub common: Vec<ObjSymbol>,
     pub line_info: Option<BTreeMap<u32, u32>>,

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();
     }