From a6c731f6e7d1014aab5989c98a6432d28080905b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 1 May 2016 14:22:52 -0400 Subject: [PATCH] libglnx porting: Migrate from GSConsole To GLnxConsoleRef. There were some subtleties here, for example we used to reference `GSConsole` inside the progress changed function, which at first seems like an ABI hazard, because e.g. rpm-ostree or xdg-app could still be passing a `GSConsole` instance there. Luckily, it turns out to be compatible to just start calling libglnx here. Another issue was that due to libglnx's use of the cleanup function, we needed to ensure we always called `ostree_async_progress_finish()` *before* the cleanup function was invoked. Closes: #280 Approved by: giuseppe --- libglnx | 2 +- src/libostree/ostree-repo.c | 11 +++-- src/ostree/ot-admin-builtin-switch.c | 40 +++++++--------- src/ostree/ot-admin-builtin-upgrade.c | 36 ++++++-------- src/ostree/ot-builtin-pull-local.c | 20 ++++---- src/ostree/ot-builtin-pull.c | 68 +++++++++++++-------------- 6 files changed, 81 insertions(+), 96 deletions(-) diff --git a/libglnx b/libglnx index 76952275..47ddbfa5 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 769522753c25537e520adc322fa62e5390272add +Subproject commit 47ddbfa56341df3a9453854e1101e1c2f2359ddb diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index b4d702a1..75bc5c2a 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -27,6 +27,7 @@ #include #include #include "otutil.h" +#include #include "ostree-core-private.h" #include "ostree-repo-private.h" @@ -4120,12 +4121,15 @@ ostree_repo_pull_with_options (OstreeRepo *self, * custom status message, or else outstanding fetch progress in bytes/sec, * or else outstanding content or metadata writes to the repository in * number of objects. + * + * Compatibility note: this function previously assumed that @user_data + * was a pointer to a #GSConsole instance. This is no longer the case, + * and @user_data is ignored. **/ void ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress, gpointer user_data) { - GSConsole *console = user_data; GString *buf; g_autofree char *status = NULL; guint outstanding_fetches; @@ -4135,7 +4139,8 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress guint fetched_delta_parts; guint total_delta_parts; - if (!console) + /* Historical note; we used to treat this as a GSConsole instance */ + if (user_data == NULL) return; buf = g_string_new (""); @@ -4202,7 +4207,7 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress g_string_append_printf (buf, "Scanning metadata: %u", n_scanned_metadata); } - gs_console_begin_status_line (console, buf->str, NULL, NULL); + glnx_console_text (buf->str); g_string_free (buf, TRUE); } diff --git a/src/ostree/ot-admin-builtin-switch.c b/src/ostree/ot-admin-builtin-switch.c index 0e0101a9..b313e830 100644 --- a/src/ostree/ot-admin-builtin-switch.c +++ b/src/ostree/ot-admin-builtin-switch.c @@ -61,8 +61,6 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro glnx_unref_object OstreeSysrootUpgrader *upgrader = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; gboolean changed; - GSConsole *console = NULL; - gboolean in_status_line = FALSE; GKeyFile *old_origin; GKeyFile *new_origin = NULL; @@ -125,28 +123,24 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro if (!ostree_sysroot_upgrader_set_origin (upgrader, new_origin, cancellable, error)) goto out; - console = gs_console_get (); - if (console) - { - gs_console_begin_status_line (console, "", NULL, NULL); - in_status_line = TRUE; - progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, console); - } + { g_auto(GLnxConsoleRef) console = { 0, }; + glnx_console_lock (&console); - /* Always allow older...there's not going to be a chronological - * relationship necessarily. - */ - if (!ostree_sysroot_upgrader_pull (upgrader, 0, - OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER, - progress, &changed, - cancellable, error)) - goto out; + if (console.is_tty) + progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, &console); - if (in_status_line) - { - gs_console_end_status_line (console, NULL, NULL); - in_status_line = FALSE; - } + /* Always allow older...there's not going to be a chronological + * relationship necessarily. + */ + if (!ostree_sysroot_upgrader_pull (upgrader, 0, + OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER, + progress, &changed, + cancellable, error)) + goto out; + + if (progress) + ostree_async_progress_finish (progress); + } if (!ostree_sysroot_upgrader_deploy (upgrader, cancellable, error)) goto out; @@ -171,8 +165,6 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro ret = TRUE; out: - if (in_status_line) - gs_console_end_status_line (console, NULL, NULL); if (new_origin) g_key_file_unref (new_origin); if (context) diff --git a/src/ostree/ot-admin-builtin-upgrade.c b/src/ostree/ot-admin-builtin-upgrade.c index b3b531c0..81f9bb6f 100644 --- a/src/ostree/ot-admin-builtin-upgrade.c +++ b/src/ostree/ot-admin-builtin-upgrade.c @@ -55,8 +55,6 @@ ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GErr g_autoptr(GFile) deployment_path = NULL; g_autoptr(GFile) deployment_origin_path = NULL; g_autoptr(GKeyFile) origin = NULL; - GSConsole *console = NULL; - gboolean in_status_line = FALSE; glnx_unref_object OstreeAsyncProgress *progress = NULL; gboolean changed; OstreeSysrootUpgraderPullFlags upgraderpullflags = 0; @@ -108,27 +106,23 @@ ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GErr } } - console = gs_console_get (); - if (console) - { - gs_console_begin_status_line (console, "", NULL, NULL); - in_status_line = TRUE; - progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, console); - } + { g_auto(GLnxConsoleRef) console = { 0, }; + glnx_console_lock (&console); - if (opt_allow_downgrade) - upgraderpullflags |= OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER; + if (console.is_tty) + progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, &console); - if (!ostree_sysroot_upgrader_pull (upgrader, 0, upgraderpullflags, - progress, &changed, - cancellable, error)) - goto out; + if (opt_allow_downgrade) + upgraderpullflags |= OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER; + + if (!ostree_sysroot_upgrader_pull (upgrader, 0, upgraderpullflags, + progress, &changed, + cancellable, error)) + goto out; - if (in_status_line) - { - gs_console_end_status_line (console, NULL, NULL); - in_status_line = FALSE; - } + if (progress) + ostree_async_progress_finish (progress); + } if (!changed) { @@ -148,8 +142,6 @@ ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (in_status_line) - gs_console_end_status_line (console, NULL, NULL); if (context) g_option_context_free (context); return ret; diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c index 36057ec6..7fb6f03a 100644 --- a/src/ostree/ot-builtin-pull-local.c +++ b/src/ostree/ot-builtin-pull-local.c @@ -55,7 +55,6 @@ ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GEr glnx_unref_object OstreeRepo *repo = NULL; int i; const char *src_repo_arg; - GSConsole *console = NULL; g_autofree char *src_repo_uri = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; g_autoptr(GPtrArray) refs_to_fetch = NULL; @@ -132,14 +131,11 @@ ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GEr g_ptr_array_add (refs_to_fetch, NULL); } - console = gs_console_get (); - if (console) - { - gs_console_begin_status_line (console, "", NULL, NULL); - progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, console); - } - { GVariantBuilder builder; + g_auto(GLnxConsoleRef) console = { 0, }; + + glnx_console_lock (&console); + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_add (&builder, "{s@v}", "flags", @@ -158,17 +154,21 @@ ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GEr g_variant_builder_add (&builder, "{s@v}", "depth", g_variant_new_variant (g_variant_new_int32 (opt_depth))); + if (console.is_tty) + progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, &console); + if (!ostree_repo_pull_with_options (repo, src_repo_uri, g_variant_builder_end (&builder), progress, cancellable, error)) goto out; + + if (progress) + ostree_async_progress_finish (progress); } ret = TRUE; out: - if (progress) - ostree_async_progress_finish (progress); if (context) g_option_context_free (context); if (repo) diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index f69d276c..99b25937 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -58,16 +58,17 @@ static void gpg_verify_result_cb (OstreeRepo *repo, const char *checksum, OstreeGpgVerifyResult *result, - GSConsole *console) + GLnxConsoleRef *console) { - /* Temporarily place the GSConsole stream (which is just stdout) - * back in normal mode before printing GPG verification results. */ - gs_console_end_status_line (console, NULL, NULL); + /* Temporarily place the tty back in normal mode before printing GPG + * verification results. + */ + glnx_console_unlock (console); g_print ("\n"); ostree_print_gpg_verify_result (result); - gs_console_begin_status_line (console, "", NULL, NULL); + glnx_console_lock (console); } static gboolean printed_console_progress; @@ -111,7 +112,6 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** gboolean ret = FALSE; g_autofree char *remote = NULL; OstreeRepoPullFlags pullflags = 0; - GSConsole *console = NULL; g_autoptr(GPtrArray) refs_to_fetch = NULL; g_autoptr(GPtrArray) override_commit_ids = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; @@ -206,30 +206,13 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** g_ptr_array_add (refs_to_fetch, NULL); } - if (!opt_dry_run) - { - console = gs_console_get (); - if (console) - { - gs_console_begin_status_line (console, "", NULL, NULL); - progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, console); - signal_handler_id = g_signal_connect (repo, "gpg-verify-result", - G_CALLBACK (gpg_verify_result_cb), - console); - } - } - else - { - progress = ostree_async_progress_new_and_connect (dry_run_console_progress_changed, console); - signal_handler_id = g_signal_connect (repo, "gpg-verify-result", - G_CALLBACK (gpg_verify_result_cb), - console); - } - { GVariantBuilder builder; + g_auto(GLnxConsoleRef) console = { 0, }; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); + glnx_console_lock (&console); + if (opt_url) g_variant_builder_add (&builder, "{s@v}", "override-url", g_variant_new_variant (g_variant_new_string (opt_url))); @@ -257,25 +240,38 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** g_variant_builder_add (&builder, "{s@v}", "override-commit-ids", g_variant_new_variant (g_variant_new_strv ((const char*const*)override_commit_ids->pdata, override_commit_ids->len))); + if (!opt_dry_run) + { + if (console.is_tty) + progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, &console); + } + else + { + progress = ostree_async_progress_new_and_connect (dry_run_console_progress_changed, NULL); + } + + if (console.is_tty) + { + signal_handler_id = g_signal_connect (repo, "gpg-verify-result", + G_CALLBACK (gpg_verify_result_cb), + &console); + } + if (!ostree_repo_pull_with_options (repo, remote, g_variant_builder_end (&builder), progress, cancellable, error)) goto out; + + if (progress) + ostree_async_progress_finish (progress); + + if (opt_dry_run) + g_assert (printed_console_progress); } - if (progress) - ostree_async_progress_finish (progress); - - if (opt_dry_run) - g_assert (printed_console_progress); - ret = TRUE; out: if (signal_handler_id > 0) g_signal_handler_disconnect (repo, signal_handler_id); - - if (console) - gs_console_end_status_line (console, NULL, NULL); - if (context) g_option_context_free (context); return ret;