Skip to content
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.

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

rust/lockfile: Add more metadata to generated lockfiles #1938

Merged
merged 2 commits into from
Nov 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 35 additions & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ systemd = "0.4.0"
indicatif = "0.11.0"
lazy_static = "1.1.0"
envsubst = "0.1.0"
chrono = { version = "0.4.9", features = ["serde"] }

[lib]
name = "rpmostree_rust"
Expand Down
5 changes: 5 additions & 0 deletions rust/src/libdnf_sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@ use glib_sys;

#[repr(C)]
pub(crate) struct DnfPackage(libc::c_void);
#[repr(C)]
pub(crate) struct DnfRepo(libc::c_void);

#[allow(dead_code)]
extern {
pub(crate) fn dnf_package_get_nevra(package: *mut DnfPackage) -> *const libc::c_char;
pub(crate) fn dnf_package_get_name(package: *mut DnfPackage) -> *const libc::c_char;
pub(crate) fn dnf_package_get_evr(package: *mut DnfPackage) -> *const libc::c_char;
pub(crate) fn dnf_package_get_arch(package: *mut DnfPackage) -> *const libc::c_char;

pub(crate) fn dnf_repo_get_id(repo: *mut DnfRepo) -> *const libc::c_char;
pub(crate) fn dnf_repo_get_timestamp_generated(repo: *mut DnfRepo) -> u64;
}

/* And some helper rpm-ostree C functions to deal with libdnf stuff. These are prime candidates for
Expand Down
50 changes: 50 additions & 0 deletions rust/src/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use std::collections::{HashMap, BTreeMap};
use std::iter::Extend;
use std::path::Path;
use std::io;
use chrono::prelude::*;
use std::convert::TryInto;

use crate::utils;

Expand Down Expand Up @@ -49,6 +51,18 @@ fn lockfile_parse_multiple<P: AsRef<Path>>(filenames: &[P]) -> Fallible<Lockfile
///
/// ```
/// {
/// "metatada": {
/// "generated": "<rfc3339-timestamp>",
/// "rpmmd_repos": {
/// "repo1": {
/// "generated": "<rfc3339-timestamp>"
/// },
/// "repo2": {
/// "generated": "<rfc3339-timestamp>"
/// },
/// ...
/// }
/// }
/// "packages": {
/// "name1": {
/// "evra": "EVRA1",
Expand All @@ -70,6 +84,19 @@ fn lockfile_parse_multiple<P: AsRef<Path>>(filenames: &[P]) -> Fallible<Lockfile
#[serde(deny_unknown_fields)]
struct LockfileConfig {
packages: BTreeMap<String, LockedPackage>,
metadata: Option<LockfileConfigMetadata>,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as this lands in cosa, it will force everyone to upgrade rpm-ostree. I think that's OK. Just noting.

struct LockfileConfigMetadata {
generated: Option<DateTime<Utc>>,
rpmmd_repos: Option<BTreeMap<String, LockfileRepoMetadata>>,
}

#[derive(Serialize, Deserialize, Debug)]
struct LockfileRepoMetadata {
generated: DateTime<Utc>,
}

#[derive(Serialize, Deserialize, Debug)]
Expand Down Expand Up @@ -197,13 +224,25 @@ mod ffi {
pub extern "C" fn ror_lockfile_write(
filename: *const libc::c_char,
packages: *mut glib_sys::GPtrArray,
rpmmd_repos: *mut glib_sys::GPtrArray,
gerror: *mut *mut glib_sys::GError,
) -> libc::c_int {
let filename = ffi_view_os_str(filename);
let packages: Vec<*mut DnfPackage> = ffi_ptr_array_to_vec(packages);
let rpmmd_repos: Vec<*mut DnfRepo> = ffi_ptr_array_to_vec(rpmmd_repos);

// get current time, but scrub nanoseconds; it's overkill to serialize that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we instead something like reference the last time the file changed in git? Dunno.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh yeah, that's in fact exactly what I initially did (see the commit message in coreos/fedora-coreos-releng-automation@c932c34). Though there were concerns that it was too magic/implicit.

let now = {
let t = Utc::now();
Utc::today().and_hms_nano(t.hour(), t.minute(), t.second(), 0)
};

let mut lockfile = LockfileConfig {
packages: BTreeMap::new(),
metadata: Some(LockfileConfigMetadata {
generated: Some(now),
rpmmd_repos: Some(BTreeMap::new()),
})
};

for pkg in packages {
Expand All @@ -226,6 +265,17 @@ mod ffi {
unsafe { glib_sys::g_free(chksum as *mut libc::c_void) };
}

/* just take the ref here to be less verbose */
let lockfile_repos = lockfile.metadata.as_mut().unwrap().rpmmd_repos.as_mut().unwrap();

for rpmmd_repo in rpmmd_repos {
let id = ffi_new_string(unsafe { dnf_repo_get_id(rpmmd_repo) });
let generated = unsafe { dnf_repo_get_timestamp_generated(rpmmd_repo) };
lockfile_repos.insert(id, LockfileRepoMetadata {
generated: Utc.timestamp(generated.try_into().unwrap(), 0),
});
}

int_glib_error(utils::write_file(filename, |w| {
serde_json::to_writer_pretty(w, &lockfile).map_err(failure::Error::from)
}), gerror)
Expand Down
5 changes: 4 additions & 1 deletion src/app/rpmostree-compose-builtin-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,10 @@ install_packages (RpmOstreeTreeComposeContext *self,
g_autoptr(GPtrArray) pkgs = rpmostree_context_get_packages (self->corectx);
g_assert (pkgs);

if (!ror_lockfile_write (opt_write_lockfile, pkgs, error))
g_autoptr(GPtrArray) rpmmd_repos =
rpmostree_get_enabled_rpmmd_repos (rpmostree_context_get_dnf (self->corectx),
DNF_REPO_ENABLED_PACKAGES);
if (!ror_lockfile_write (opt_write_lockfile, pkgs, rpmmd_repos, error))
return FALSE;
}

