From db23ba587ef2ce4826c7f972e27f6b92d46e12f9 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Sat, 28 Oct 2023 16:41:54 -0400
Subject: [PATCH] tar: Fix multiple hardlinks

The previous commit handled the case of *one* hardlink,
but the logic for the secondary one was backwards.

I noticed this when generating a derived image, in some
cases we only end up with `/usr/lib/sysimage/rpm-ostree-base-db`
as modified, which causes really confusing semantics that
look like packages going missing.
---
 lib/src/tar/write.rs |  6 +++---
 lib/tests/it/main.rs | 24 ++++++++++++------------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/src/tar/write.rs b/lib/src/tar/write.rs
index 4844134a..0d13e002 100644
--- a/lib/src/tar/write.rs
+++ b/lib/src/tar/write.rs
@@ -223,12 +223,12 @@ pub(crate) fn filter_tar(
                     dest.append_data(&mut header, path, data)?;
                     // And cache this file path as the new link target
                     new_sysroot_link_targets.insert(target.to_owned(), path.to_owned());
-                } else if let Some(target) = new_sysroot_link_targets.get(path) {
-                    tracing::debug!("Relinking {path} to {target}");
+                } else if let Some(real_target) = new_sysroot_link_targets.get(target) {
+                    tracing::debug!("Relinking {path} to {real_target}");
                     // We found a 2nd (or 3rd, etc.) link into /sysroot; rewrite the link
                     // target to be the first file outside of /sysroot we found.
                     let mut header = header.clone();
-                    dest.append_link(&mut header, path, target)?;
+                    dest.append_link(&mut header, path, real_target)?;
                 } else {
                     tracing::debug!("Found unhandled modified link from {path} to {target}");
                 }
diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs
index 5a10b816..b42e91d7 100644
--- a/lib/tests/it/main.rs
+++ b/lib/tests/it/main.rs
@@ -1261,12 +1261,14 @@ async fn test_container_write_derive_sysroot_hardlink() -> Result<()> {
             h.set_size(data.len() as u64);
             tar.append_data(&mut h, objpath, std::io::Cursor::new(data))
                 .context("appending object")?;
-            let targetpath = Utf8Path::new("usr/bin/bash");
-            h.set_size(0);
-            h.set_mtime(now);
-            h.set_entry_type(tar::EntryType::Link);
-            tar.append_link(&mut h, targetpath, objpath)
-                .context("appending target")?;
+            for path in ["usr/bin/bash", "usr/bin/bash-hardlinked"] {
+                let targetpath = Utf8Path::new(path);
+                h.set_size(0);
+                h.set_mtime(now);
+                h.set_entry_type(tar::EntryType::Link);
+                tar.append_link(&mut h, targetpath, objpath)
+                    .context("appending target")?;
+            }
             Ok::<_, anyhow::Error>(())
         },
         None,
@@ -1294,12 +1296,10 @@ async fn test_container_write_derive_sysroot_hardlink() -> Result<()> {
     )
     .ignore_stdout()
     .run()?;
-    let r = cmd!(
-        sh,
-        "ostree --repo=dest/repo cat {merge_commit} /usr/bin/bash"
-    )
-    .read()?;
-    assert_eq!(r.as_str(), "hello");
+    for path in ["/usr/bin/bash", "/usr/bin/bash-hardlinked"] {
+        let r = cmd!(sh, "ostree --repo=dest/repo cat {merge_commit} {path}").read()?;
+        assert_eq!(r.as_str(), "hello");
+    }
 
     Ok(())
 }