Skip to content

Etc merge changes #2301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Johan-Liebert1 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Etc merge changes #2301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fccb8fb
etc-merge: Handle unmergable paths properly
Johan-Liebert1 Jul 6, 2026
954905d
cfs/stage: Compute etc diff before staging
Johan-Liebert1 Jul 7, 2026
df2466f
tmt: Add tests for etc merge conflict
Johan-Liebert1 Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 140 additions & 30 deletions crates/etc-merge/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ fn stat_eq_ignore_mtime(this: &Stat, other: &Stat) -> bool {
return true;
}

#[derive(Debug)]
pub struct UnmergablePaths {
pub path: PathBuf,
pub reason: String,
}

/// Represents the differences between two directory trees.
#[derive(Debug)]
pub struct Diff {
Expand All @@ -88,6 +94,8 @@ pub struct Diff {
modified: Vec<PathBuf>,
/// Paths that exist in the pristine /etc but not in the current one
removed: Vec<PathBuf>,
/// Paths that are unmergable
pub unmergable_paths: Vec<UnmergablePaths>,
}

fn collect_all_files(
Expand Down Expand Up @@ -170,6 +178,47 @@ fn get_deletions(
Ok(())
}

fn check_if_mergable(
new: &Directory<CustomMetadata>,
current_inode: &Inode<CustomMetadata>,
current_path: &PathBuf,
diff: &mut Diff,
) {
match current_inode {
// If currently 'file' is a directory, make sure it's not a regular file
// new_etc as well, else we can't merge
Inode::Directory(..) => {
let new_dir = new.get_directory(&current_path.as_os_str());

if matches!(new_dir, Err(ImageError::NotADirectory(..))) {
diff.unmergable_paths.push(UnmergablePaths {
path: current_path.clone(),
reason: format!(
"Directory '{}' now defaults to a file in new etc",
current_path.display()
),
});
}
}

// If currently 'file' is not a directory, make sure it's not a directory in the
// new_etc either, else we can't merge
Inode::Leaf(..) => {
let new_dir = new.get_directory(&current_path.as_os_str());

if matches!(new_dir, Ok(..)) {
diff.unmergable_paths.push(UnmergablePaths {
path: current_path.clone(),
reason: format!(
"File '{}' now defaults to a directory in new etc",
current_path.display()
),
});
}
}
}
}

// 1. Files in the currently booted deployment’s /etc which were modified from the default /usr/etc (of the same deployment) are retained.
//
// 2. Files in the currently booted deployment’s /etc which were not modified from the default /usr/etc (of the same deployment)
Expand All @@ -190,6 +239,7 @@ fn get_modifications(
current: &Directory<CustomMetadata>,
pristine_leaves: &[Leaf<CustomMetadata>],
current_leaves: &[Leaf<CustomMetadata>],
new_leaves: &[Leaf<CustomMetadata>],
new: &Directory<CustomMetadata>,
mut current_path: PathBuf,
diff: &mut Diff,
Expand All @@ -216,26 +266,45 @@ fn get_modifications(
&curr_dir,
pristine_leaves,
current_leaves,
new_leaves,
new,
current_path.clone(),
diff,
)?;

// This directory or its contents were modified/added
// Check if the new directory was deleted from new_etc
// If it was, we want to add the directory back
if new.get_directory_opt(&current_path.as_os_str())?.is_none() {
if diff.added.len() != total_added {
diff.added.insert(total_added, current_path.clone());
} else if diff.modified.len() != total_modified {
diff.modified.insert(total_modified, current_path.clone());
match new.get_directory(&current_path.as_os_str()) {
Ok(..) => {
// Directory exists in both current and new etc.
// Modifications/additions within this directory are handled recursively.
// No additional action needed here.
}

Err(ImageError::NotFound(..)) | Err(ImageError::NotADirectory(..)) => {
// This directory was deleted in new_etc
// If it was modified in the current etc, we want this back
//
// Was a directory in current etc, but is now
// a file/symlink in the new etc
if diff.added.len() != total_added {
diff.added.insert(total_added, current_path.clone());
} else if diff.modified.len() != total_modified {
diff.modified.insert(total_added, current_path.clone());
}
}

Err(e) => Err(e)?,
}

if diff.modified.len() != total_modified || diff.added.len() != total_added
{
check_if_mergable(new, inode, &current_path, diff);
}
}

Err(ImageError::NotFound(..)) => {
// Dir not found in original /etc, dir was added
diff.added.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);

// Also add every file inside that dir
collect_all_files(&curr_dir, current_path.clone(), &mut diff.added);
Expand All @@ -245,18 +314,21 @@ fn get_modifications(
// Some directory was changed to a file/symlink
// This should be counted in the diff, but we don't really merge this
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}

Err(e) => Err(e)?,
Err(e) => Err(e).with_context(|| format!("Opening pristine {path:?}"))?,
}
}

Inode::Leaf(leaf_id, _) => match pristine.leaf_id(path) {
Ok(old_leaf_id) => {
let leaf = &current_leaves[leaf_id.0];
let old_leaf = &pristine_leaves[old_leaf_id.0];

if !stat_eq_ignore_mtime(&old_leaf.stat, &leaf.stat) {
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
current_path.pop();
continue;
}
Expand All @@ -266,19 +338,22 @@ fn get_modifications(
if old_meta.content_hash != current_meta.content_hash {
// File modified in some way
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}
}

(Symlink(old_link), Symlink(current_link)) => {
if old_link != current_link {
// Symlink modified in some way
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}
}

(Symlink(..), Regular(..)) | (Regular(..), Symlink(..)) => {
// File changed to symlink or vice-versa
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}

(a, b) => {
Expand All @@ -290,14 +365,16 @@ fn get_modifications(
Err(ImageError::IsADirectory(..)) => {
// A directory was changed to a file
diff.modified.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}

Err(ImageError::NotFound(..)) => {
// File not found in original /etc, file was added
diff.added.push(current_path.clone());
check_if_mergable(new, inode, &current_path, diff);
}

Err(e) => Err(e).context(format!("{path:?}"))?,
Err(e) => Err(e).with_context(|| format!("Opening pristine {path:?}"))?,
},
}

Expand Down Expand Up @@ -385,13 +462,15 @@ pub fn compute_diff(
added: vec![],
modified: vec![],
removed: vec![],
unmergable_paths: vec![],
};

get_modifications(
&pristine_etc_files.root,
&current_etc_files.root,
&pristine_etc_files.leaves,
&current_etc_files.leaves,
&new_etc_files.leaves,
&new_etc_files.root,
PathBuf::new(),
&mut diff,
Expand All @@ -407,6 +486,19 @@ pub fn compute_diff(
Ok(diff)
}

pub fn print_unmergable_paths(diff: &Diff, writer: &mut impl Write) {
use owo_colors::OwoColorize;

for unmergable in &diff.unmergable_paths {
let _ = writeln!(
writer,
"{} {}",
ModificationType::Unmergable.magenta(),
unmergable.reason
);
}
}

/// Prints a colorized summary of differences to standard output.
pub fn print_diff(diff: &Diff, writer: &mut impl Write) {
use owo_colors::OwoColorize;
Expand All @@ -422,6 +514,8 @@ pub fn print_diff(diff: &Diff, writer: &mut impl Write) {
for removed in &diff.removed {
let _ = writeln!(writer, "{} {removed:?}", ModificationType::Removed.red());
}

print_unmergable_paths(diff, writer);
}

#[context("Collecting xattrs")]
Expand Down Expand Up @@ -589,6 +683,7 @@ enum ModificationType {
Added,
Modified,
Removed,
Unmergable,
}

impl std::fmt::Display for ModificationType {
Expand All @@ -603,6 +698,7 @@ impl ModificationType {
ModificationType::Added => "+",
ModificationType::Modified => "~",
ModificationType::Removed => "-",
ModificationType::Unmergable => "*",
}
}
}
Expand All @@ -613,23 +709,35 @@ fn create_dir_with_perms(
stat: &Stat,
new_inode: Option<&Inode<CustomMetadata>>,
) -> anyhow::Result<()> {
// The new directory is not present in the new_etc, so we create it, else we only copy the
// metadata
if new_inode.is_none() {
// Here we use `create_dir_all` to create every parent as we will set the permissions later
// on. Due to the fact that we have an ordered (sorted) list of directories and directory
// entries and we have a DFS traversal, we will always have directory creation starting from
// the parent anyway.
//
// The exception being, if a directory is modified in the current_etc, and a new directory
// is added inside the modified directory, say `dir/prems` has its permissions modified and
// `dir/prems/new` is the new directory created. Since we handle added files/directories first,
// we will create the directories `perms/new` with directory `new` also getting its
// permissions set, but `perms` will not. `perms` will have its permissions set up when we
// handle the modified directories.
new_etc_fd
.create_dir_all(&dir_name)
.context(format!("Failed to create dir {dir_name:?}"))?;
match new_inode {
Some(inode) => match inode {
Inode::Directory(..) => { /* no-op */ }

Inode::Leaf(..) => {
anyhow::bail!(
"Modified config directory {dir_name:?} newly defaults to file. Cannot merge"
)
}
},

// The new directory is not present in the new_etc, so we create it, else we only copy the
// metadata
None => {
// Here we use `create_dir_all` to create every parent as we will set the permissions later
// on. Due to the fact that we have an ordered (sorted) list of directories and directory
// entries and we have a DFS traversal, we will always have directory creation starting from
// the parent anyway.
//
// The exception being, if a directory is modified in the current_etc, and a new directory
// is added inside the modified directory, say `dir/prems` has its permissions modified and
// `dir/prems/new` is the new directory created. Since we handle added files/directories first,
// we will create the directories `perms/new` with directory `new` also getting its
// permissions set, but `perms` will not. `perms` will have its permissions set up when we
// handle the modified directories.
new_etc_fd
.create_dir_all(&dir_name)
.context(format!("Failed to create dir {dir_name:?}"))?;
}
}

new_etc_fd
Expand Down Expand Up @@ -730,12 +838,14 @@ fn merge_modified_files(
file,
current_inode.stat(current_leaves),
new_inode,
)?;
)
.context("Merging directory")?;
}

Inode::Leaf(leaf_id, _) => {
let leaf = &current_leaves[leaf_id.0];
merge_leaf(current_etc_fd, new_etc_fd, leaf, new_inode, file)?
merge_leaf(current_etc_fd, new_etc_fd, leaf, new_inode, file)
.context("Merging leaf")?
}
};
}
Expand All @@ -755,7 +865,7 @@ fn merge_modified_files(
}
},

Err(e) => Err(e)?,
Err(e) => Err(e).with_context(|| format!("Opening {file:?} in new etc"))?,
};
}

