Improve progress output

This rolls up several libglnx changes: https://github.com/GNOME/libglnx/pull/101

Now of course things are trickier here because we have an internal
abstraction over directly emitting to a console versus sending the
result over DBus.  Further complicating things is that some things
call into libdnf and thus *require* use of `DnfState` which does
not give us the "n items" information, versus other parts which
we implement and can do what we want.

Even *further* complicating things is that we have to take care around non-CLI
callers like Cockpit; so I didn't try to pass the "n items" over DBus, rather
just reimplemented the "insert into text" that libglnx is doing.

Anyways overall this looks better IMO for all cases.

Update submodule: libglnx

Closes: #1143
Approved by: jlebon
This commit is contained in:
Colin Walters 2017-12-12 15:27:46 -05:00 committed by Atomic Bot
parent 1793480155
commit dac5ccc76e
6 changed files with 72 additions and 58 deletions

@ -1 +1 @@
Subproject commit e627524af9ae499c1edd04795442f8bdb05b5223
Subproject commit 96b1fd9578b3d6ff2d8e0707068f5ef450eba98c

View File

@ -130,12 +130,25 @@ sysroot_output_cb (RpmOstreeOutputType type, void *data, void *opaque)
rpmostree_transaction_emit_task_end (RPMOSTREE_TRANSACTION (transaction),
((RpmOstreeOutputTaskEnd*)data)->text);
break;
case RPMOSTREE_OUTPUT_PERCENT_PROGRESS:
case RPMOSTREE_OUTPUT_PROGRESS_PERCENT:
rpmostree_transaction_emit_percent_progress (RPMOSTREE_TRANSACTION (transaction),
((RpmOstreeOutputPercentProgress*)data)->text,
((RpmOstreeOutputPercentProgress*)data)->percentage);
((RpmOstreeOutputProgressPercent*)data)->text,
((RpmOstreeOutputProgressPercent*)data)->percentage);
break;
case RPMOSTREE_OUTPUT_PERCENT_PROGRESS_END:
case RPMOSTREE_OUTPUT_PROGRESS_N_ITEMS:
{
RpmOstreeOutputProgressNItems *nitems = data;
/* We still emit PercentProgress for compatibility with older clients as
* well as Cockpit. It's not worth trying to deal with version skew just
* for this yet.
*/
int percentage = (nitems->current == nitems->total) ? 100 :
(((double)(nitems->current)) / (nitems->total) * 100);
g_autofree char *newtext = g_strdup_printf ("%s (%u/%u)", nitems->text, nitems->current, nitems->total);
rpmostree_transaction_emit_percent_progress (RPMOSTREE_TRANSACTION (transaction), newtext, percentage);
}
break;
case RPMOSTREE_OUTPUT_PROGRESS_END:
rpmostree_transaction_emit_progress_end (RPMOSTREE_TRANSACTION (transaction));
break;
}

View File

