keremet/rpm-ostree
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

lockfile: Return LockfileConfig rather than Vec<StringMapping>

Browse Source 
I don't like the use of `HY_GLOB` in the lockfile package matching. We
have all the information in the Rust object, so it's silly to condense
that to a single string in a hashmap.

Fix this by returning the `LockfileConfig` object itself and then adding
a function to fetch the list of locked packages. This allows the C++
side to see all the individual fields which makes filtering trivial.

The next step is moving all the code which needs the lockfile to Rust.
Then we can drop the shared `LockedPackage` type.

(I did start on converting `find_locked_packages`, though it requires
adding bindings for all the `HyQuery` stuff, which...  isn't great (and
also runs into the fact that `hy_query_run` needs to return a
GPtrArray). I think instead of a 1:1 mapping, we'll probably want the
libdnf-sys API wrappers to provide some sugar for the common paths.)
This commit is contained in:
Jonathan Lebon 2021-03-09 15:58:30 -05:00 committed by OpenShift Merge Robot
parent c03f5d50a3
commit 5f17bc5c87
6 changed files with 89 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
rust/src/lib.rs
View File

@ -263,14 +263,25 @@ pub mod ffi {
fn serialize_to_dir(&self, output_dir: &str) -> Result<()>;
}
struct LockedPackage {
name: String,
evr: String,
arch: String,
digest: String,
}
// lockfile.rs
extern "Rust" {
fn lockfile_read(filenames: &Vec<String>) -> Result<Vec<StringMapping>>;
type LockfileConfig;
fn lockfile_read(filenames: &Vec<String>) -> Result<Box<LockfileConfig>>;
fn lockfile_write(
filename: &str,
packages: Pin<&mut CxxGObjectArray>,
rpmmd_repos: Pin<&mut CxxGObjectArray>,
) -> Result<()>;
fn get_locked_packages(&self) -> Result<Vec<LockedPackage>>;
}
// rpmutils.rs

64
rust/src/lockfile.rs
View File