Expand Down
25 changes: 6 additions & 19 deletions src/libpriv/rpmostree-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,21 +215,6 @@ add_canonicalized_string_array (GVariantBuilder *builder,
g_variant_new_strv ((const char*const*)sorted, count));
}

static GPtrArray *
get_enabled_rpmmd_repos (DnfContext *dnfctx, DnfRepoEnabled enablement)
{
g_autoptr(GPtrArray) ret = g_ptr_array_new ();
GPtrArray *repos = dnf_context_get_repos (dnfctx);

for (guint i = 0; i < repos->len; i++)
{
DnfRepo *repo = repos->pdata[i];
if (dnf_repo_get_enabled (repo) & enablement)
g_ptr_array_add (ret, repo);
}
return g_steal_pointer (&ret);
}

/* Get a bool from @keyfile, adding it to @builder */
static void
tf_bind_boolean (GKeyFile *keyfile,
Expand Down Expand Up @@ -611,7 +596,8 @@ rpmostree_context_get_rpmmd_repo_commit_metadata (RpmOstreeContext *self)
{
g_auto(GVariantBuilder) repo_list_builder;
g_variant_builder_init (&repo_list_builder, (GVariantType*)"aa{sv}");
g_autoptr(GPtrArray) repos = get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
g_autoptr(GPtrArray) repos =
rpmostree_get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
for (guint i = 0; i < repos->len; i++)
{
g_auto(GVariantBuilder) repo_builder;
Expand Down Expand Up @@ -774,7 +760,8 @@ rpmostree_context_setup (RpmOstreeContext *self,
return FALSE;
}

g_autoptr(GPtrArray) repos = get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
g_autoptr(GPtrArray) repos =
rpmostree_get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
if (repos->len == 0 && !self->pkgcache_only)
{
/* To be nice, let's only make this fatal if "packages" is empty (e.g. if
Expand Down Expand Up @@ -1058,7 +1045,7 @@ rpmostree_context_download_metadata (RpmOstreeContext *self,
dnf_context_set_enable_filelists (self->dnfctx, FALSE);

g_autoptr(GPtrArray) rpmmd_repos =
get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
rpmostree_get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);

if (self->pkgcache_only)
{
Expand Down Expand Up @@ -1183,7 +1170,7 @@ journal_rpmmd_info (RpmOstreeContext *self)
* system ""up-to-dateness"".
*/
{ g_autoptr(GPtrArray) repos =
get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
rpmostree_get_enabled_rpmmd_repos (self->dnfctx, DNF_REPO_ENABLED_PACKAGES);
g_autoptr(GString) enabled_repos = g_string_new ("");
g_autoptr(GString) enabled_repos_solvables = g_string_new ("");
g_autoptr(GString) enabled_repos_timestamps = g_string_new ("");
Expand Down
16 changes: 16 additions & 0 deletions src/libpriv/rpmostree-rpm-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1455,3 +1455,19 @@ rpmostree_get_rojig_branch_pkg (DnfPackage *pkg)
dnf_package_get_evr (pkg),
dnf_package_get_arch (pkg));
}

GPtrArray*
rpmostree_get_enabled_rpmmd_repos (DnfContext *dnfctx,
DnfRepoEnabled enablement)
{
g_autoptr(GPtrArray) ret = g_ptr_array_new ();
GPtrArray *repos = dnf_context_get_repos (dnfctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One side note...from the current libdnf source I'm not sure this order is stable. They are pulled unordered from g_dir_read_name() and passed to g_ptr_array_sort() which is guaranteed stable, but we're not usually adding cost, so the array will remain unordered.

We should probably do something more like "sort by cost, then name" or something in libdnf?

(We want this to be stable or otherwise we'll be reordering things in the JSON)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least as far as the JSON output, we use a BTreeMap on the repo name, so the order in which we serialize should already be stable (learned that lesson from #1865).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, OK.


for (guint i = 0; i < repos->len; i++)
{
DnfRepo *repo = repos->pdata[i];
if (dnf_repo_get_enabled (repo) & enablement)
g_ptr_array_add (ret, repo);
}
return g_steal_pointer (&ret);
}
3 changes: 3 additions & 0 deletions src/libpriv/rpmostree-rpm-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,6 @@ rpmostree_nevra_to_cache_branch (const char *nevra,
GError **error);
char *
rpmostree_cache_branch_to_nevra (const char *cachebranch);

GPtrArray *
rpmostree_get_enabled_rpmmd_repos (DnfContext *dnfctx, DnfRepoEnabled enablement);
4 changes: 3 additions & 1 deletion tests/compose-tests/test-lockfile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ assert_has_file "versions.lock"
assert_jq versions.lock \
'.packages["test-pkg"].evra = "1.0-1.x86_64"' \
'.packages["test-pkg-common"].evra = "1.0-1.x86_64"' \
'.packages["another-test-pkg"].evra = "1.0-1.x86_64"'
'.packages["another-test-pkg"].evra = "1.0-1.x86_64"' \
'.metadata.rpmmd_repos|length > 0' \
'.metadata.generated'
echo "ok lockfile created"
# Read lockfile back
build_rpm test-pkg-common version 2.0
Expand Down