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 "<count>-<name>" 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPBRXWMhtDUsQDdZKmt9eZ

* Contributors: credit softwaregames1 by their GitHub profile name

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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/<name> 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

contributors/people.toml

@@ -264,3 +264,6 @@ sync_github_name = false
 
 [people.drunkeninja42]
 real_name = "Kushagra Sharma"
+
+[people.softwaregames1]
+sync_github_name = true

crates/engine/src/print_cmds.rs

@@ -1143,12 +1143,31 @@ fn package(s: &mut Session, p: &Value) -> Result<Value> {
     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.

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