tar: Create a new xattrs file for each checksum

This removes the code from import/export that links duplicate xattrs to
a single xattrs file. This is to avoid creating too many (over 65,000)
hardlinks to a single file. This will likely cause some image sizes to
be increased, however compression should limit the size increase.

Assisted-by: Claude Code
Signed-off-by: ckyrouac <[email protected]>

Author: ckyrouac

crates/ostree-ext/src/tar/export.rs

@@ -125,7 +125,6 @@ struct OstreeTarWriter<'a, W: std::io::Write> {
     wrote_dirtree: HashSet<String>,
     wrote_dirmeta: HashSet<String>,
     wrote_content: HashSet<String>,
-    wrote_xattrs: HashSet<String>,
 }
 
 pub(crate) fn object_path(objtype: ostree::ObjectType, checksum: &str) -> Utf8PathBuf {
@@ -146,11 +145,6 @@ fn v1_xattrs_object_path(checksum: &str) -> Utf8PathBuf {
     format!("{OSTREEDIR}/repo/objects/{first}/{rest}.file-xattrs").into()
 }
 
-fn v1_xattrs_link_object_path(checksum: &str) -> Utf8PathBuf {
-    let (first, rest) = checksum.split_at(2);
-    format!("{OSTREEDIR}/repo/objects/{first}/{rest}.file-xattrs-link").into()
-}
-
 /// Check for "denormal" symlinks which contain "//"
 // See https://github.com/fedora-sysv/chkconfig/pull/67
 // [root@cosa-devsh ~]# rpm -qf /usr/lib/systemd/systemd-sysv-install
@@ -196,7 +190,6 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
             wrote_dirmeta: HashSet::new(),
             wrote_dirtree: HashSet::new(),
             wrote_content: HashSet::new(),
-            wrote_xattrs: HashSet::new(),
         };
         Ok(r)
     }
@@ -225,18 +218,6 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
         tar_append_default_data(self.out, path, buf)
     }
 
-    /// Add an hardlink entry with default permissions (root/root 0644)
-    fn append_default_hardlink(&mut self, path: &Utf8Path, link_target: &Utf8Path) -> Result<()> {
-        let mut h = tar::Header::new_gnu();
-        h.set_entry_type(tar::EntryType::Link);
-        h.set_uid(0);
-        h.set_gid(0);
-        h.set_mode(0o644);
-        h.set_size(0);
-        self.out.append_link(&mut h, path, link_target)?;
-        Ok(())
-    }
-
     /// Write the initial /sysroot/ostree/repo structure.
     fn write_repo_structure(&mut self) -> Result<()> {
         if self.wrote_initdirs {
@@ -386,24 +367,8 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
         let xattrs_data = xattrs.data_as_bytes();
         let xattrs_data = xattrs_data.as_ref();
 
-        let xattrs_checksum = {
-            let digest = openssl::hash::hash(openssl::hash::MessageDigest::sha256(), xattrs_data)?;
-            hex::encode(digest)
-        };
-
-        let path = v1_xattrs_object_path(&xattrs_checksum);
-        // Write xattrs content into a separate `.file-xattrs` object.
-        if !self.wrote_xattrs.contains(&xattrs_checksum) {
-            let inserted = self.wrote_xattrs.insert(xattrs_checksum);
-            debug_assert!(inserted);
-            self.append_default_data(&path, xattrs_data)?;
-        }
-        // Write a `.file-xattrs-link` which links the file object to
-        // the corresponding detached xattrs.
-        {
-            let link_obj_path = v1_xattrs_link_object_path(checksum);
-            self.append_default_hardlink(&link_obj_path, &path)?;
-        }
+        let path = v1_xattrs_object_path(&checksum);
+        self.append_default_data(&path, xattrs_data)?;
 
         Ok(true)
     }
@@ -939,12 +904,4 @@ mod tests {
         let output = v1_xattrs_object_path(checksum);
         assert_eq!(&output, expected);
     }
-
-    #[test]
-    fn test_v1_xattrs_link_object_path() {
-        let checksum = "b8627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7";
-        let expected = "sysroot/ostree/repo/objects/b8/627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7.file-xattrs-link";
-        let output = v1_xattrs_link_object_path(checksum);
-        assert_eq!(&output, expected);
-    }
 }