Expand Down
23 changes: 17 additions & 6 deletions crates/lib/src/bootc_composefs/finalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@ use cap_std_ext::cap_std::{ambient_authority, fs::Dir};
use cap_std_ext::dirext::CapStdExtDirExt;
use composefs::generic_tree::{FileSystem, Stat};
use composefs_ctl::composefs;
use etc_merge::{compute_diff, merge, print_diff, traverse_etc};
use etc_merge::{Diff, compute_diff, merge, print_diff, traverse_etc};
use rustix::fs::fsync;

use fn_error_context::context;

pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs) -> Result<()> {
pub(crate) async fn get_etc_diff(
storage: &Storage,
booted_cfs: &BootedComposefs,
new_etc: Option<&Dir>,
print: bool,
) -> Result<Diff> {
let host = get_composefs_status(storage, booted_cfs).await?;
let booted_composefs = host.require_composefs_booted()?;

Expand All @@ -37,16 +42,22 @@ pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs
Dir::open_ambient_dir(erofs_tmp_mnt.dir.path().join("etc"), ambient_authority())?;
let current_etc = Dir::open_ambient_dir("/etc", ambient_authority())?;

let (pristine_files, current_files, _) = traverse_etc(&pristine_etc, &current_etc, None)?;
let (pristine_files, current_files, new_etc_files) =
traverse_etc(&pristine_etc, &current_etc, new_etc)?;

let diff = compute_diff(
&pristine_files,
&current_files,
&FileSystem::new(Stat::uninitialized()),
&new_etc_files
.as_ref()
.unwrap_or(&FileSystem::new(Stat::uninitialized())),
)?;

print_diff(&diff, &mut std::io::stdout());
if print {
print_diff(&diff, &mut std::io::stdout());
}

Ok(())
Ok(diff)
}

pub(crate) async fn composefs_backend_finalize(
Expand Down
Loading
Loading