Skip to content

Commit

Permalink
daemon: Tweak default flag handling logic
Browse files Browse the repository at this point in the history
When hoisting deploy flags from the option dict, we want the default
values to take effect *only if* the option wasn't specified in the dict.
Instead, because we initialized the return value with the default flags,
the option set couldn't override a flag which was part of the defaults
(IOW, a flag that's on by default couldn't be turned off through the
options dict).

Came upon this issue by trying to use the older `Deploy()` and
`Rebase()` D-Bus APIs which pass default values like `ALLOW_DOWNGRADE`,
but the option dict specifies `allow-downgrade=false`. (We could say,
"you should use the newer `UpdateDeployment()` API", but the cat is
mostly out of the bag at this point on options that get shoved into the
dict and take effect on both APIs).
  • Loading branch information
jlebon authored and cgwalters committed Aug 21, 2019
1 parent 313b3e6 commit c279f92
Showing 1 changed file with 25 additions and 10 deletions.
35 changes: 25 additions & 10 deletions src/daemon/rpmostreed-transaction-types.c
Original file line number Diff line number Diff line change
Expand Up @@ -1480,19 +1480,34 @@ vardict_lookup_strv_canonical (GVariantDict *dict,
return v;
}

static inline RpmOstreeTransactionDeployFlags
set_deploy_flag (RpmOstreeTransactionDeployFlags flags,
RpmOstreeTransactionDeployFlags flag,
gboolean val)
{
/* first, make sure it's cleared */
flags &= ~flag;
if (val)
flags |= flag;
return flags;
}

/* @defaults contains some default flags. They only take effect if the vardict option they
* correspond to wasn't specified. */
static RpmOstreeTransactionDeployFlags
deploy_flags_from_options (GVariantDict *dict,
RpmOstreeTransactionDeployFlags defaults)
{
RpmOstreeTransactionDeployFlags ret = defaults;
if (vardict_lookup_bool (dict, "allow-downgrade", FALSE))
ret |= RPMOSTREE_TRANSACTION_DEPLOY_FLAG_ALLOW_DOWNGRADE;
if (vardict_lookup_bool (dict, "no-pull-base", FALSE))
ret |= RPMOSTREE_TRANSACTION_DEPLOY_FLAG_NO_PULL_BASE;
if (vardict_lookup_bool (dict, "dry-run", FALSE))
ret |= RPMOSTREE_TRANSACTION_DEPLOY_FLAG_DRY_RUN;
if (vardict_lookup_bool (dict, "download-only", FALSE))
ret |= RPMOSTREE_TRANSACTION_DEPLOY_FLAG_DOWNLOAD_ONLY;
gboolean val;
if (g_variant_dict_lookup (dict, "allow-downgrade", "b", &val))
ret = set_deploy_flag (ret, RPMOSTREE_TRANSACTION_DEPLOY_FLAG_ALLOW_DOWNGRADE, val);
if (g_variant_dict_lookup (dict, "no-pull-base", "b", &val))
ret = set_deploy_flag (ret, RPMOSTREE_TRANSACTION_DEPLOY_FLAG_NO_PULL_BASE, val);
if (g_variant_dict_lookup (dict, "dry-run", "b", &val))
ret = set_deploy_flag (ret, RPMOSTREE_TRANSACTION_DEPLOY_FLAG_DRY_RUN, val);
if (g_variant_dict_lookup (dict, "download-only", "b", &val))
ret = set_deploy_flag (ret, RPMOSTREE_TRANSACTION_DEPLOY_FLAG_DOWNLOAD_ONLY, val);
return ret;
}

Expand Down Expand Up @@ -1521,7 +1536,7 @@ RpmostreedTransaction *
rpmostreed_transaction_new_deploy (GDBusMethodInvocation *invocation,
OstreeSysroot *sysroot,
const char *osname,
RpmOstreeTransactionDeployFlags flags,
RpmOstreeTransactionDeployFlags default_flags,
GVariant *options,
RpmOstreeUpdateDeploymentModifiers *modifiers,
GUnixFDList *fd_list,
Expand Down Expand Up @@ -1558,7 +1573,7 @@ rpmostreed_transaction_new_deploy (GDBusMethodInvocation *invocation,
self->options = g_variant_dict_ref (options_dict);
self->modifiers = g_variant_dict_new (modifiers);

self->flags = deploy_flags_from_options (self->options, flags);
self->flags = deploy_flags_from_options (self->options, default_flags);

const char *refspec = vardict_lookup_ptr (self->modifiers, "set-refspec", "&s");
/* Canonicalize here; the later code actually ends up peeling it
Expand Down

0 comments on commit c279f92

Please sign in to comment.