From a1cc4e086dfb569d60076e474128e894690c5dd3 Mon Sep 17 00:00:00 2001 From: JIMMY_JFEXP <108483685+Kelorastus@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:49:53 +0200 Subject: [PATCH] Marketplace Finish: repair sets whose marker says complete but files are missing "2 chosen maps are only partly downloaded (Population density, Yandex satellite imagery). Finish" - and Finish did nothing, silently. Root cause: the banner and the button asked two different questions. market::unfinished counts a set as unfinished when check_installed's file count found it short (launcher.log: "popdark says it is installed but holds 293056 of 400268 files", "yandex ... 2371 of 28213"). Finish went through apply_market, whose market::to_add only reads the markers - .blocks-done.json listing every block - so it found nothing to add, nothing to remove, and returned Ok. And even a forced install could not have repaired it: the installer skips every block the marker lists (install.rs `pending`). - odian-packs: InstallOptions::recheck_tile_sets. For those sets install_tiles counts the tiles on disk per block (tiles_on_disk_per_block: z/x/y.ext names only, so .blocks-done.json and the tile server's .missing negatives never count), takes off `done` every block holding fewer than the index names (forget_short_blocks - more is still whole: a PC bakes its own tiles in), rewrites the marker at once so a stopped run reads as partial, and fetches just those blocks; finish_block's rename replaces what is there. The drive-copy path skips its "recorded complete" shortcut for them too. - launcher: Finish passes market::short_tile_sets(market_checked) through Inner::market_recheck (taken by install_market_items), apply_market runs when a re-check is all there is to do, and the log says which sets are repaired. Counted, not hashed: the index names a block's tiles only by digest, so a block whose lost tiles were replaced by as many baked ones reads whole. Spot-check: odian-packs install::tests (a short block is dropped, a whole one and one with extra baked tiles kept, markers not counted); launcher market::tests::finish_repairs_the_sets_a_count_found_short with the counts from the real log. --- crates/odian-packs/src/install.rs | 124 ++++++++++++++++++++++++++++-- src-tauri/src/launcher.rs | 53 ++++++++++--- src-tauri/src/market.rs | 26 +++++++ 3 files changed, 188 insertions(+), 15 deletions(-) diff --git a/crates/odian-packs/src/install.rs b/crates/odian-packs/src/install.rs index ae1c8cd..991f16f 100644 --- a/crates/odian-packs/src/install.rs +++ b/crates/odian-packs/src/install.rs @@ -36,11 +36,25 @@ pub struct InstallOptions { pub tile_sets: Option>, /// The source files to install, by manifest path; `None` is all of them. pub src_paths: Option>, + /// Tile sets (`style/dataset`) whose `.blocks-done.json` is NOT to be taken at its word: + /// each block it lists is counted on disk first, and one holding fewer tiles than the + /// index names is fetched again. For a set a count has already found short - its marker + /// says every block landed, the folder says otherwise (tiles deleted by hand, a disk + /// cleaner, a copy that lost files) - which without this no install could ever repair, + /// since the marker is exactly what an install skips on. + pub recheck_tile_sets: BTreeSet, } impl Default for InstallOptions { fn default() -> Self { - InstallOptions { workers: 6, only: vec![Stage::App, Stage::Db, Stage::Src, Stage::Tiles], db_paths: None, tile_sets: None, src_paths: None } + InstallOptions { + workers: 6, + only: vec![Stage::App, Stage::Db, Stage::Src, Stage::Tiles], + db_paths: None, + tile_sets: None, + src_paths: None, + recheck_tile_sets: BTreeSet::new(), + } } } @@ -179,7 +193,7 @@ where stage_index += 1; let keep = |s: &&TileSet| opts.tile_sets.as_ref().is_none_or(|sets| sets.contains(&format!("{}/{}", s.style, s.dataset))); let tiles = TilesSection { sets: tiles.sets.iter().filter(keep).cloned().collect(), ..tiles.clone() }; - summary.tiles = Some(install_tiles(&ctx, stage_index, &version, &tiles, &data_dir).await?); + summary.tiles = Some(install_tiles(&ctx, stage_index, &version, &tiles, &data_dir, &opts.recheck_tile_sets).await?); } check_cancel()?; @@ -603,6 +617,7 @@ async fn copy_install_tiles( tiles: &TilesSection, root: &Path, data_dir: &Path, + recheck: &BTreeSet, ) -> Result> { let total: u64 = tiles.sets.iter().map(|s| s.bytes).sum(); let files: u64 = tiles.sets.iter().map(|s| s.files).sum(); @@ -615,8 +630,10 @@ async fn copy_install_tiles( // This PC already recorded the very same set as complete: `.blocks-done.json` names // this index and all of its blocks, the record the marketplace itself reads to say // "Installed". Hashing its hundreds of thousands of files to learn what the marker - // already states costs minutes a set and can only ever conclude "identical". - if set_recorded_complete(&to, set) { + // already states costs minutes a set and can only ever conclude "identical" - unless + // the caller has counted the set short, when the copy below (which leaves identical + // files alone) is the repair. + if set_recorded_complete(&to, set) && !recheck.contains(&format!("{style}/{dataset}")) { ctx.reporter.begin_sub(format!("tiles {style}/{dataset}"), "files", set.files); ctx.reporter.items_done(set.files); ctx.reporter.add_done_bytes(set.bytes); @@ -753,10 +770,59 @@ fn copy_checked(from: &Path, to: &Path, reporter: &Reporter, cancel: &Cancellati Ok((size, true)) } -async fn install_tiles(ctx: &Ctx<'_>, stage_index: u32, version: &str, tiles: &TilesSection, data_dir: &Path) -> Result> { +/// How many tiles `set_dir` holds, per block. Only names the installer itself would write +/// count (`z/x/y.ext`, `parse_tile_name`'s rule), so the markers beside them - its own +/// `.blocks-done.json`, the tile server's `.missing` negatives - never pass for tiles. +fn tiles_on_disk_per_block(set_dir: &Path) -> HashMap<(u64, u64, u64), u64> { + let mut counts = HashMap::new(); + let Ok(zooms) = std::fs::read_dir(set_dir) else { return counts }; + for zoom in zooms.flatten() { + let Ok(columns) = std::fs::read_dir(zoom.path()) else { continue }; + let z = zoom.file_name().to_string_lossy().into_owned(); + for column in columns.flatten() { + let Ok(rows) = std::fs::read_dir(column.path()) else { continue }; + let x = column.file_name().to_string_lossy().into_owned(); + for row in rows.flatten() { + if !row.file_type().is_ok_and(|kind| kind.is_file()) { + continue; + } + let name = format!("{z}/{x}/{}", row.file_name().to_string_lossy()); + if let Some((z, x, y, _)) = crate::tiles::parse_tile_name(&name) { + *counts.entry(crate::tiles::block_of(z, x, y)).or_default() += 1; + } + } + } + } + counts +} + +/// Take off `done` every block the disk holds fewer tiles of than the index names, and say +/// how many. A block holding MORE is whole: a PC bakes tiles of its own into the same folders. +/// Counted, not hashed - the index names a block's tiles only through their digest - so a +/// block whose missing tiles were replaced by as many baked ones reads whole; what a count +/// cannot see, a hash of every tile of the set could, at minutes a set. +fn forget_short_blocks(done: &mut BTreeSet, blocks: &[IndexBlock], on_disk: &HashMap<(u64, u64, u64), u64>) -> usize { + let mut dropped = 0; + for block in blocks { + let held = on_disk.get(&(block.z, block.bx, block.by)).copied().unwrap_or(0); + if held < block.files && done.remove(&block.done_key()) { + dropped += 1; + } + } + dropped +} + +async fn install_tiles( + ctx: &Ctx<'_>, + stage_index: u32, + version: &str, + tiles: &TilesSection, + data_dir: &Path, + recheck: &BTreeSet, +) -> Result> { if let Some(root) = ctx.backend.install_root() { let root = root.to_path_buf(); - return copy_install_tiles(ctx, stage_index, tiles, &root, data_dir).await; + return copy_install_tiles(ctx, stage_index, tiles, &root, data_dir, recheck).await; } let total: u64 = tiles.sets.iter().map(|s| s.bytes).sum(); let blocks: u64 = tiles.sets.iter().map(|s| s.blocks).sum(); @@ -787,6 +853,16 @@ async fn install_tiles(ctx: &Ctx<'_>, stage_index: u32, version: &str, tiles: &T done.extend(list.iter().filter_map(Value::as_str).map(str::to_string)); } } + if !done.is_empty() && recheck.contains(&format!("{style}/{dataset}")) { + ctx.reporter.begin_sub(format!("tiles {style}/{dataset}: counting what is on disk"), "", 0); + let dir = set_dir.clone(); + let on_disk = blocking(move || Ok(tiles_on_disk_per_block(&dir))).await?; + if forget_short_blocks(&mut done, &index.blocks, &on_disk) > 0 { + // Written now, not when the blocks land: a run stopped before then must leave + // a set that reads as partial, so the next one resumes instead of trusting it. + atomic_write(&done_path, blocks_done_json(&set.index_sha256, &done).as_bytes())?; + } + } let pending: Vec = index.blocks.iter().filter(|b| !done.contains(&b.done_key())).cloned().collect(); ctx.reporter.begin_sub(format!("tiles {style}/{dataset}"), "blocks", index.blocks.len() as u64); let skipped_bytes: u64 = index.blocks.iter().filter(|b| done.contains(&b.done_key())).map(|b| b.bytes).sum(); @@ -854,6 +930,42 @@ async fn install_tiles(ctx: &Ctx<'_>, stage_index: u32, version: &str, tiles: &T mod tests { use super::*; + fn block(z: u64, bx: u64, by: u64, files: u64, digest: &str) -> IndexBlock { + IndexBlock { z, bx, by, files, bytes: files * 10, digest: digest.to_string() } + } + + #[test] + fn a_block_the_disk_holds_short_of_is_fetched_again() { + let dir = std::env::temp_dir().join(format!("odian-recheck-{}", crate::copy::unique_hex())); + // Block (8, 2, 1) is x 128..=191, y 64..=127: two of its three tiles are here, plus a + // tile server negative and the installer's marker, neither of which is a tile. + // Block (8, 0, 0) has its one tile and one this PC baked itself: more is still whole. + for rel in ["8/128/64.png", "8/129/64.png", "8/129/65.png.missing", "8/1/1.png", "8/2/2.png"] { + let path = dir.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, b"t").unwrap(); + } + std::fs::write(dir.join(DONE_MARKER), b"{}").unwrap(); + + let on_disk = tiles_on_disk_per_block(&dir); + assert_eq!(on_disk.get(&(8, 2, 1)), Some(&2)); + assert_eq!(on_disk.get(&(8, 0, 0)), Some(&2)); + + let blocks = [block(8, 2, 1, 3, "aa"), block(8, 0, 0, 1, "bb"), block(9, 0, 0, 4, "cc")]; + let mut done: BTreeSet = blocks.iter().map(IndexBlock::done_key).collect(); + // The short block and the one with nothing on disk go; the whole one stays. + assert_eq!(forget_short_blocks(&mut done, &blocks, &on_disk), 2); + assert_eq!(done, ["8/0/0:bb".to_string()].into_iter().collect::>()); + // A block that was never marked done is not "forgotten": it was already pending. + assert_eq!(forget_short_blocks(&mut done, &blocks, &on_disk), 0); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn an_empty_or_missing_folder_counts_nothing() { + assert!(tiles_on_disk_per_block(Path::new(r"Z:\no\such\set")).is_empty()); + } + #[test] fn progress_files_match_python_json_dumps() { let verified: BTreeSet = [2, 0, 1].into_iter().collect(); diff --git a/src-tauri/src/launcher.rs b/src-tauri/src/launcher.rs index 1617356..ae7f42d 100644 --- a/src-tauri/src/launcher.rs +++ b/src-tauri/src/launcher.rs @@ -190,6 +190,10 @@ struct Inner { dev_refusal: Option, /// What the last count found on disk, kept for as long as nothing can have changed it. market_checked: Option>, + /// Tile sets (`style/dataset`) the next marketplace download re-counts block by block + /// instead of trusting their markers: the ones `market_checked` found short, handed over by + /// Finish and taken by `install_market_items`. + market_recheck: BTreeSet, /// Read once at start: this launcher ships on an ODIAN drive (`settings::portable_root`). portable: bool, /// A newer launcher is being checked for or downloaded (see `update_self`): nothing may @@ -444,7 +448,7 @@ impl Launcher { .map_err(|e| format!("cannot start the HTTP client: {e}"))?; Ok(Launcher { http, - inner: Mutex::new(Inner { settings, state, online: false, available: None, token, phase: Phase::Checking, error: None, sync: None, engine_url, live: None, map_from_site: None, map_problem: None, tiles_waiting: None, unfinished: None, dev: None, dev_refusal: None, market_checked: None, portable: crate::settings::portable_drive().is_some(), updating_self: false, offline_reason: None, drive_extras: None }), + inner: Mutex::new(Inner { settings, state, online: false, available: None, token, phase: Phase::Checking, error: None, sync: None, engine_url, live: None, map_from_site: None, map_problem: None, tiles_waiting: None, unfinished: None, dev: None, dev_refusal: None, market_checked: None, market_recheck: BTreeSet::new(), portable: crate::settings::portable_drive().is_some(), updating_self: false, offline_reason: None, drive_extras: None }), stack: Mutex::new(None), stack_states: std::sync::Mutex::new(Vec::new()), sign_in_cancel: Notify::new(), @@ -818,11 +822,19 @@ impl Launcher { /// Download what was chosen and never fully arrived, without changing the choice and /// without removing anything: the selection it applies is everything this PC holds or /// asked for, so `apply_market` finds nothing to delete and only fills the gaps. + /// + /// "Never fully arrived" is also a set whose marker says every block landed while the count + /// found files missing: the hub counts those as unfinished (`market::unfinished`), so Finish + /// must fetch them too. They go down as sets to re-count block by block - the installer + /// skips on that very marker, and without this Finish found nothing to add and did nothing. pub async fn finish_market(&self, app: &AppHandle) -> Result<(), String> { let (manifest, _, install_dir) = self.market_manifest(app).await?; - let extras = self.inner.lock().await.state.extras.clone().unwrap_or_default(); + let (extras, short) = { + let inner = self.inner.lock().await; + (inner.state.extras.clone().unwrap_or_default(), market::short_tile_sets(inner.market_checked.as_deref().unwrap_or_default())) + }; let keep = market::held_keys(&manifest, &install_dir.join("data"), &extras); - self.apply_market(app, keep.into_iter().collect()).await + self.apply_market_rechecking(app, keep.into_iter().collect(), short).await } /// Ask the site what it has baked since this PC last looked. Quiet on failure: a site @@ -1583,6 +1595,11 @@ impl Launcher { } pub async fn apply_market(&self, app: &AppHandle, selected: Vec) -> Result<(), String> { + self.apply_market_rechecking(app, selected, BTreeSet::new()).await + } + + /// `apply_market`, also repairing `recheck` (`style/dataset`): sets counted short on disk. + async fn apply_market_rechecking(&self, app: &AppHandle, selected: Vec, recheck: BTreeSet) -> Result<(), String> { let (manifest, installed, install_dir) = self.market_manifest(app).await?; let extras = market::optional_keys(&manifest, selected); { @@ -1598,13 +1615,17 @@ impl Launcher { } let data = install_dir.join("data"); let adds = market::to_add(&manifest, &data, &extras); - if !installed || (adds.is_empty() && market::to_remove(&manifest, &data, &extras).is_empty()) { + // Only a set still chosen and still in this version is worth re-counting. + let (wanted, _) = market::install_filter(&manifest, &extras, true); + let recheck: BTreeSet = recheck.into_iter().filter(|set| wanted.contains(set)).collect(); + if !installed || (adds.is_empty() && recheck.is_empty() && market::to_remove(&manifest, &data, &extras).is_empty()) { self.emit(app).await; return Ok(()); } - if !adds.is_empty() && !self.inner.lock().await.online { + if (!adds.is_empty() || !recheck.is_empty()) && !self.inner.lock().await.online { return Err("Your DHP is unreachable: connect to download marketplace items.".to_string()); } + self.inner.lock().await.market_recheck = recheck; self.start_run(app, None, true).await } @@ -1675,21 +1696,35 @@ impl Launcher { install_dir: &Path, cancel: &CancellationToken, ) -> Result { - let (installed, extras) = { - let inner = self.inner.lock().await; - (inner.state.installed_version.clone(), inner.state.extras.clone().unwrap_or_default()) + let (installed, extras, recheck) = { + let mut inner = self.inner.lock().await; + // Taken, not read: a later marketplace download re-counts only if Finish asks again. + (inner.state.installed_version.clone(), inner.state.extras.clone().unwrap_or_default(), std::mem::take(&mut inner.market_recheck)) }; let installed = installed.ok_or_else(|| "ODIAN is not installed.".to_string())?; let verified = odian_packs::version_manifest(source, &installed, odian_packs::bundled_public_key()).await?; let manifest = verified.manifest(); let titles: Vec = market::to_add(manifest, &install_dir.join("data"), &extras).into_iter().map(|item| item.title).collect(); - self.log(app, "info", format!("Downloading {}", titles.join(", "))); + if !titles.is_empty() { + self.log(app, "info", format!("Downloading {}", titles.join(", "))); + } + if !recheck.is_empty() { + self.log( + app, + "info", + format!( + "Fetching what is missing on disk from {} (their markers say complete; the count says otherwise).", + recheck.iter().cloned().collect::>().join(", ") + ), + ); + } let (tile_sets, src_paths) = market::install_filter(manifest, &extras, true); let options = InstallOptions { only: vec![Stage::Src, Stage::Tiles], tile_sets: Some(tile_sets), src_paths: Some(src_paths), + recheck_tile_sets: recheck, ..InstallOptions::default() }; let progress = { diff --git a/src-tauri/src/market.rs b/src-tauri/src/market.rs index 1045828..e76263e 100644 --- a/src-tauri/src/market.rs +++ b/src-tauri/src/market.rs @@ -337,6 +337,17 @@ pub fn unfinished( .collect() } +/// The tile sets a count found short, as the installer names them (`style/dataset`): what +/// Finish hands it to re-count block by block, because their markers say they are whole. +pub fn short_tile_sets(checked: &[Check]) -> BTreeSet { + checked + .iter() + .filter(|check| !check.whole) + .filter_map(|check| check.key.strip_prefix("tiles:")) + .map(str::to_string) + .collect() +} + /// Whether the chosen optional items still need downloading. pub fn to_add(manifest: &Manifest, data_dir: &Path, extras: &BTreeSet) -> Vec { items(manifest, data_dir, extras) @@ -389,6 +400,21 @@ pub fn remove_counting( mod tests { use super::*; + #[test] + fn finish_repairs_the_sets_a_count_found_short() { + // What the log said on a real PC: two markers claiming "installed" over missing files. + let check = |key: &str, found, expected| Check { key: key.to_string(), found, expected, whole: found >= expected }; + let checked = [ + check("tiles:popdark/protomaps_20260423", 293_056, 400_268), + check("tiles:yandex/protomaps_20260423", 2_371, 28_213), + check("tiles:dark_v2/protomaps_20260423", 95_000, 90_501), + ]; + let short: Vec = short_tile_sets(&checked).into_iter().collect(); + // In the installer's own spelling, and a set holding MORE than it names is not short. + assert_eq!(short, ["popdark/protomaps_20260423", "yandex/protomaps_20260423"]); + assert!(short_tile_sets(&[]).is_empty()); + } + fn manifest() -> Manifest { serde_json::from_value(serde_json::json!({ "schema": 1, "product": "odian", "version": "0.61.0", "released_at": "2026-09-22T12:00:00Z",