@ -279,7 +279,6 @@ struct _RpmOstreeContext {
gboolean async_running;
GCancellable *async_cancellable;
GError *async_error;
DnfState *async_dnfstate;
GPtrArray *pkgs_to_download;
GPtrArray *pkgs_to_import;
guint n_async_pkgs_imported;
@ -748,7 +747,7 @@ on_hifstate_percentage_changed (DnfState *hifstate,
gpointer user_data)
{
const char *text = user_data;
rpmostree_output_percent_progress (text, percentage);
rpmostree_output_progress_percent (text, percentage);
}
static gboolean
@ -1074,7 +1073,7 @@ rpmostree_context_download_metadata (RpmOstreeContext *self,
did_update = TRUE;
g_signal_handler_disconnect (hifstate, progress_sigid);
rpmostree_output_percent_progress_end ();
rpmostree_output_progress_end ();
}
guint64 ts = dnf_repo_get_timestamp_generated (repo);
@ -1106,7 +1105,7 @@ rpmostree_context_download_metadata (RpmOstreeContext *self,
if (!dnf_context_setup_sack (self->dnfctx, hifstate, error))
return FALSE;
g_signal_handler_disconnect (hifstate, progress_sigid);
rpmostree_output_percent_progress_end ();
rpmostree_output_progress_end ();
}
/* A lot of code to simply log a message to the systemd journal with the state
@ -2157,14 +2156,13 @@ rpmostree_context_download (RpmOstreeContext *self,
return FALSE;
g_signal_handler_disconnect (hifstate, progress_sigid);
rpmostree_output_percent_progress_end ();
rpmostree_output_progress_end ();
}
}
return TRUE;
}
/* Returns: (transfer none): The jigdo package */
DnfPackage *
rpmostree_context_get_jigdo_pkg (RpmOstreeContext *self)
@ -2181,12 +2179,6 @@ rpmostree_context_get_jigdo_checksum (RpmOstreeContext *self)
return self->jigdo_checksum;
}
static inline void
dnf_state_assert_done (DnfState *dnfstate)
{
g_assert (dnf_state_done (dnfstate, NULL));
}
static void
on_async_import_done (GObject *obj,
GAsyncResult *res,
@ -2206,7 +2198,8 @@ on_async_import_done (GObject *obj,
g_assert_cmpint (self->n_async_pkgs_imported, <, self->pkgs_to_import->len);
self->n_async_pkgs_imported++;
dnf_state_assert_done (self->async_dnfstate);
rpmostree_output_progress_n_items ("Importing", self->n_async_pkgs_imported,
self->pkgs_to_import->len);
if (self->n_async_pkgs_imported == self->pkgs_to_import->len)
self->async_running = FALSE;
}
@ -2236,13 +2229,6 @@ rpmostree_context_import_jigdo (RpmOstreeContext *self,
return FALSE;
{
glnx_unref_object DnfState *hifstate = dnf_state_new ();
dnf_state_set_number_steps (hifstate, self->pkgs_to_import->len);
guint progress_sigid = g_signal_connect (hifstate, "percentage-changed",
G_CALLBACK (on_hifstate_percentage_changed),
"Importing:");
self->async_dnfstate = hifstate;
self->async_running = TRUE;
self->async_cancellable = cancellable;
@ -2306,8 +2292,7 @@ rpmostree_context_import_jigdo (RpmOstreeContext *self,
return FALSE;
}
g_signal_handler_disconnect (hifstate, progress_sigid);
rpmostree_output_percent_progress_end ();
rpmostree_output_progress_end ();
}
if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
@ -2721,7 +2706,8 @@ on_async_relabel_done (GObject *obj,
data->n_changed_files += n_relabeled;
data->n_changed_pkgs++;
}
dnf_state_assert_done (self->async_dnfstate);
rpmostree_output_progress_n_items ("Relabeling", self->n_async_pkgs_relabeled,
self->pkgs_to_relabel->len);
if (self->n_async_pkgs_relabeled == self->pkgs_to_relabel->len)
self->async_running = FALSE;
}
@ -2744,14 +2730,6 @@ rpmostree_context_relabel (RpmOstreeContext *self,
g_return_val_if_fail (ostreerepo != NULL, FALSE);
glnx_unref_object DnfState *hifstate = dnf_state_new ();
g_autofree char *prefix = g_strdup_printf ("Relabeling %d package%s:", n, _NS(n));
dnf_state_set_number_steps (hifstate, self->pkgs_to_relabel->len);
guint progress_sigid = g_signal_connect (hifstate, "percentage-changed",
G_CALLBACK (on_hifstate_percentage_changed),
prefix);
/* Prep a txn and tmpdir for all of the relabels */
g_auto(RpmOstreeRepoAutoTransaction) txn = { 0, };
if (!rpmostree_repo_auto_transaction_start (&txn, ostreerepo, FALSE, cancellable, error))
@ -2762,7 +2740,6 @@ rpmostree_context_relabel (RpmOstreeContext *self,
&relabel_tmpdir, error))
return FALSE;
self->async_dnfstate = hifstate;
self->async_running = TRUE;
self->async_cancellable = cancellable;
@ -2785,15 +2762,13 @@ rpmostree_context_relabel (RpmOstreeContext *self,
g_propagate_error (error, g_steal_pointer (&self->async_error));
return FALSE;
}
self->async_dnfstate = NULL;
rpmostree_output_progress_end ();
/* Commit */
if (!ostree_repo_commit_transaction (ostreerepo, NULL, cancellable, error))
return FALSE;
g_signal_handler_disconnect (hifstate, progress_sigid);
rpmostree_output_percent_progress_end ();
sd_journal_send ("MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(RPMOSTREE_MESSAGE_SELINUX_RELABEL),
"MESSAGE=Relabeled %u/%u pkgs", data.n_changed_pkgs, n_to_relabel,
"RELABELED_PKGS=%u/%u", data.n_changed_pkgs, n_to_relabel,

View File

@ -47,14 +47,23 @@ rpmostree_output_default_handler (RpmOstreeOutputType type,
case RPMOSTREE_OUTPUT_TASK_END:
g_print ("%s\n", ((RpmOstreeOutputTaskEnd*)data)->text);
break;
case RPMOSTREE_OUTPUT_PERCENT_PROGRESS:
case RPMOSTREE_OUTPUT_PROGRESS_PERCENT:
if (!console.locked)
glnx_console_lock (&console);
glnx_console_progress_text_percent (
((RpmOstreeOutputPercentProgress*)data)->text,
((RpmOstreeOutputPercentProgress*)data)->percentage);
((RpmOstreeOutputProgressPercent*)data)->text,
((RpmOstreeOutputProgressPercent*)data)->percentage);
break;
case RPMOSTREE_OUTPUT_PERCENT_PROGRESS_END:
case RPMOSTREE_OUTPUT_PROGRESS_N_ITEMS:
{
RpmOstreeOutputProgressNItems *nitems = data;
if (!console.locked)
glnx_console_lock (&console);
glnx_console_progress_n_items (nitems->text, nitems->current, nitems->total);
}
break;
case RPMOSTREE_OUTPUT_PROGRESS_END:
if (console.locked)
glnx_console_unlock (&console);
break;
@ -104,14 +113,21 @@ rpmostree_output_task_end (const char *format, ...)
}
void
rpmostree_output_percent_progress (const char *text, int percentage)
rpmostree_output_progress_percent (const char *text, int percentage)
{
RpmOstreeOutputPercentProgress progress = { text, percentage };
active_cb (RPMOSTREE_OUTPUT_PERCENT_PROGRESS, &progress, active_cb_opaque);
RpmOstreeOutputProgressPercent progress = { text, percentage };
active_cb (RPMOSTREE_OUTPUT_PROGRESS_PERCENT, &progress, active_cb_opaque);
}
void
rpmostree_output_percent_progress_end (void)
rpmostree_output_progress_n_items (const char *text, guint current, guint total)
{
active_cb (RPMOSTREE_OUTPUT_PERCENT_PROGRESS_END, NULL, active_cb_opaque);
RpmOstreeOutputProgressNItems progress = { text, current, total };
active_cb (RPMOSTREE_OUTPUT_PROGRESS_N_ITEMS, &progress, active_cb_opaque);
}
void
rpmostree_output_progress_end (void)
{
active_cb (RPMOSTREE_OUTPUT_PROGRESS_END, NULL, active_cb_opaque);
}

View File

@ -22,8 +22,9 @@ typedef enum {
RPMOSTREE_OUTPUT_MESSAGE,
RPMOSTREE_OUTPUT_TASK_BEGIN,
RPMOSTREE_OUTPUT_TASK_END,
RPMOSTREE_OUTPUT_PERCENT_PROGRESS,
RPMOSTREE_OUTPUT_PERCENT_PROGRESS_END,
RPMOSTREE_OUTPUT_PROGRESS_N_ITEMS,
RPMOSTREE_OUTPUT_PROGRESS_PERCENT,
RPMOSTREE_OUTPUT_PROGRESS_END,
} RpmOstreeOutputType;
void
@ -52,10 +53,19 @@ rpmostree_output_task_end (const char *format, ...) G_GNUC_PRINTF (1,2);
typedef struct {
const char *text;
guint32 percentage;
} RpmOstreeOutputPercentProgress;
} RpmOstreeOutputProgressPercent;
typedef struct {
const char *text;
guint current;
guint total;
} RpmOstreeOutputProgressNItems;
void
rpmostree_output_percent_progress (const char *text, int percentage);
rpmostree_output_progress_n_items (const char *text, guint current, guint total);
void
rpmostree_output_percent_progress_end (void);
rpmostree_output_progress_percent (const char *text, int percentage);
void
rpmostree_output_progress_end (void);

View File

@ -135,12 +135,12 @@ echo "ok cleanup"
# install foo and make sure it was imported
vm_rpmostree install foo | tee output.txt
assert_file_has_content output.txt '^Importing:'
assert_file_has_content output.txt '^Importing (1/1)'
# upgrade with same foo in repos --> shouldn't re-import
vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck
vm_rpmostree upgrade | tee output.txt
assert_not_file_has_content output.txt '^Importing:'
assert_not_file_has_content output.txt '^Importing ('
# upgrade with different foo in repos --> should re-import
c1=$(sha256sum ${test_tmpdir}/yumrepo/packages/x86_64/foo-1.0-1.x86_64.rpm)
@ -150,7 +150,7 @@ if cmp -s c1 c2; then
assert_not_reached "RPM rebuild yielded same SHA256"
fi
vm_rpmostree upgrade | tee output.txt
assert_file_has_content output.txt '^Importing:'
assert_file_has_content output.txt '^Importing (1/1)'
echo "ok invalidate pkgcache from RPM chksum"
# make sure installing in /boot translates to /usr/lib/ostree-boot