tempfiles.rs: remove duplicate tmpfiles entries

rpmostree-cxxrs.cxx

@@ -2922,6 +2922,9 @@ extern "C"
   void rpmostreecxx$cxxbridge1$cache_branch_to_nevra (::rust::Str nevra,
                                                       ::rust::String *return$) noexcept;
 
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (::std::int32_t rootfs) noexcept;
+
   ::std::uint32_t
   rpmostreecxx$cxxbridge1$CxxGObjectArray$length (::rpmostreecxx::CxxGObjectArray &self) noexcept
   {
@@ -6059,6 +6062,16 @@ cache_branch_to_nevra (::rust::Str nevra) noexcept
   rpmostreecxx$cxxbridge1$cache_branch_to_nevra (nevra, &return$.value);
   return ::std::move (return$.value);
 }
+
+void
+deduplicate_tmpfiles_entries (::std::int32_t rootfs)
+{
+  ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (rootfs);
+  if (error$.ptr)
+    {
+      throw ::rust::impl< ::rust::Error>::error (error$);
+    }
+}
 } // namespace rpmostreecxx
 
 extern "C"
@@ -6773,5 +6786,6 @@ Vec< ::rpmostreecxx::LockedPackage>::truncate (::std::size_t len)
 {
   return cxxbridge1$rust_vec$rpmostreecxx$LockedPackage$truncate (this, len);
 }
+
 } // namespace cxxbridge1
 } // namespace rust

rpmostree-cxxrs.h

@@ -2043,4 +2043,6 @@ ::rpmostreecxx::GKeyFile *treefile_to_origin (::rpmostreecxx::Treefile const &tf
 void origin_validate_roundtrip (::rpmostreecxx::GKeyFile const &kf) noexcept;
 
 ::rust::String cache_branch_to_nevra (::rust::Str nevra) noexcept;
+
+void deduplicate_tmpfiles_entries (::std::int32_t rootfs);
 } // namespace rpmostreecxx

rust/src/lib.rs

