mirror of
https://github.com/containers/bootc.git
synced 2026-02-05 15:45:53 +01:00
Don't exclusively own files in ESP
I have gone back and forth on whether bootupd should *exclusively* own `/boot/efi` or not. In the "adopted" case, we clearly cannot. Let's be conservative for now and change the way we compute diffs to be "relative" - this is a cleaner approach than "compute diff but ignore removals".
This commit is contained in:
@@ -395,7 +395,7 @@ fn compute_status_efi(
|
||||
});
|
||||
};
|
||||
let fsdiff = if let Some(saved_filesystem) = saved.filesystem.as_ref() {
|
||||
Some(esptree.diff(&saved_filesystem)?)
|
||||
Some(saved_filesystem.changes(&esptree)?)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -403,11 +403,7 @@ fn compute_status_efi(
|
||||
let (installed, installed_digest) = {
|
||||
let content = InstalledContent::from_file_tree(esptree);
|
||||
let drift = if let Some(fsdiff) = fsdiff {
|
||||
if saved.adopted {
|
||||
!fsdiff.is_only_removals()
|
||||
} else {
|
||||
fsdiff.count() > 0
|
||||
}
|
||||
fsdiff.count() > 0
|
||||
} else {
|
||||
// TODO detect state outside of filesystem tree
|
||||
false
|
||||
@@ -425,11 +421,9 @@ fn compute_status_efi(
|
||||
|
||||
let update_esp = efi_content_version_from_ostree(sysroot_path)?;
|
||||
let update_esp_tree = update_esp.content.filesystem.as_ref().unwrap();
|
||||
let update = if !saved.adopted && update_esp.content.digest == installed_digest {
|
||||
ComponentUpdate::LatestUpdateInstalled
|
||||
} else {
|
||||
let diff = installed_tree.diff(update_esp_tree)?;
|
||||
if saved.adopted && diff.is_only_removals() {
|
||||
let update = {
|
||||
let diff = installed_tree.updates(&update_esp_tree)?;
|
||||
if diff.count() == 0 {
|
||||
ComponentUpdate::LatestUpdateInstalled
|
||||
} else {
|
||||
ComponentUpdate::Available(Box::new(ComponentUpdateAvailable {
|
||||
|
||||
@@ -54,10 +54,6 @@ impl FileTreeDiff {
|
||||
pub(crate) fn count(&self) -> usize {
|
||||
self.additions.len() + self.removals.len() + self.changes.len()
|
||||
}
|
||||
|
||||
pub(crate) fn is_only_removals(&self) -> bool {
|
||||
self.count() == self.removals.len()
|
||||
}
|
||||
}
|
||||
|
||||
impl FileMetadata {
|
||||
@@ -151,7 +147,25 @@ impl FileTree {
|
||||
}
|
||||
|
||||
/// Determine the changes *from* self to the updated tree
|
||||
#[allow(dead_code)] // Used in tests and kept for completeness
|
||||
pub(crate) fn diff(&self, updated: &Self) -> Result<FileTreeDiff> {
|
||||
self.diff_impl(updated, true)
|
||||
}
|
||||
|
||||
/// Determine any changes only using the files tracked in self as
|
||||
/// a reference. In other words, this will ignore any unknown
|
||||
/// files and not count them as additions.
|
||||
pub(crate) fn changes(&self, current: &Self) -> Result<FileTreeDiff> {
|
||||
self.diff_impl(current, false)
|
||||
}
|
||||
|
||||
/// The inverse of `changes` - determine if there are any files
|
||||
/// changed or added in `current` compared to self.
|
||||
pub(crate) fn updates(&self, current: &Self) -> Result<FileTreeDiff> {
|
||||
current.diff_impl(self, false)
|
||||
}
|
||||
|
||||
fn diff_impl(&self, updated: &Self, check_additions: bool) -> Result<FileTreeDiff> {
|
||||
let mut additions = HashSet::new();
|
||||
let mut removals = HashSet::new();
|
||||
let mut changes = HashSet::new();
|
||||
@@ -165,11 +179,13 @@ impl FileTree {
|
||||
removals.insert(k.clone());
|
||||
}
|
||||
}
|
||||
for k in updated.children.keys() {
|
||||
if self.children.get(k).is_some() {
|
||||
continue;
|
||||
if check_additions {
|
||||
for k in updated.children.keys() {
|
||||
if self.children.get(k).is_some() {
|
||||
continue;
|
||||
}
|
||||
additions.insert(k.clone());
|
||||
}
|
||||
additions.insert(k.clone());
|
||||
}
|
||||
Ok(FileTreeDiff {
|
||||
additions,
|
||||
@@ -405,6 +421,13 @@ mod tests {
|
||||
let diff = run_diff(&a, &b)?;
|
||||
assert_eq!(diff.count(), 1);
|
||||
assert_eq!(diff.removals.len(), 1);
|
||||
let ta = FileTree::new_from_dir(&a)?;
|
||||
let tb = FileTree::new_from_dir(&b)?;
|
||||
let cdiff = ta.changes(&tb)?;
|
||||
assert_eq!(cdiff.count(), 1);
|
||||
assert_eq!(cdiff.removals.len(), 1);
|
||||
let udiff = ta.updates(&tb)?;
|
||||
assert_eq!(udiff.count(), 0);
|
||||
test_apply(&pa, &pb).context("testing apply 1")?;
|
||||
|
||||
b.create_dir("foo", 0o755)?;
|
||||
|
||||
Reference in New Issue
Block a user