From 1e5b5f76170e7fdfc5cdc8aaa1a9ecfc6e2ca81f Mon Sep 17 00:00:00 2001 From: softwaregames1 <167585946+softwaregames1@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:17:17 +0100 Subject: [PATCH] Package: never overwrite one link's copy with another, list a missing link once (#9185) * Package: never overwrite one link's copy with another, list a missing link once Copies whose names collided were renamed to "-" once, without checking that name too: links a/x.png, b/2-x.png and c/x.png packaged the third over the second's copy, so two layers showed the same image. The name now steps until no earlier copy has it, ignoring case as macOS and Windows folders do. A missing link placed on several layers was listed once per layer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LPBRXWMhtDUsQDdZKmt9eZ * Contributors: credit softwaregames1 by their GitHub profile name Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LPBRXWMhtDUsQDdZKmt9eZ * Package: never overwrite a file already in Links A second package into the same folder picked the same Links/ and replaced the earlier copy. A name taken by a file already there is now stepped like one taken by another link in this run. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LPBRXWMhtDUsQDdZKmt9eZ * Apply suggestion from @coderabbitai[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- contributors/people.toml | 3 ++ crates/engine/src/print_cmds.rs | 25 ++++++++-- crates/engine/src/print_cmds/tests.rs | 66 +++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/contributors/people.toml b/contributors/people.toml index 242fb5ac80..4f345cfa18 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -264,3 +264,6 @@ sync_github_name = false [people.drunkeninja42] real_name = "Kushagra Sharma" + +[people.softwaregames1] +sync_github_name = true diff --git a/crates/engine/src/print_cmds.rs b/crates/engine/src/print_cmds.rs index 71efe29735..d115566a7c 100644 --- a/crates/engine/src/print_cmds.rs +++ b/crates/engine/src/print_cmds.rs @@ -1143,12 +1143,31 @@ fn package(s: &mut Session, p: &Value) -> Result { for (_, _, l) in doc.walk() { let LayerContent::Smart(so) = &l.content else { continue }; let SmartSource::Linked { path } = &so.source else { continue }; + if missing.contains(path) { + continue; + } let target = match copied.iter().find(|(src, _)| src == path) { Some((_, t)) => t.clone(), None => { - let mut fname = file_name(path); - if copied.iter().any(|(_, t)| file_name(t) == fname) { - fname = format!("{}-{}", copied.len(), fname); + // A name no earlier copy has, ignoring case (macOS and Windows folders do): a + // renamed copy can't take the name of another link either. A file already in + // Links (an earlier package into the same folder) is never overwritten. + let base = file_name(path); + let taken = |n: &str| { + copied.iter().any(|(_, t)| file_name(t).to_lowercase() == n.to_lowercase()) + || std::path::Path::new(&join(&links_dir, n)).exists() + || std::fs::read_dir(&links_dir) + .ok() + .into_iter() + .flatten() + .flatten() + .any(|entry| entry.file_name().to_string_lossy().to_lowercase() == n.to_lowercase()) + }; + let mut fname = base.clone(); + let mut k = copied.len(); + while taken(&fname) { + fname = format!("{k}-{base}"); + k += 1; } let t = join(&links_dir, &fname); // A path that is not followed is packaged as missing, like a file that is gone. diff --git a/crates/engine/src/print_cmds/tests.rs b/crates/engine/src/print_cmds/tests.rs index 43f49101a7..c4c211d16e 100644 --- a/crates/engine/src/print_cmds/tests.rs +++ b/crates/engine/src/print_cmds/tests.rs @@ -463,6 +463,72 @@ fn package_copies_links_and_relinks() { ); } +/// Links whose names collide get distinct copies, even when a renamed copy's name is already +/// another link's name (`b/2-x.png` copied before the third `x.png` would be renamed to it). +#[test] +fn package_never_overwrites_one_link_copy_with_another() { + let dir = tmp("package-collide"); + let mut s = session("rgb", 8); + let mut links = Vec::new(); + for (sub, name, colour) in [("a", "x.png", "#ff0000"), ("b", "2-x.png", "#00ff00"), ("c", "x.png", "#0000ff")] { + std::fs::create_dir_all(format!("{dir}/{sub}")).unwrap(); + let link = format!("{dir}/{sub}/{name}"); + let mut src = Session::new(); + src.execute("file.new", json!({"width": 4, "height": 4, "background": colour})).unwrap(); + crate::file_cmds::save_doc(&src.active().unwrap().doc, &link, None).unwrap(); + s.execute("file.placeLinked", json!({"path": link})).unwrap(); + links.push(std::fs::read(&link).unwrap()); + } + let r = s.execute("file.package", json!({"dir": format!("{dir}/pkg")})).unwrap(); + let copies: Vec<&str> = r["links"].as_array().unwrap().iter().map(|v| v.as_str().unwrap()).collect(); + assert_eq!(copies.len(), 3); + let unique: std::collections::HashSet<&&str> = copies.iter().collect(); + assert_eq!(unique.len(), 3, "{copies:?}"); + for (copy, bytes) in copies.iter().zip(&links) { + assert_eq!(&std::fs::read(copy).unwrap(), bytes, "{copy}"); + } + let _ = std::fs::remove_dir_all(&dir); +} + +/// A second package into the same folder copies its links beside the first one's, never over them. +#[test] +fn package_keeps_files_already_in_links() { + let dir = tmp("package-again"); + let link = format!("{dir}/art.png"); + let mut src = Session::new(); + src.execute("file.new", json!({"width": 4, "height": 4, "background": "#ff0000"})).unwrap(); + crate::file_cmds::save_doc(&src.active().unwrap().doc, &link, None).unwrap(); + let mut s = session("rgb", 8); + s.execute("file.placeLinked", json!({"path": link})).unwrap(); + let first = s.execute("file.package", json!({"dir": format!("{dir}/pkg")})).unwrap(); + let first = first["links"][0].as_str().unwrap().to_string(); + let kept = std::fs::read(&first).unwrap(); + let second = s.execute("file.package", json!({"dir": format!("{dir}/pkg")})).unwrap(); + let second = second["links"][0].as_str().unwrap(); + assert_ne!(second, first); + assert!(second.ends_with("Links/0-art.png"), "{second}"); + assert_eq!(std::fs::read(&first).unwrap(), kept); + assert_eq!(std::fs::read(second).unwrap(), kept); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A missing link placed twice is reported once. +#[test] +fn package_lists_a_missing_link_once() { + let dir = tmp("package-missing-once"); + let link = format!("{dir}/gone.png"); + let mut src = Session::new(); + src.execute("file.new", json!({"width": 4, "height": 4})).unwrap(); + crate::file_cmds::save_doc(&src.active().unwrap().doc, &link, None).unwrap(); + let mut s = session("rgb", 8); + s.execute("file.placeLinked", json!({"path": link})).unwrap(); + s.execute("file.placeLinked", json!({"path": link})).unwrap(); + std::fs::remove_file(&link).unwrap(); + let r = s.execute("file.package", json!({"dir": format!("{dir}/pkg")})).unwrap(); + assert_eq!(r["missing"], json!([link])); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn paths_export_as_illustrator_postscript() { let mut s = session("rgb", 8);