Skip to content

Commit

Permalink
utils.rs: remove duplicate tmpfiles entries
Browse files Browse the repository at this point in the history
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
  • Loading branch information
HuijingHei committed Dec 1, 2023
1 parent a8335b1 commit b6ca110
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 7 deletions.
14 changes: 14 additions & 0 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions rpmostree-cxxrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()>;
Expand Down
83 changes: 83 additions & 0 deletions rust/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -22,6 +26,7 @@ use std::collections::{HashMap, HashSet};
use std::io::prelude::*;
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};

Expand Down Expand Up @@ -304,6 +309,84 @@ 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";

// 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: HashMap<String, String> =
save_tmpfile_entries(&tmpfiles_dir)?
.into_iter()
.map(|s| {
let parts: Vec<&str> = s.split_whitespace().collect();
let entry = parts.get(1).unwrap();
(entry.to_string(), s.to_string())
})
.collect::<HashMap<String, String>>();

// remove autovar.conf first, then save system entries to hashmaps
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: HashSet<String> =
save_tmpfile_entries(&tmpfiles_dir)?
.into_iter()
.map(|s| {
let parts: Vec<&str> = s.split_whitespace().collect();
let entry = parts.get(1).unwrap();
entry.to_string()
})
.collect::<HashSet<String>>();

// remove duplicated entries in auto-generated tmpfiles.d,
// which are already in system tmpfiles
let _ = system_tmpfiles_entries.into_iter().map(|key| rpmostree_tmpfiles_entries.retain(|k, _| 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 {
entries.push_str(&format!("{}\n", value));
}

let perms = Permissions::from_mode(0o644);
tmpfiles_dir.atomic_write_with_perms(&AUTOVAR_PATH, entries.as_bytes(), perms)?;
}
Ok(())
}

#[context("Save tmpfiles entries")]
pub(crate) fn save_tmpfile_entries(tmpfiles_dir: &Dir) -> Result<Vec<String>> {
let mut entries: Vec<String> = Vec::new();
for entry in tmpfiles_dir.entries()? {
let entry = entry?;
let name = entry.file_name();
if !name.to_string_lossy().ends_with(".conf") {
continue;
}
let mut tmp_entries = tmpfiles_dir
.read_to_string(name)
.unwrap()
.lines()
.map(|s| s.to_string())
.filter(|s| !s.is_empty() && !s.starts_with('#'))
.collect::<Vec<String>>();

entries.append(&mut tmp_entries);
}
Ok(entries)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
14 changes: 9 additions & 5 deletions src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -4442,6 +4440,12 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
task->end (msg);
}

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

/* 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)
Expand Down
6 changes: 4 additions & 2 deletions src/libpriv/rpmostree-importer.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit b6ca110

Please sign in to comment.