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

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 11, 2019

E.g. the generation timestamp, repos that were enabled, and their
generation timestamps.

This is just generally useful, though I'd like to make use specifically
of the new metadata.generated key in FCOS to drive versioning:

coreos/fedora-coreos-releng-automation#50

E.g. the generation timestamp, repos that were enabled, and their
generation timestamps.

This is just generally useful, though I'd like to make use specifically
of the new `metadata.generated` key in FCOS to drive versioning:

coreos/fedora-coreos-releng-automation#50
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

/approve

}

#[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.

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.

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.

@cgwalters
Copy link
Member

/lgtm

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 2589cd1 into coreos:master Nov 12, 2019
@jlebon jlebon deleted the pr/lockfile-timestamp branch April 23, 2023 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants