treefile: Fix hashing of externals

See https://github.com/coreos/rpm-ostree/pull/2206#issuecomment-721372634

The commit 7f579a55d3fb7ec1cb9f74f8ec6bc36675df2ccc broke hashing
of overlay commits; this is a super evil bug because it causes us
to silently do the wrong thing.

The cause here is the GLib bindings don't (AFAICS) support getting
a `&mut` for a GLib boxed value.

Move all of the treefile checksum code into one place - this is
far saner.  The reason I didn't do this before is that it
will cause a spurious rebuild when one updates rpm-ostree, but...eh.
This commit is contained in:
Colin Walters 2020-11-03 21:29:59 +00:00 committed by OpenShift Merge Robot
parent ed44037155
commit 6ccf0e06bb
2 changed files with 32 additions and 36 deletions

View File

@ -45,8 +45,6 @@ pub struct Treefile {
rojig_spec: Option<CUtf8Buf>, rojig_spec: Option<CUtf8Buf>,
serialized: CUtf8Buf, serialized: CUtf8Buf,
externals: TreefileExternals, externals: TreefileExternals,
// This is a checksum over *all* the treefile inputs (recursed treefiles + externals).
checksum: CUtf8Buf,
} }
// We only use this while parsing // We only use this while parsing
@ -433,13 +431,6 @@ impl Treefile {
_ => (None, None), _ => (None, None),
}; };
let serialized = Treefile::serialize_json_string(&parsed.config)?; let serialized = Treefile::serialize_json_string(&parsed.config)?;
// Notice we hash the *reserialization* of the final flattened treefile only so that e.g.
// comments/whitespace/hash table key reorderings don't trigger a respin. We could take
// this further by using a custom `serialize_with` for Vecs where ordering doesn't matter
// (or just sort the Vecs).
let mut hasher = glib::Checksum::new(glib::ChecksumType::Sha256);
parsed.config.hasher_update(&mut hasher)?;
parsed.externals.hasher_update(&mut hasher)?;
Ok(Box::new(Treefile { Ok(Box::new(Treefile {
primary_dfd: dfd, primary_dfd: dfd,
parsed: parsed.config, parsed: parsed.config,
@ -448,7 +439,6 @@ impl Treefile {
rojig_spec, rojig_spec,
serialized, serialized,
externals: parsed.externals, externals: parsed.externals,
checksum: CUtf8Buf::from_string(hasher.get_string().unwrap()),
})) }))
} }
@ -511,7 +501,15 @@ impl Treefile {
} }
} }
fn update_checksum(&self, repo: &ostree::Repo, checksum: &mut glib::Checksum) -> Result<()> { fn get_checksum(&self, repo: &ostree::Repo) -> Result<String> {
// Notice we hash the *reserialization* of the final flattened treefile only so that e.g.
// comments/whitespace/hash table key reorderings don't trigger a respin. We could take
// this further by using a custom `serialize_with` for Vecs where ordering doesn't matter
// (or just sort the Vecs).
let mut hasher = glib::Checksum::new(glib::ChecksumType::Sha256);
self.parsed.hasher_update(&mut hasher)?;
self.externals.hasher_update(&mut hasher)?;
let it = self.parsed.ostree_layers.iter().flat_map(|x| x.iter()); let it = self.parsed.ostree_layers.iter().flat_map(|x| x.iter());
let it = it.chain( let it = it.chain(
self.parsed self.parsed
@ -519,14 +517,17 @@ impl Treefile {
.iter() .iter()
.flat_map(|x| x.iter()), .flat_map(|x| x.iter()),
); );
for v in it { for v in it {
let rev = repo.resolve_rev(v, false)?; let rev = repo.resolve_rev(v, false)?;
let (commit, _) = repo.load_commit(rev.as_str())?; let rev = rev.as_str();
let (commit, _) = repo.load_commit(rev)?;
let content_checksum = let content_checksum =
ostree::commit_get_content_checksum(&commit).expect("content checksum"); ostree::commit_get_content_checksum(&commit).expect("content checksum");
checksum.update(content_checksum.as_bytes()); let content_checksum = content_checksum.as_str();
hasher.update(content_checksum.as_bytes());
} }
Ok(()) Ok(hasher.get_string().expect("hash"))
} }
/// Generate a rojig spec file. /// Generate a rojig spec file.
@ -1506,19 +1507,6 @@ mod ffi {
ret.to_glib_full() ret.to_glib_full()
} }
#[no_mangle]
pub extern "C" fn ror_treefile_update_checksum(
tf: *mut Treefile,
repo: *mut ostree_sys::OstreeRepo,
checksum: *mut glib_sys::GChecksum,
gerror: *mut *mut glib_sys::GError,
) -> libc::c_int {
let repo: ostree::Repo = unsafe { from_glib_none(repo) };
let mut checksum: glib::Checksum = unsafe { from_glib_none(checksum) };
let tf = ref_from_raw_ptr(tf);
int_glib_error(tf.update_checksum(&repo, &mut checksum), gerror)
}
#[no_mangle] #[no_mangle]
pub extern "C" fn ror_treefile_get_rojig_spec_path(tf: *mut Treefile) -> *const libc::c_char { pub extern "C" fn ror_treefile_get_rojig_spec_path(tf: *mut Treefile) -> *const libc::c_char {
let tf = ref_from_raw_ptr(tf); let tf = ref_from_raw_ptr(tf);
@ -1539,9 +1527,20 @@ mod ffi {
} }
#[no_mangle] #[no_mangle]
pub extern "C" fn ror_treefile_get_checksum(tf: *mut Treefile) -> *const libc::c_char { pub extern "C" fn ror_treefile_get_checksum(
tf: *mut Treefile,
repo: *mut ostree_sys::OstreeRepo,
gerror: *mut *mut glib_sys::GError,
) -> *mut libc::c_char {
let tf = ref_from_raw_ptr(tf); let tf = ref_from_raw_ptr(tf);
tf.checksum.as_ptr() let repo: ostree::Repo = unsafe { from_glib_none(repo) };
match tf.get_checksum(&repo) {
Ok(c) => c.to_glib_full(),
Err(ref e) => {
error_to_glib(e, gerror);
ptr::null_mut()
}
}
} }
#[no_mangle] #[no_mangle]

View File

@ -61,18 +61,15 @@ rpmostree_composeutil_checksum (HyGoal goal,
/* Hash in the treefile inputs (this includes all externals like postprocess, add-files, /* Hash in the treefile inputs (this includes all externals like postprocess, add-files,
* etc... and the final flattened treefile -- see treefile.rs for more details). */ * etc... and the final flattened treefile -- see treefile.rs for more details). */
g_checksum_update (checksum, (guint8*)ror_treefile_get_checksum (tf), -1); g_autofree char *tf_checksum = ror_treefile_get_checksum (tf, repo, error);
if (!tf_checksum)
return FALSE;
g_checksum_update (checksum, (guint8*) tf_checksum, -1);
/* Hash in each package */ /* Hash in each package */
if (!rpmostree_dnf_add_checksum_goal (checksum, goal, NULL, error)) if (!rpmostree_dnf_add_checksum_goal (checksum, goal, NULL, error))
return FALSE; return FALSE;
if (tf)
{
if (!ror_treefile_update_checksum (tf, repo, checksum, error))
return FALSE;
}
*out_checksum = g_strdup (g_checksum_get_string (checksum)); *out_checksum = g_strdup (g_checksum_get_string (checksum));
return TRUE; return TRUE;
} }