@@ -710,6 +710,7 @@ pub mod ffi {
     extern "Rust" {
         fn prepare_rpm_layering(rootfs: i32, merge_passwd_dir: &str) -> Result<bool>;
         fn complete_rpm_layering(rootfs: i32) -> Result<()>;
+        fn deduplicate_tmpfiles_entries(rootfs: i32) -> Result<()>;
         fn passwd_cleanup(rootfs: i32) -> Result<()>;
         fn migrate_group_except_root(rootfs: i32, preserved_groups: &Vec<String>) -> Result<()>;
         fn migrate_passwd_except_root(rootfs: i32) -> Result<()>;
@@ -986,6 +987,8 @@ mod rpmutils;
 pub(crate) use self::rpmutils::*;
 mod testutils;
 pub(crate) use self::testutils::*;
+mod tmpfiles;
+pub use self::tmpfiles::*;
 mod treefile;
 pub use self::treefile::*;
 pub mod utils;

rust/src/tmpfiles.rs

@@ -0,0 +1,175 @@
+/*
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ */
+use crate::cxxrsutil::*;
+use crate::ffiutil;
+
+use anyhow::{Context, Result};
+use cap_std::fs::{Dir, Permissions};
+use cap_std_ext::dirext::CapStdExtDirExt;
+use fn_error_context::context;
+use std::collections::{HashMap, HashSet};
+use std::fmt::Write;
+use std::os::unix::prelude::PermissionsExt;
+use std::path::Path;
+
+const TMPFILESD: &str = "usr/lib/tmpfiles.d";
+const RPMOSTREE_TMPFILESD: &str = "usr/lib/rpm-ostree/tmpfiles.d";
+const AUTOVAR_PATH: &str = "rpm-ostree-autovar.conf";
+
+#[context("Deduplicate tmpfiles entries")]
+pub fn deduplicate_tmpfiles_entries(tmprootfs_dfd: i32) -> CxxResult<()> {
+    let tmprootfs_dfd = unsafe { ffiutil::ffi_dirfd(tmprootfs_dfd)? };
+
+    // scan all rpm-ostree auto generated entries and save
+    let tmpfiles_dir = tmprootfs_dfd
+        .open_dir(RPMOSTREE_TMPFILESD)
+        .context("readdir {RPMOSTREE_TMPFILESD}")?;
+    let mut rpmostree_tmpfiles_entries = save_tmpfile_entries(&tmpfiles_dir)?
+        .map(|s| {
+            let entry = tmpfiles_entry_get_path(&s.as_str())?;
+            anyhow::Ok((entry.to_string(), s.to_string()))
+        })
+        .collect::<Result<HashMap<String, String>>>()?;
+
+    // remove autovar.conf first, then scan all system entries and save
+    let tmpfiles_dir = tmprootfs_dfd
+        .open_dir(TMPFILESD)
+        .context("readdir {TMPFILESD}")?;
+
+    if tmpfiles_dir.try_exists(AUTOVAR_PATH)? {
+        tmpfiles_dir.remove_file(AUTOVAR_PATH)?;
+    }
+    let system_tmpfiles_entries = save_tmpfile_entries(&tmpfiles_dir)?
+        .map(|s| {
+            let entry = tmpfiles_entry_get_path(&s.as_str())?;
+            anyhow::Ok(entry.to_string())
+        })
+        .collect::<Result<HashSet<String>>>()?;
+
+    // remove duplicated entries in auto-generated tmpfiles.d,
+    // which are already in system tmpfiles
+    for key in system_tmpfiles_entries.into_iter() {
+        rpmostree_tmpfiles_entries.retain(|k, _value| k != &key);
+    }
+
+    {
+        // save the noduplicated entries
+        let mut entries = String::from("# This file was generated by rpm-ostree.\n");
+        for (_key, value) in rpmostree_tmpfiles_entries {
+            writeln!(entries, "{value}").unwrap();
+        }
+
+        let perms = Permissions::from_mode(0o644);
+        tmpfiles_dir.atomic_write_with_perms(&AUTOVAR_PATH, entries.as_bytes(), perms)?;
+    }
+    Ok(())
+}
+
+// #[context("Scan all tmpfiles conf and save entries")]
+fn save_tmpfile_entries(tmpfiles_dir: &Dir) -> Result<impl Iterator<Item = String>> {
+    let entries = tmpfiles_dir
+        .entries()?
+        .filter_map(|name| {
+            let name = name.unwrap().file_name();
+            if let Some(extension) = Path::new(&name).extension() {
+                if extension != "conf" {
+                    return None;
+                }
+            } else {
+                return None;
+            }
+            Some(
+                tmpfiles_dir
+                    .read_to_string(name)
+                    .ok()?
+                    .lines()
+                    .filter(|s| !s.is_empty() && !s.starts_with('#'))
+                    .map(|s| s.to_string())
+                    .collect::<Vec<_>>(),
+            )
+        })
+        .flatten()
+        .collect::<Vec<_>>();
+
+    Ok(entries.into_iter())
+}
+
+#[context("Scan tmpfiles entries and get path")]
+fn tmpfiles_entry_get_path(line: &str) -> Result<&str> {
+    line.split_whitespace()
+        .nth(1)
+        .ok_or_else(|| anyhow::anyhow!("Malformed tmpfiles.d entry ({line})"))
+}
+
+#[cfg(test)]
+mod tests {
+    use rustix::fd::AsRawFd;
+
+    use super::*;
+    #[test]
+    fn test_tmpfiles_entry_get_path() {
+        let cases = [
+            ("z /dev/kvm          0666 - kvm -", "/dev/kvm"),
+            ("d /run/lock/lvm 0700 root root -", "/run/lock/lvm"),
+            ("a+      /var/lib/tpm2-tss/system/keystore   -    -    -     -           default:group:tss:rwx", "/var/lib/tpm2-tss/system/keystore"),
+        ];
+        for (input, expected) in cases {
+            let path = tmpfiles_entry_get_path(input).unwrap();
+            assert_eq!(path, expected, "Input: {input}");
+        }
+    }
+
+    fn newroot() -> Result<cap_std_ext::cap_tempfile::TempDir> {
+        let root = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
+        root.create_dir_all(RPMOSTREE_TMPFILESD)?;
+        root.create_dir_all(TMPFILESD)?;
+        Ok(root)
+    }
+
+    #[test]
+    fn test_deduplicate_noop() -> Result<()> {
+        let root = &newroot()?;
+        deduplicate_tmpfiles_entries(root.as_raw_fd())?;
+        Ok(())
+    }
+
+    // The first and 3rd are duplicates
+    const PKG_FILESYSTEM_CONTENTS: &str = indoc::indoc! { r#"
+d /var/cache 0755 root root - -
+d /var/lib/games 0755 root root - -
+d /var/tmp 1777 root root - -
+d /var/spool/mail 0775 root mail - -
+"# };
+    const VAR_CONF: &str = indoc::indoc! { r#"
+d /var/cache 0755 - - -
+"# };
+    const TMP_CONF: &str = indoc::indoc! { r#"
+q /var/tmp 1777 root root 30d
+"# };
+
+    #[test]
+    fn test_deduplicate() -> Result<()> {
+        let root = &newroot()?;
+        let rpmostree_tmpfiles_dir = root.open_dir(RPMOSTREE_TMPFILESD)?;
+        let tmpfiles_dir = root.open_dir(TMPFILESD)?;
+        rpmostree_tmpfiles_dir
+            .atomic_write(format!("pkg-filesystem.conf"), PKG_FILESYSTEM_CONTENTS)?;
+        tmpfiles_dir.atomic_write("var.conf", VAR_CONF)?;
+        tmpfiles_dir.atomic_write("tmp.conf", TMP_CONF)?;
+        assert!(!tmpfiles_dir.try_exists(AUTOVAR_PATH)?);
+        deduplicate_tmpfiles_entries(root.as_raw_fd())?;
+        let contents = tmpfiles_dir.read_to_string(AUTOVAR_PATH).unwrap();
+        assert!(contents.contains("# This file was generated by rpm-ostree."));
+        let entries = contents
+            .lines()
+            .filter(|l| !(l.is_empty() || l.starts_with('#')))
+            .collect::<Vec<_>>();
+        assert_eq!(entries.len(), 2);
+        assert_eq!(entries[0], "d /var/lib/games 0755 root root - -");
+        assert_eq!(entries[1], "d /var/spool/mail 0775 root mail - -");
+        Ok(())
+    }
+}

src/app/rpm-ostree-0-integration.conf

@@ -14,5 +14,4 @@ d /var/usrlocal/share 0755 root root -
 d /var/usrlocal/src 0755 root root -
 d /var/mnt 0755 root root -
 d /run/media 0755 root root -
-d /var/lib 0755 root root -
 L /var/lib/rpm - - - - ../../usr/share/rpm

src/libpriv/rpmostree-core.cxx

@@ -3085,13 +3085,11 @@ delete_package_from_root (RpmOstreeContext *self, rpmte pkg, int rootfs_dfd, GHa
         }
     }
 
-  /* And finally, delete any automatically generated tmpfiles.d dropin. Though
-   * ideally we'd have a way to be sure that this was the rpm-ostree generated
-   * one. */
+  /* And finally, delete any automatically generated tmpfiles.d dropin. */
   glnx_autofd int tmpfiles_dfd = -1;
-  if (!glnx_opendirat (rootfs_dfd, "usr/lib/tmpfiles.d", TRUE, &tmpfiles_dfd, error))
+  if (!glnx_opendirat (rootfs_dfd, "usr/lib/rpm-ostree/tmpfiles.d", TRUE, &tmpfiles_dfd, error))
     return FALSE;
-  g_autofree char *dropin = g_strdup_printf ("pkg-%s.conf", rpmteN (pkg));
+  g_autofree char *dropin = g_strdup_printf ("%s.conf", rpmteN (pkg));
   if (!glnx_shutil_rm_rf_at (tmpfiles_dfd, dropin, cancellable, error))
     return FALSE;
 
@@ -4464,6 +4462,12 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
         return FALSE;
     }
 
+  /* Remove duplicated tmpfiles entries.
+   * See https://github.com/coreos/rpm-ostree/issues/26
+   */
+  if (overlays->len > 0 || overrides_remove->len > 0 || overrides_replace->len > 0)
+    ROSCXX_TRY (deduplicate_tmpfiles_entries (tmprootfs_dfd), error);
+
   /* Undo the /etc move above */
   CXX_TRY (etc_guard->undo (), error);
 

src/libpriv/rpmostree-importer.cxx

@@ -627,10 +627,12 @@ import_rpm_to_repo (RpmOstreeImporter *self, char **out_csum, char **out_metadat
 
       if (!glnx_mkdtemp ("rpm-ostree-import.XXXXXX", 0700, &tmpdir, error))
         return FALSE;
-      if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/lib/tmpfiles.d", 0755, cancellable, error))
+      if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/lib/rpm-ostree/tmpfiles.d", 0755, cancellable,
+                                   error))
         return FALSE;
       if (!glnx_file_replace_contents_at (
-              tmpdir.fd, glnx_strjoina ("usr/lib/tmpfiles.d/", "pkg-", pkg_name.c_str (), ".conf"),
+              tmpdir.fd,
+              glnx_strjoina ("usr/lib/rpm-ostree/tmpfiles.d/", pkg_name.c_str (), ".conf"),
               (guint8 *)content.data (), content.size (), GLNX_FILE_REPLACE_NODATASYNC, cancellable,
               error))
         return FALSE;

src/libpriv/rpmostree-postprocess.cxx

@@ -425,7 +425,20 @@ postprocess_final (int rootfs_dfd, rpmostreecxx::Treefile &treefile, gboolean un
 
   ROSCXX_TRY (rootfs_prepare_links (rootfs_dfd), error);
 
-  ROSCXX_TRY (convert_var_to_tmpfiles_d (rootfs_dfd, *cancellable), error);
+  if (!unified_core_mode)
+    ROSCXX_TRY (convert_var_to_tmpfiles_d (rootfs_dfd, *cancellable), error);
+  else
+    {
+      /* In unified core mode, /var entries are converted to tmpfiles.d at
+       * import time and scriptlets are prevented from writing to /var. What
+       * remains is just the compat symlinks that we created ourselves, which we
+       * should stop writing since it duplicates other tmpfiles.d entries. */
+      if (!glnx_shutil_rm_rf_at (rootfs_dfd, "var", cancellable, error))
+        return FALSE;
+      /* but we still want the mount point as part of the OSTree commit */
+      if (mkdirat (rootfs_dfd, "var", 0755) < 0)
+        return glnx_throw_errno_prefix (error, "mkdirat(var)");
+    }
 
   if (!rpmostree_rootfs_postprocess_common (rootfs_dfd, cancellable, error))
     return FALSE;

tests/compose/test-basic-unified.sh

@@ -119,20 +119,19 @@ echo "ok no cachedir"
 basic_test
 
 # This one is done by postprocessing /var
-ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-filesystem.conf > autovar.txt
+rpmostree_tmpfiles_path="/usr/lib/rpm-ostree/tmpfiles.d"
+ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/filesystem.conf > autovar.txt
 # Picked this one at random as an example of something that won't likely be
 # converted to tmpfiles.d upstream.  But if it is, we can change this test.
 assert_file_has_content_literal autovar.txt 'd /var/cache 0755 root root - -'
-ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-chrony.conf > autovar.txt
+ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/chrony.conf > autovar.txt
 # And this one has a non-root uid
 assert_file_has_content_literal autovar.txt 'd /var/lib/chrony 0750 chrony chrony - -'
 # see rpmostree-importer.c
-if ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-rpm.conf > rpm.txt 2>/dev/null; then
+if ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/rpm.conf > rpm.txt 2>/dev/null; then
     assert_not_file_has_content rpm.txt 'd /var/lib/rpm'
 fi
-ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-pam.conf > autovar.txt
-# Verify translating /var/run -> /run
-assert_file_has_content_literal autovar.txt 'd /run/console'
+
 echo "ok autovar"
 
 rpm-ostree db list --repo="${repo}" "${treeref}" --advisories > db-list-adv.txt

tests/kolainst/nondestructive/misc.sh

@@ -33,6 +33,28 @@ rm -f out.txt
 rpm-ostree -h usroverlay >out.txt
 assert_file_has_content out.txt "ostree admin unlock"
 
+# Verify https://github.com/coreos/rpm-ostree/issues/26
+tmpfiles="/usr/lib/tmpfiles.d/rpm-ostree-autovar.conf"
+# Duplication in tmp.conf
+assert_not_file_has_content $tmpfiles 'd /var/tmp'
+# Duplication in var.conf
+assert_not_file_has_content $tmpfiles 'd /var/cache '
+assert_file_has_content_literal $tmpfiles 'd /var/lib/chrony 0750 chrony chrony - -'
+
+set +x # so our grepping doesn't get a hit on itself
+if journalctl --grep 'Duplicate line'; then
+    fatal "Should not get logs (Duplicate line)"
+fi
+set -x # restore
+
+# Verify remove can trigger the generation of rpm-ostree-autovar.conf
+rpm-ostree override remove chrony
+deploy=$(rpm-ostree status --json | jq -r '.deployments[0]["id"]' | awk -F'-' '{print $3}')
+osname=$(rpm-ostree status --json | jq -r '.deployments[0]["osname"]')
+cat /ostree/deploy/${osname}/deploy/${deploy}/${tmpfiles} > autovar.txt
+assert_not_file_has_content autovar.txt '/var/lib/chrony'
+rpm-ostree cleanup -pr
+
 # make sure that package-related entries are always present,
 # even when they're empty.
 # Validate there's no live state by default.

tests/vmcheck/test-layering-basic-1.sh

@@ -52,9 +52,9 @@ vm_reboot
 
 vm_cmd rpm -qlv test-opt
 root=$(vm_get_deployment_root 0)
-vm_has_files $root/usr/lib/opt/app/bin/foo $root/usr/lib/tmpfiles.d/pkg-test-opt.conf
-vm_cmd cat $root/usr/lib/tmpfiles.d/pkg-test-opt.conf
-vm_cmd grep -q /usr/lib/opt/app $root/usr/lib/tmpfiles.d/pkg-test-opt.conf
+vm_has_files $root/usr/lib/opt/app/bin/foo $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf
+vm_cmd cat $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf
+vm_cmd grep -q /usr/lib/opt/app $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf
 vm_cmd ls -al /var/opt/ /var/app
 vm_cmd test -d "'/opt/some lib/subdir'"
 vm_cmd test -d '/opt/quote\"ed/subdir'

tests/vmcheck/test-override-replace-2.sh

@@ -134,13 +134,15 @@ echo "ok override replace both"
 
 # And now verify https://github.com/coreos/rpm-ostree/pull/3228
 prev_root=$(vm_get_deployment_root 0)
-vm_cmd grep ' /var/pkg-with-var ' "${prev_root}/usr/lib/tmpfiles.d/pkg-pkg-with-var.conf"
+vm_cmd grep ' /var/pkg-with-var ' "${prev_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf"
 vm_build_rpm pkg-with-var version 2 \
   files "/var/pkg-with-different-var" \
   install "mkdir -p '%{buildroot}/var/pkg-with-different-var'"
 vm_rpmostree override replace $YUMREPO/pkg-with-var-2-1.x86_64.rpm
 new_root=$(vm_get_deployment_root 0)
-vm_cmd grep ' /var/pkg-with-different-var ' "${new_root}/usr/lib/tmpfiles.d/pkg-pkg-with-var.conf"
+vm_cmd grep ' /var/pkg-with-different-var ' "${new_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf"
+vm_cmd ! grep ' /var/pkg-with-var ' "${new_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf"
+vm_cmd ! grep ' /var/pkg-with-var ' "${new_root}/usr/lib/tmpfiles.d/rpm-ostree-autovar.conf"
 vm_rpmostree cleanup -p
 echo "ok override replace deletes tmpfiles.d dropin"