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 Nov 30, 2023
1 parent ae2002c commit 8c90c75
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 5 deletions.
14 changes: 14 additions & 0 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
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
101 changes: 101 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 @@ -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};

Expand Down Expand Up @@ -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::*;
Expand Down
1 change: 0 additions & 1 deletion src/app/rpm-ostree-0-integration.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
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-", pkg_name.c_str (), ".conf"),
(guint8 *)content.data (), content.size (), GLNX_FILE_REPLACE_NODATASYNC, cancellable,
error))
return FALSE;
Expand Down
15 changes: 14 additions & 1 deletion src/libpriv/rpmostree-postprocess.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 8c90c75

Please sign in to comment.