crates/ostree-ext/src/tar/import.rs

@@ -48,11 +48,10 @@ pub(crate) struct Importer {
     repo: ostree::Repo,
     remote: Option<String>,
     verify_text: Option<String>,
-    // Cache of xattrs, keyed by their content checksum.
+    // Cache of xattrs, keyed by file checksum.
     xattrs: HashMap<String, glib::Variant>,
-    // Reusable buffer for xattrs references. It maps a file checksum (.0)
-    // to an xattrs checksum (.1) in the `xattrs` cache above.
-    next_xattrs: Option<(String, String)>,
+    // The next expected file checksum that should use cached xattrs.
+    next_xattrs: Option<String>,
 
     // Reusable buffer for reads.  See also https://github.com/rust-lang/rust/issues/78485
     buf: Vec<u8>,
@@ -156,12 +155,6 @@ fn parse_checksum(parent: &str, name: &Utf8Path) -> Result<String> {
     validate_sha256(reassembled)
 }
 
-/// Parse a `.file-xattrs-link` link target into the corresponding checksum.
-fn parse_xattrs_link_target(path: &Utf8Path) -> Result<String> {
-    let (parent, rest, _objtype) = parse_object_entry_path(path)?;
-    parse_checksum(parent, rest)
-}
-
 impl Importer {
     /// Create an importer which will import an OSTree commit object.
     pub(crate) fn new_for_commit(repo: &ostree::Repo, remote: Option<String>) -> Self {
@@ -360,7 +353,7 @@ impl Importer {
         let size: usize = entry.header().size()?.try_into()?;
 
         // Pop the queued xattrs reference.
-        let (file_csum, xattrs_csum) = self
+        let file_csum = self
             .next_xattrs
             .take()
             .ok_or_else(|| anyhow!("Missing xattrs reference"))?;
@@ -375,12 +368,12 @@ impl Importer {
             return Ok(());
         }
 
-        // Retrieve xattrs content from the cache.
+        // Retrieve xattrs content from the cache (now keyed by file checksum).
         let xattrs = self
             .xattrs
-            .get(&xattrs_csum)
+            .get(checksum)
             .cloned()
-            .ok_or_else(|| anyhow!("Failed to find xattrs content {}", xattrs_csum,))?;
+            .ok_or_else(|| anyhow!("Failed to find xattrs content for {}", checksum))?;
 
         match entry.header().entry_type() {
             tar::EntryType::Regular => {
@@ -423,7 +416,6 @@ impl Importer {
                 Ok(())
             }
             "file-xattrs" => self.process_file_xattrs(entry, checksum),
-            "file-xattrs-link" => self.process_file_xattrs_link(entry, checksum),
             "xattrs" => self.process_xattr_ref(entry, checksum),
             kind => {
                 let objtype = objtype_from_string(kind)
@@ -447,50 +439,36 @@ impl Importer {
     #[context("Processing file xattrs")]
     fn process_file_xattrs(
         &mut self,
-        entry: tar::Entry<impl std::io::Read>,
-        checksum: String,
-    ) -> Result<()> {
-        self.cache_xattrs_content(entry, Some(checksum))?;
-        Ok(())
-    }
-
-    /// Process a `.file-xattrs-link` object (v1).
-    ///
-    /// This is an hardlink that contains extended attributes for a content object.
-    /// When the max hardlink count is reached, this object may also be encoded as
-    /// a regular file instead.
-    #[context("Processing xattrs link")]
-    fn process_file_xattrs_link(
-        &mut self,
-        entry: tar::Entry<impl std::io::Read>,
+        mut entry: tar::Entry<impl std::io::Read>,
         checksum: String,
     ) -> Result<()> {
-        use tar::EntryType::{Link, Regular};
-        if let Some(prev) = &self.next_xattrs {
-            bail!(
-                "Found previous dangling xattrs for file object '{}'",
-                prev.0
-            );
+        // Read the xattrs content directly and associate with the file checksum
+        let header = entry.header();
+        if header.entry_type() != tar::EntryType::Regular {
+            return Err(anyhow!(
+                "Invalid xattr entry of type {:?}",
+                header.entry_type()
+            ));
+        }
+        let size = header.size()?;
+        if size > MAX_XATTR_SIZE as u64 {
+            return Err(anyhow!(
+                "xattrs object of size {} exceeds {} bytes",
+                size,
+                MAX_XATTR_SIZE
+            ));
         }
 
-        // Extract the xattrs checksum from the link target or from the content (v1).
-        // Later, it will be used as the key for a lookup into the `self.xattrs` cache.
-        let xattrs_checksum = match entry.header().entry_type() {
-            Link => {
-                let link_target = entry
-                    .link_name()?
-                    .ok_or_else(|| anyhow!("No xattrs link content for {}", checksum))?;
-                let xattr_target = Utf8Path::from_path(&link_target)
-                    .ok_or_else(|| anyhow!("Invalid non-UTF8 xattrs link {}", checksum))?;
-                parse_xattrs_link_target(xattr_target)?
-            }
-            Regular => self.cache_xattrs_content(entry, None)?,
-            x => bail!("Unexpected xattrs type '{:?}' found for {}", x, checksum),
-        };
+        let mut contents = vec![0u8; size as usize];
+        entry.read_exact(contents.as_mut_slice())?;
+        let data: glib::Bytes = contents.as_slice().into();
+
+        // Parse as the correct xattrs variant type
+        let v = Variant::from_bytes::<&[(&[u8], &[u8])]>(&data);
 
-        // Now xattrs are properly cached for the next content object in the stream,
-        // which should match `checksum`.
-        self.next_xattrs = Some((checksum, xattrs_checksum));
+        // Store xattrs directly keyed by file checksum
+        self.xattrs.insert(checksum.clone(), v);
+        self.next_xattrs = Some(checksum);
 
         Ok(())
     }
@@ -505,10 +483,7 @@ impl Importer {
         target: String,
     ) -> Result<()> {
         if let Some(prev) = &self.next_xattrs {
-            bail!(
-                "Found previous dangling xattrs for file object '{}'",
-                prev.0
-            );
+            bail!("Found previous dangling xattrs for file object '{}'", prev);
         }
 
         // Parse the xattrs checksum from the link target (v0).
@@ -528,9 +503,12 @@ impl Importer {
             .to_string();
         let xattrs_checksum = validate_sha256(xattr_target)?;
 
-        // Now xattrs are properly cached for the next content object in the stream,
-        // which should match `checksum`.
-        self.next_xattrs = Some((target, xattrs_checksum));
+        // Cache the xattrs for this target file using its checksum from the v0 format
+        // We need to look up the xattrs by the xattrs_checksum and store it by target
+        if let Some(xattrs) = self.xattrs.get(&xattrs_checksum).cloned() {
+            self.xattrs.insert(target.clone(), xattrs);
+        }
+        self.next_xattrs = Some(target);
 
         Ok(())
     }
@@ -915,26 +893,4 @@ mod tests {
         let output = parse_checksum(parent, &Utf8PathBuf::from(name)).unwrap();
         assert_eq!(output, expected);
     }
-
-    #[test]
-    fn test_parse_xattrs_link_target() {
-        let err_cases = &[
-            "",
-            "b8627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7.file-xattrs",
-            "../b8/62.file-xattrs",
-        ];
-        for input in err_cases {
-            parse_xattrs_link_target(Utf8Path::new(input)).unwrap_err();
-        }
-
-        let ok_cases = &[
-            "../b8/627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7.file-xattrs",
-            "sysroot/ostree/repo/objects/b8/627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7.file-xattrs",
-        ];
-        let expected = "b8627e3ef0f255a322d2bd9610cfaaacc8f122b7f8d17c0e7e3caafa160f9fc7";
-        for input in ok_cases {
-            let output = parse_xattrs_link_target(Utf8Path::new(input)).unwrap();
-            assert_eq!(output, expected);
-        }
-    }
 }