Skip to content

Live updates prep ↨#709

Closed
cgwalters wants to merge 6 commits intocoreos:masterfrom
cgwalters:live-updates-prep
Closed

Live updates prep ↨#709
cgwalters wants to merge 6 commits intocoreos:masterfrom
cgwalters:live-updates-prep

Conversation

@cgwalters
Copy link
Copy Markdown
Member

@cgwalters cgwalters commented Mar 27, 2017

Prep for: #652

@rh-atomic-bot
Copy link
Copy Markdown

☔ The latest upstream changes (presumably 9c02342) made this pull request unmergeable. Please resolve the merge conflicts.

@cgwalters
Copy link
Copy Markdown
Member Author

bot, test this please

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 13, 2017

test comment

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 13, 2017

bot, retest this please

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 13, 2017

test comment

Copy link
Copy Markdown
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Looks good! Just a few nits.

rpmostree_syscore_get_origin_merge_deployment (OstreeSysroot *self, const char *osname)
{
g_autoptr(GPtrArray) deployments = ostree_sysroot_get_deployments (self);
guint i;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: this could be declared in the for loop initializer.

const gboolean is_booted = ostree_deployment_equal (deployment, booted_deployment);
const gboolean is_merge_or_booted = is_booted ||
ostree_deployment_equal (deployment, merge_deployment);
const gboolean is_last = i == (deployments->len - 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: can we wrap this 🌯 in parentheses?

@@ -385,6 +385,59 @@ rpmostree_syscore_add_deployment (OstreeSysroot *sysroot,
return g_steal_pointer (&new_deployments);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The commit message should be s/rpmostree_syscore_filter_deployments/rpmostree_syscore_add_deployment/ right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, looks like it. I rebased and reworded ⬇️

This is prep for livefs.  We need to tweak the logic from what
core libostree has in `ostree_sysroot_simple_write_deployment()`,
and while we could land improved logic there, I think it makes
sense to carry this here until we're confident enough in the logic
to make it ABI.

This does depend on a [new libostree API](ostreedev/ostree#745)
that allows writing deployments without doing cleanup.

The `bump_mtime()` bit is also prep for livefs, carrying in this patch to avoid
splitting things too much.
While nothing else besides cleanup right now would call this directly, the code
should live close to the other very similar function:
`rpmostree_syscore_add_deployment()`.
I want to use this in livefs, where I'll end up doing some diff
computations on the server and am currently rendering text there.

It might also be a step towards using this in `db diff`.
@cgwalters
Copy link
Copy Markdown
Member Author

OK, this one should be ready.

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 26, 2017

@rh-atomic-bot r+ 72b8dc8

@rh-atomic-bot
Copy link
Copy Markdown

⌛ Testing commit 72b8dc8 with merge 7d98e3e...

rh-atomic-bot pushed a commit that referenced this pull request Apr 26, 2017
While nothing else besides cleanup right now would call this directly, the code
should live close to the other very similar function:
`rpmostree_syscore_add_deployment()`.

Closes: #709
Approved by: jlebon
rh-atomic-bot pushed a commit that referenced this pull request Apr 26, 2017
We now avoid doing cleanup twice.

Closes: #709
Approved by: jlebon
rh-atomic-bot pushed a commit that referenced this pull request Apr 26, 2017
I want to use this in livefs, where I'll end up doing some diff
computations on the server and am currently rendering text there.

It might also be a step towards using this in `db diff`.

Closes: #709
Approved by: jlebon
@rh-atomic-bot
Copy link
Copy Markdown

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing 7d98e3e to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants