app/pkg-builtins: Add --idempotent

Add a new `install/uninstall --idempotent` option to make it easier to
interact with the CLI through scripts. E.g. one doesn't have to check
first if a request has already been installed/uninstalled.

Closes: #1467

Closes: #1478
Approved by: cgwalters
This commit is contained in:
Jonathan Lebon 2018-07-30 15:10:50 -04:00 committed by Atomic Bot
parent a9e1de0276
commit d35fbb665e
6 changed files with 59 additions and 18 deletions

View File

@ -32,6 +32,7 @@
static char *opt_osname;
static gboolean opt_reboot;
static gboolean opt_dry_run;
static gboolean opt_idempotent;
static gchar **opt_install;
static gchar **opt_uninstall;
static gboolean opt_cache_only;
@ -44,6 +45,7 @@ static GOptionEntry option_entries[] = {
{ "reboot", 'r', 0, G_OPTION_ARG_NONE, &opt_reboot, "Initiate a reboot after operation is complete", NULL },
{ "dry-run", 'n', 0, G_OPTION_ARG_NONE, &opt_dry_run, "Exit after printing the transaction", NULL },
{ "allow-inactive", 0, 0, G_OPTION_ARG_NONE, &opt_allow_inactive, "Allow inactive package requests", NULL },
{ "idempotent", 0, 0, G_OPTION_ARG_NONE, &opt_idempotent, "Do nothing if package already (un)installed", NULL },
{ NULL }
};
@ -91,6 +93,7 @@ pkg_change (RpmOstreeCommandInvocation *invocation,
g_variant_dict_insert (&dict, "dry-run", "b", opt_dry_run);
g_variant_dict_insert (&dict, "allow-inactive", "b", opt_allow_inactive);
g_variant_dict_insert (&dict, "no-layering", "b", opt_uninstall_all);
g_variant_dict_insert (&dict, "idempotent-layering", "b", opt_idempotent);
g_autoptr(GVariant) options = g_variant_ref_sink (g_variant_dict_end (&dict));
gboolean met_local_pkg = FALSE;

View File

@ -329,6 +329,9 @@
"allow-inactive-requests" (type 'b')
When installing packages, allow package requests which would
not immediately be active.
"idempotent-layering" (type 'b')
Don't error out on requests in install-* or uninstall-*
modifiers that are already satisfied.
-->
<method name="UpdateDeployment">
<arg type="a{sv}" name="modifiers" direction="in"/>

View File

@ -787,6 +787,7 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
const gboolean no_layering = deploy_has_bool_option (self, "no-layering");
const gboolean no_initramfs = deploy_has_bool_option (self, "no-initramfs");
const gboolean cache_only = deploy_has_bool_option (self, "cache-only");
const gboolean idempotent_layering = deploy_has_bool_option (self, "idempotent-layering");
const gboolean download_only =
((self->flags & RPMOSTREE_TRANSACTION_DEPLOY_FLAG_DOWNLOAD_ONLY) > 0);
/* Mainly for the `install` and `override` commands */
@ -934,29 +935,27 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
changed = TRUE;
}
gboolean remove_changed = FALSE;
if (no_layering)
{
gboolean layering_changed = FALSE;
if (!rpmostree_origin_remove_all_packages (origin, &layering_changed, error))
if (!rpmostree_origin_remove_all_packages (origin, &remove_changed, error))
return FALSE;
changed = changed || layering_changed;
}
else if (self->uninstall_pkgs)
{
if (!rpmostree_origin_remove_packages (origin, self->uninstall_pkgs, error))
if (!rpmostree_origin_remove_packages (origin, self->uninstall_pkgs,
idempotent_layering, &remove_changed, error))
return FALSE;
/* in reality, there may not be any new layer required (if e.g. we're
* removing a duplicate provides), though the origin has changed so we
* need to create a new deployment -- see also
* https://github.com/projectatomic/rpm-ostree/issues/753 */
changed = TRUE;
g_string_append_printf (txn_title, "; uninstall: %u",
g_strv_length (self->uninstall_pkgs));
}
/* In reality, there may not be any new layer required even if `remove_changed` is TRUE
* (if e.g. we're removing a duplicate provides). But the origin has changed so we need to
* create a new deployment; see https://github.com/projectatomic/rpm-ostree/issues/753 */
changed = changed || remove_changed;
/* lazily loaded cache that's used in a few conditional blocks */
g_autoptr(RpmOstreeRefSack) base_rsack = NULL;
@ -998,12 +997,12 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
}
}
if (!rpmostree_origin_add_packages (origin, self->install_pkgs, FALSE, error))
gboolean add_changed = FALSE;
if (!rpmostree_origin_add_packages (origin, self->install_pkgs, FALSE,
idempotent_layering, &add_changed, error))
return FALSE;
/* here too -- we could optimize this under certain conditions
* (see related blurb in maybe_do_local_assembly()) */
changed = TRUE;
changed = changed || add_changed;
g_string_append_printf (txn_title, "; install: %u",
g_strv_length (self->install_pkgs));
@ -1021,10 +1020,13 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
g_string_append_printf (txn_title, "; localinstall: %u", pkgs->len);
g_ptr_array_add (pkgs, NULL);
if (!rpmostree_origin_add_packages (origin, (char**)pkgs->pdata, TRUE, error))
gboolean add_changed = FALSE;
if (!rpmostree_origin_add_packages (origin, (char**)pkgs->pdata, TRUE,
idempotent_layering, &add_changed, error))
return FALSE;
changed = TRUE;
changed = changed || add_changed;
}
}

View File

@ -652,6 +652,8 @@ gboolean
rpmostree_origin_add_packages (RpmOstreeOrigin *origin,
char **packages,
gboolean local,
gboolean allow_existing,
gboolean *out_changed,
GError **error)
{
gboolean changed = FALSE;
@ -685,6 +687,9 @@ rpmostree_origin_add_packages (RpmOstreeOrigin *origin,
if (requested || requested_local)
{
if (allow_existing)
continue;
if (requested)
return glnx_throw (error, "Package/capability '%s' is already requested", pkg);
else
@ -705,6 +710,7 @@ rpmostree_origin_add_packages (RpmOstreeOrigin *origin,
local ? origin->cached_local_packages
: origin->cached_packages, local);
*out_changed = changed;
return TRUE;
}
@ -730,6 +736,8 @@ build_name_to_nevra_map (GHashTable *nevras,
gboolean
rpmostree_origin_remove_packages (RpmOstreeOrigin *origin,
char **packages,
gboolean allow_noent,
gboolean *out_changed,
GError **error)
{
gboolean changed = FALSE;
@ -761,7 +769,7 @@ rpmostree_origin_remove_packages (RpmOstreeOrigin *origin,
g_hash_table_lookup (name_to_nevra, package)));
local_changed = TRUE;
}
else
else if (!allow_noent)
return glnx_throw (error, "Package/capability '%s' is not currently requested",
package);
}
@ -774,6 +782,7 @@ rpmostree_origin_remove_packages (RpmOstreeOrigin *origin,
update_keyfile_pkgs_from_cache (origin, "packages", "requested-local",
origin->cached_local_packages, TRUE);
*out_changed = changed || local_changed;
return TRUE;
}

View File

@ -152,11 +152,15 @@ gboolean
rpmostree_origin_add_packages (RpmOstreeOrigin *origin,
char **packages,
gboolean local,
gboolean allow_existing,
gboolean *out_changed,
GError **error);
gboolean
rpmostree_origin_remove_packages (RpmOstreeOrigin *origin,
char **packages,
gboolean allow_noent,
gboolean *out_changed,
GError **error);
gboolean
rpmostree_origin_remove_all_packages (RpmOstreeOrigin *origin,

View File

@ -90,8 +90,28 @@ vm_rpmostree db diff --format=diff \
assert_file_has_content_literal 'db-diff.txt' "+foo-1.0-1.x86_64"
echo "ok pkg-add foo"
# Test idempotent install
old_pending=$(vm_get_pending_csum)
if vm_rpmostree install foo-1.0 &> out.txt; then
assert_not_reached "installed foo twice?"
fi
assert_file_has_content_literal out.txt 'already requested'
vm_rpmostree install foo-1.0 --idempotent
assert_streq $old_pending $(vm_get_pending_csum)
echo "ok idempotent install"
vm_rpmostree uninstall foo-1.0
# Test idempotent uninstall
old_pending=$(vm_get_pending_csum)
if vm_rpmostree uninstall foo-1.0 &> out.txt; then
assert_not_reached "uninstalled foo twice?"
fi
assert_file_has_content_literal out.txt 'not currently requested'
vm_rpmostree uninstall foo-1.0 --idempotent
assert_streq $old_pending $(vm_get_pending_csum)
echo "ok idempotent uninstall"
# Test `rpm-ostree status --pending-exit-77`
rc=0
vm_rpmostree status --pending-exit-77 || rc=$?