@ -12,7 +12,7 @@
pub use self::ffi::*;
use crate::utils;
use anyhow::Result;
use anyhow::{anyhow, Result};
use chrono::prelude::*;
use openat_ext::OpenatDirExt;
use serde_derive::{Deserialize, Serialize};
@ -23,6 +23,9 @@ use std::iter::Extend;
use std::path::Path;
use std::pin::Pin;
use crate::cxxrsutil::CxxResult;
use libdnf_sys::*;
/// Given a lockfile filename, parse it
fn lockfile_parse<P: AsRef<Path>>(filename: P) -> Result<LockfileConfig> {
let filename = filename.as_ref();
@ -90,7 +93,7 @@ fn lockfile_parse_multiple<P: AsRef<Path>>(filenames: &[P]) -> Result<LockfileCo
///
#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
struct LockfileConfig {
pub(crate) struct LockfileConfig {
packages: BTreeMap<String, LockedPackage>,
metadata: Option<LockfileConfigMetadata>,
}
@ -120,25 +123,37 @@ enum LockedPackage {
},
}
impl LockedPackage {
fn evra_glob(&self) -> String {
match self {
LockedPackage::Evr { evr, digest: _ } => format!("{}.*", evr),
LockedPackage::Evra { evra, digest: _ } => evra.into(),
}
}
fn digest(&self) -> String {
match self {
LockedPackage::Evr { evr: _, digest } => digest.clone().unwrap_or_default(),
LockedPackage::Evra { evra: _, digest } => digest.clone().unwrap_or_default(),
}
}
}
impl LockfileConfig {
fn merge(&mut self, other: LockfileConfig) {
self.packages.extend(other.packages);
}
pub(crate) fn get_locked_packages(&self) -> CxxResult<Vec<crate::ffi::LockedPackage>> {
self.packages
.iter()
.map(|(k, v)| match v {
LockedPackage::Evr { evr, digest } => Ok(crate::ffi::LockedPackage {
name: k.clone(),
evr: evr.clone(),
arch: "".into(),
digest: digest.clone().unwrap_or_default(),
}),
LockedPackage::Evra { evra, digest } => {
let evr_arch: Vec<&str> = evra.rsplitn(2, '.').collect();
if evr_arch.len() != 2 {
Err(anyhow!("package {} has malformed evra: {}", k, evra).into())
} else {
Ok(crate::ffi::LockedPackage {
name: k.clone(),
evr: evr_arch[1].into(),
arch: evr_arch[0].into(),
digest: digest.clone().unwrap_or_default(),
})
}
}
})
.collect()
}
}
#[cfg(test)]
@ -238,19 +253,8 @@ mod tests {
}
}
use crate::cxxrsutil::CxxResult;
use crate::ffi::*;
use libdnf_sys::*;
pub(crate) fn lockfile_read(filenames: &Vec<String>) -> CxxResult<Vec<StringMapping>> {
Ok(lockfile_parse_multiple(&filenames)?
.packages
.into_iter()
.map(|(k, v)| StringMapping {
k: format!("{}-{}", k, v.evra_glob()),
v: v.digest(),
})
.collect())
pub(crate) fn lockfile_read(filenames: &Vec<String>) -> CxxResult<Box<LockfileConfig>> {
Ok(Box::new(lockfile_parse_multiple(&filenames)?))
}
pub(crate) fn lockfile_write(

9
src/app/rpmostree-compose-builtin-tree.cxx
View File

@ -704,14 +704,7 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr,
if (opt_lockfiles)
{
rust::Vec<rust::String> lockfiles;
for (char **it = opt_lockfiles; it && *it; it++)
lockfiles.push_back(std::string(*it));
auto mappings = rpmostreecxx::lockfile_read (lockfiles);
g_autoptr(GHashTable) map = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
for (auto & mapping : mappings)
g_hash_table_insert (map, g_strdup (mapping.k.c_str()), g_strdup (mapping.v.c_str()));
rpmostree_context_set_vlockmap (self->corectx, map, opt_lockfile_strict);
rpmostree_context_set_lockfile (self->corectx, opt_lockfiles, opt_lockfile_strict);
g_print ("Loaded lockfiles:\n %s\n", g_strjoinv ("\n ", opt_lockfiles));
}

8
src/libpriv/rpmostree-core-private.h
View File

@ -20,6 +20,8 @@
#pragma once
#include <optional>
#include "libglnx.h"
#include "rpmostree-rojig-core.h"
#include "rpmostree-core.h"
@ -77,8 +79,8 @@ struct _RpmOstreeContext {
GHashTable *pkgs_to_remove; /* pkgname --> gv_nevra */
GHashTable *pkgs_to_replace; /* new gv_nevra --> old gv_nevra */
GHashTable *vlockmap; /* nevra --> repochecksum */
gboolean vlockmap_strict;
std::optional<rust::Box<rpmostreecxx::LockfileConfig>> lockfile;
gboolean lockfile_strict;
GLnxTmpDir tmpdir;
@ -89,4 +91,4 @@ struct _RpmOstreeContext {
GLnxTmpDir repo_tmpdir; /* Used to assemble+commit if no base rootfs provided */
};
G_END_DECLS
G_END_DECLS

61
src/libpriv/rpmostree-core.cxx
View File

@ -355,8 +355,6 @@ rpmostree_context_finalize (GObject *object)
g_clear_pointer (&rctx->pkgs_to_remove, g_hash_table_unref);
g_clear_pointer (&rctx->pkgs_to_replace, g_hash_table_unref);
g_clear_pointer (&rctx->vlockmap, g_hash_table_unref);
(void)glnx_tmpdir_delete (&rctx->tmpdir, NULL, NULL);
(void)glnx_tmpdir_delete (&rctx->repo_tmpdir, NULL, NULL);
@ -796,7 +794,7 @@ rpmostree_context_setup (RpmOstreeContext *self,
/* only enable lockfile-repos if we actually have a lockfile so we don't even waste
* time fetching metadata */
if (self->vlockmap)
if (self->lockfile)
{
g_autofree char **enabled_lockfile_repos = NULL;
if (g_variant_dict_lookup (self->spec->dict, "lockfile-repos", "^a&s", &enabled_lockfile_repos))
@ -1912,17 +1910,20 @@ static GPtrArray*
find_locked_packages (RpmOstreeContext *self,
GError **error)
{
g_assert (self->vlockmap);
g_assert (self->lockfile);
DnfContext *dnfctx = self->dnfctx;
DnfSack *sack = dnf_context_get_sack (dnfctx);
g_autoptr(GPtrArray) pkgs =
g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
GLNX_HASH_TABLE_FOREACH_KV (self->vlockmap, const char*, nevra, const char*, chksum)
auto locked_pkgs = (*self->lockfile)->get_locked_packages();
for (auto & pkg : locked_pkgs)
{
g_assert (chksum);
hy_autoquery HyQuery query = hy_query_create (sack);
hy_query_filter (query, HY_PKG_NEVRA, HY_GLOB, nevra);
hy_query_filter (query, HY_PKG_NAME, HY_EQ, pkg.name.c_str());
hy_query_filter (query, HY_PKG_EVR, HY_EQ, pkg.evr.c_str());
if (pkg.arch.length() > 0)
hy_query_filter (query, HY_PKG_ARCH, HY_EQ, pkg.arch.c_str());
g_autoptr(GPtrArray) matches = hy_query_run (query);
gboolean at_least_one = FALSE;
@ -1930,7 +1931,7 @@ find_locked_packages (RpmOstreeContext *self,
for (guint i = 0; i < matches->len; i++)
{
auto match = static_cast<DnfPackage *>(matches->pdata[i]);
if (!*chksum)
if (pkg.digest.length() == 0)
{
/* we could optimize this path outside the loop in the future using the
* g_ptr_array_extend_and_steal API, though that's still too new for e.g. el8 */
@ -1940,7 +1941,7 @@ find_locked_packages (RpmOstreeContext *self,
else
{
auto repodata_chksum = rpmostreecxx::get_repodata_chksum_repr(*match);
if (!g_str_equal (chksum, repodata_chksum.c_str()))
if (pkg.digest != repodata_chksum) /* we're comparing two rust::String here */
n_checksum_mismatches++;
else
{
@ -1950,10 +1951,17 @@ find_locked_packages (RpmOstreeContext *self,
}
}
if (!at_least_one)
return (GPtrArray*)glnx_null_throw (error, "Couldn't find locked package '%s'%s%s "
"(pkgs matching NEVRA: %d; mismatched checksums: %d)",
nevra, *chksum ? " with checksum " : "",
*chksum ? chksum : "", matches->len, n_checksum_mismatches);
{
g_autofree char *spec =
g_strdup_printf ("%s-%s%s%s", pkg.name.c_str(), pkg.evr.c_str(),
pkg.arch.length() > 0 ? "." : "",
pkg.arch.length() > 0 ? pkg.arch.c_str() : "");
return (GPtrArray*)glnx_null_throw (error, "Couldn't find locked package '%s'%s%s "
"(pkgs matching NEVRA: %d; mismatched checksums: %d)",
spec, pkg.digest.length() > 0 ? " with checksum " : "",
pkg.digest.length() > 0 ? pkg.digest.c_str() : "",
matches->len, n_checksum_mismatches);
}
}
return util::move_nullify (pkgs);
@ -2015,7 +2023,7 @@ rpmostree_context_prepare (RpmOstreeContext *self,
g_assert_cmpint (g_strv_length (removed_base_pkgnames), ==, 0);
}
if (self->vlockmap)
if (self->lockfile)
{
/* we only support pure installs for now (compose case) */
g_assert (!self->rojig_pure);
@ -2085,7 +2093,7 @@ rpmostree_context_prepare (RpmOstreeContext *self,
* uninstall. We don't want to mix those two steps, otherwise we might confuse libdnf,
* see: https://github.com/rpm-software-management/libdnf/issues/700 */
if (self->vlockmap != NULL)
if (self->lockfile)
{
/* first, find our locked pkgs in the rpmmd */
g_autoptr(GPtrArray) locked_pkgs = find_locked_packages (self, error);
@ -2097,7 +2105,7 @@ rpmostree_context_prepare (RpmOstreeContext *self,
for (guint i = 0; i < locked_pkgs->len; i++)
dnf_packageset_add (locked_pset, static_cast<DnfPackage*>(locked_pkgs->pdata[i]));
if (self->vlockmap_strict)
if (self->lockfile_strict)
{
/* In strict mode, we basically *only* want locked packages to be considered, so
* exclude everything else. Note we still don't directly do `hy_goal_install`
@ -2135,13 +2143,11 @@ rpmostree_context_prepare (RpmOstreeContext *self,
/* map of all packages with names found in the lockfile */
DnfPackageSet *named_pkgs = dnf_packageset_new (sack);
Map *named_pkgs_map = dnf_packageset_get_map (named_pkgs);
GLNX_HASH_TABLE_FOREACH (self->vlockmap, const char*, nevra)
auto locked_pkgs = (*self->lockfile)->get_locked_packages();
for (auto & pkg : locked_pkgs)
{
g_autofree char *name = NULL;
if (!rpmostree_decompose_nevra (nevra, &name, NULL, NULL, NULL, NULL, error))
return FALSE;
hy_autoquery HyQuery query = hy_query_create (sack);
hy_query_filter (query, HY_PKG_NAME, HY_EQ, name);
hy_query_filter (query, HY_PKG_NAME, HY_EQ, pkg.name.c_str());
DnfPackageSet *pset = hy_query_run_set (query);
map_or (named_pkgs_map, dnf_packageset_get_map (pset));
dnf_packageset_free (pset);
@ -2315,13 +2321,16 @@ rpmostree_context_get_packages_to_import (RpmOstreeContext *self)
/* Note this must be called *before* rpmostree_context_setup(). */
void
rpmostree_context_set_vlockmap (RpmOstreeContext *self,
GHashTable *map,
rpmostree_context_set_lockfile (RpmOstreeContext *self,
char **lockfiles,
gboolean strict)
{
g_clear_pointer (&self->vlockmap, (GDestroyNotify)g_hash_table_unref);
self->vlockmap = g_hash_table_ref (map);
self->vlockmap_strict = strict;
g_assert (!self->lockfile);
rust::Vec<rust::String> rs_lockfiles;
for (char **it = lockfiles; it && *it; it++)
rs_lockfiles.push_back(std::string(*it));
self->lockfile = rpmostreecxx::lockfile_read(rs_lockfiles);
self->lockfile_strict = strict;
}
/* XXX: push this into libdnf */

4
src/libpriv/rpmostree-core.h
View File

@ -182,8 +182,8 @@ gboolean rpmostree_context_set_packages (RpmOstreeContext *self,
GPtrArray *rpmostree_context_get_packages_to_import (RpmOstreeContext *self);
void
rpmostree_context_set_vlockmap (RpmOstreeContext *self,
GHashTable *map,
rpmostree_context_set_lockfile (RpmOstreeContext *self,
char **lockfiles,
gboolean strict);
gboolean rpmostree_download_packages (GPtrArray *packages,