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

app: Unify some cmdline txn processing #1034

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

We had duplicated code across the cmdline entrypoints for transaction
processing; things like "print pkg diff only if !opt_reboot".

This doesn't dedup all of them - there are some corner cases around
the preview logic in upgrade, and initramfs also need special
handling. I'll likely enhance this further down the line for that.

But one reason I'm doing this now is prep for:
rpm-ostree cancel

Basically, I want to add a -B/--background option we honor consistently, and
that'd be a lot easier if we have a combined "start/monitor txn" with the
post-txn option processing in one place.

@jlebon
Copy link
Member

jlebon commented Oct 3, 2017

bot, retest this please

@rh-atomic-bot
Copy link

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

@jlebon
Copy link
Member

jlebon commented Oct 3, 2017

Looks good to me! Just needs a rebase.

We had duplicated code across the cmdline entrypoints for transaction
processing; things like "print pkg diff only if !opt_reboot".

This doesn't dedup all of them - there are some corner cases around
the preview logic in `upgrade`, and `initramfs` also need special
handling.  I'll likely enhance this further down the line for that.

But one reason I'm doing this now is prep for:
[rpm-ostree cancel](coreos#1019)

Basically, I want to add a `-B/--background` option we honor consistently, and
that'd be a lot easier if we have a combined "start/monitor txn" with the
post-txn option processing in one place.
@cgwalters
Copy link
Member Author

Rebased 🏄

@jlebon
Copy link
Member

jlebon commented Oct 4, 2017

@rh-atomic-bot r+ 5c80fbc

@rh-atomic-bot
Copy link

⌛ Testing commit 5c80fbc with merge f3c63b6...

@rh-atomic-bot
Copy link

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

jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Oct 5, 2017
It's no longer being built and is now older than the latest CentOS AH
release. This should help us no longer see messages like:

(rpm-ostree pkg-add:5662): GLib-CRITICAL **: g_variant_dict_lookup:
assertion 'is_valid_dict (dict)' failed

which happen because in coreos#1034, we started using `G_VARIANT_DICT_INIT`,
whose special magic values only make sense in glib2 >= 2.50. (The alpha
image stopped at 2.46).

Saw this while debugging coreos#1035.
rh-atomic-bot pushed a commit that referenced this pull request Oct 6, 2017
It's no longer being built and is now older than the latest CentOS AH
release. This should help us no longer see messages like:

(rpm-ostree pkg-add:5662): GLib-CRITICAL **: g_variant_dict_lookup:
assertion 'is_valid_dict (dict)' failed

which happen because in #1034, we started using `G_VARIANT_DICT_INIT`,
whose special magic values only make sense in glib2 >= 2.50. (The alpha
image stopped at 2.46).

Saw this while debugging #1035.

Closes: #1040
Approved by: cgwalters
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.

3 participants