utils.rs: remove duplicate tmpfiles entries

rpm-ostree should scan the tmpfiles.d snippets at install time
and skip synthesizing entries if they are already specified.

As we may have confilicted tmpfile entries, like it will prevent
to cleanup of `/var/tmp` by `systemd-tmpfiles-clean.service`:
`pkg-filesystem.conf` has  `d /var/tmp 1777 root root - -`
`systemd's tmp.conf` has   `q /var/tmp 1777 root root 30d`

It is not as simple because the auto-conversion we do at package
import time may be duplicating a tmpfiles.d dropin that lives
in a separate package.

With Jonathan's suggestion:
- the importer code puts the generated tmpfiles.d dropins in e.g.
`/usr/lib/rpm-ostree/tmpfiles.d` instead of the canonical place
- `deduplicate_tmpfiles_entries()` builds one hashmap from
`/usr/lib/rpm-ostree/tmpfiles.d` and another from `/usr/lib/tmpfiles.d`
and generates `rpm-ostree-autovar.conf` from the difference
- delete_package_from_root() is updated to remove from
`/usr/lib/rpm-ostree/tmpfiles.d`
See #26 (comment)

Fixes #26

rpmostree-cxxrs.cxx

@@ -2921,6 +2921,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
   {
@@ -6055,6 +6058,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"
@@ -6769,5 +6782,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<()>;

rust/src/utils.rs

@@ -8,10 +8,14 @@
  */
 
 use crate::cxxrsutil::*;
+use crate::ffiutil;
 use crate::variant_utils;
 use anyhow::{bail, Context, Result};
 use camino::Utf8Path;
+use cap_std::fs::{Dir, Permissions};
 use cap_std::io_lifetimes::AsFilelike;
+use cap_std_ext::prelude::CapStdExtDirExt;
+use fn_error_context::context;
 use glib::Variant;
 use once_cell::sync::Lazy;
 use ostree_ext::prelude::*;
@@ -20,8 +24,10 @@ use regex::Regex;
 use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
 use std::io::prelude::*;
+use std::io::BufReader;
 use std::os::fd::OwnedFd;
 use std::os::unix::io::IntoRawFd;
+use std::os::unix::prelude::PermissionsExt;
 use std::path::Path;
 use std::{fs, io};
 
@@ -304,6 +310,101 @@ pub(crate) fn translate_path_for_ostree_impl(path: &str) -> Option<String> {
     None
 }
 
+#[context("Deduplicate tmpfiles entries")]
+pub fn deduplicate_tmpfiles_entries(tmprootfs_dfd: i32) -> CxxResult<()> {
+    let tmprootfs_dfd = unsafe { ffiutil::ffi_dirfd(tmprootfs_dfd)? };
+    static TMPFILESD: &str = "usr/lib/tmpfiles.d";
+    static RPMOSTREE_TMPFILESD: &str = "usr/lib/rpm-ostree/tmpfiles.d";
+    static AUTOVAR_PATH: &str = "rpm-ostree-autovar.conf";
+
+    let rpmostree_tmpfiles_list =
+        get_tmpfiles_path_list(&tmprootfs_dfd, RPMOSTREE_TMPFILESD).unwrap();
+    let system_tmpfiles_list = get_tmpfiles_path_list(&tmprootfs_dfd, TMPFILESD).unwrap();
+    if rpmostree_tmpfiles_list.is_empty() || system_tmpfiles_list.is_empty() {
+        println!("Not found any auto-generated or system tmpfiles.d config");
+        return Ok(());
+    }
+
+    // save rpm-ostree auto generated entries to hashmaps
+    let tmpfiles_dir = tmprootfs_dfd
+        .open_dir(RPMOSTREE_TMPFILESD)
+        .context("readdir {RPMOSTREE_TMPFILESD}")?;
+    let mut rpmostree_tmpfiles_entries =
+        save_tmpfile_entries(&tmpfiles_dir, &rpmostree_tmpfiles_list).unwrap();
+    // save system entries to hashmaps
+    let tmpfiles_dir = tmprootfs_dfd
+        .open_dir(TMPFILESD)
+        .context("readdir {TMPFILESD}")?;
+    let system_tmpfiles_entries =
+        save_tmpfile_entries(&tmpfiles_dir, &system_tmpfiles_list).unwrap();
+
+    // remove duplicated entries in auto-generated tmpfiles.d,
+    // which are already in system tmpfiles
+    for key in system_tmpfiles_entries.keys() {
+        if rpmostree_tmpfiles_entries.contains_key(key) {
+            rpmostree_tmpfiles_entries.remove(key);
+        }
+    }
+
+    {
+        // save the noduplicated entries
+        let mut entries = String::from("");
+        for (_key, value) in rpmostree_tmpfiles_entries {
+            entries.push_str(&format!("{}\n", value));
+        }
+
+        if tmpfiles_dir.try_exists(AUTOVAR_PATH)? {
+            tmpfiles_dir.remove_file(AUTOVAR_PATH)?
+        }
+        let perms = Permissions::from_mode(0o644);
+        tmpfiles_dir.atomic_write_with_perms(&AUTOVAR_PATH, entries.as_bytes(), perms)?;
+    }
+    Ok(())
+}
+
+#[context("Get tmpfiles files list")]
+pub(crate) fn get_tmpfiles_path_list(tmprootfs_dfd: &Dir, path: &str) -> Result<Vec<String>> {
+    let tmpfiles_dir = tmprootfs_dfd.open_dir(path).context("readdir {path}")?;
+    let mut tmpfiles_list = Vec::new();
+    for entry in tmpfiles_dir.entries()? {
+        let entry = entry?;
+        let name = entry.file_name();
+        let name = name.to_str().unwrap();
+        // skip README
+        if name == "README" || name == "rpm-ostree-autovar.conf" {
+            continue;
+        }
+        tmpfiles_list.push(format!("{name}"));
+    }
+    Ok(tmpfiles_list)
+}
+
+#[context("Save tmpfiles entries to hashmap")]
+pub(crate) fn save_tmpfile_entries(
+    tmpfiles_dir: &Dir,
+    var_list: &Vec<String>,
+) -> Result<HashMap<String, String>> {
+    let mut tmpflies_entries: HashMap<String, String> = HashMap::new();
+    for tmpflies in var_list.iter() {
+        let contents = tmpfiles_dir.open(tmpflies).map(BufReader::new)?;
+
+        for (line_num, line) in contents.lines().enumerate() {
+            let input = line
+                .with_context(|| format!("failed to read tmpfiles entry at line {}", line_num))?;
+
+            // Skip empty and comment lines
+            if input.is_empty() || input.starts_with('#') {
+                continue;
+            }
+
+            let parts: Vec<&str> = input.split_whitespace().collect();
+            let entry = parts.get(1).unwrap();
+            tmpflies_entries.insert(entry.to_string(), input.to_string());
+        }
+    }
+    Ok(tmpflies_entries)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

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

@@ -3089,7 +3089,7 @@ delete_package_from_root (RpmOstreeContext *self, rpmte pkg, int rootfs_dfd, GHa
    * ideally we'd have a way to be sure that this was the rpm-ostree generated
    * one. */
   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));
   if (!glnx_shutil_rm_rf_at (tmpfiles_dfd, dropin, cancellable, error))
@@ -4442,6 +4442,11 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
         task->end (msg);
       }
 
+      /* Remove duplicated tmpfiles entries;
+       * see https://github.com/coreos/rpm-ostree/issues/26
+       */
+      ROSCXX_TRY (deduplicate_tmpfiles_entries (tmprootfs_dfd), error);
+
       /* We want this to be the first error message if something went wrong
        * with a script; see https://github.com/projectatomic/rpm-ostree/pull/888
        * (otherwise, on a script that did `rm -rf`, we'd fail first on the renameat below)

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-", 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,27 @@ 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}/pkg-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}/pkg-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}/pkg-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'
+
+# Verify https://github.com/coreos/rpm-ostree/issues/26
+ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/rpm-ostree-autovar.conf > autovar.txt
+# Duplication in tmp.conf
+assert_not_file_has_content autovar.txt 'd /var/tmp'
+# Duplication in var.conf
+assert_not_file_has_content autovar.txt 'd /var/cache '
+assert_file_has_content_literal autovar.txt 'd /var/lib/chrony 0750 chrony chrony - -'
+
 echo "ok autovar"
 
 rpm-ostree db list --repo="${repo}" "${treeref}" --advisories > db-list-adv.txt

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/pkg-test-opt.conf
+vm_cmd cat $root/usr/lib/rpm-ostree/tmpfiles.d/pkg-test-opt.conf
+vm_cmd grep -q /usr/lib/opt/app $root/usr/lib/rpm-ostree/tmpfiles.d/pkg-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-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-pkg-with-var.conf"
+vm_cmd ! grep ' /var/pkg-with-var ' "${new_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-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"