Skip to content

Commit

Permalink
rebuild: Fix logic for container-only handling
Browse files Browse the repository at this point in the history
Right now `ex rebuild` only works in an (ostree) container and
trying to run it on an ostree host system, despite all the checking
in that function just segfaults.

And the reason here is very illuminating: We were passing `NULL`
for the out parameter for the DBus client.  Let's use that to
instead be the signal that a command is current container *only*.

But, this is really a perfect example of where Rust would have
entirely prevented this problem.
  • Loading branch information
cgwalters committed Oct 3, 2022
1 parent 9496885 commit ff6a16c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 28 deletions.
2 changes: 2 additions & 0 deletions src/app/libmain.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,8 @@ rpmostree_option_context_parse (GOptionContext *context, const GOptionEntry *mai
bool container_capable = (flags & RPM_OSTREE_BUILTIN_FLAG_CONTAINER_CAPABLE) > 0;
if (use_daemon && !(is_ostree_container && container_capable))
{
if (out_sysroot_proxy == NULL)
return glnx_throw (error, "This command can only run in an OSTree container.");
/* More gracefully handle the case where
* no --sysroot option was specified and we're not booted via ostree
* https://github.com/projectatomic/rpm-ostree/issues/1537
Expand Down
36 changes: 8 additions & 28 deletions src/app/rpmostree-builtin-rebuild.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -41,39 +41,19 @@ rpmostree_ex_builtin_rebuild (int argc, char **argv, RpmOstreeCommandInvocation
NULL, NULL, error))
return FALSE;

bool in_container = false;
if (rpmostreecxx::running_in_container ())
{
CXX_TRY_VAR (is_ostree_container, rpmostreecxx::is_ostree_container (), error);
if (!is_ostree_container)
return glnx_throw (error, "This command can only run in an OSTree container.");
in_container = true;
}

auto basearch = rpmostreecxx::get_rpm_basearch ();
CXX_TRY_VAR (treefile, rpmostreecxx::treefile_new_client_from_etc (basearch), error);

/* This is the big switch: we support running this command in two modes:
* "client containers", where the effect takes place in the active rootfs, and
* possibly eventually "client host systems", where the effect takes place in
* a new deployment. */
if (in_container)
{
if (!rpmostree_container_rebuild (*treefile, cancellable, error))
return FALSE;
/* Right now we only support running this in a container */
if (!rpmostree_container_rebuild (*treefile, cancellable, error))
return FALSE;

/* In the container flow, we effectively "consume" the treefiles after
* modifying the rootfs. */
CXX_TRY_VAR (n, rpmostreecxx::treefile_delete_client_etc (), error);
if (n == 0)
{
g_print ("No changes to apply.\n");
}
}
else
/* In the container flow, we effectively "consume" the treefiles after
* modifying the rootfs. */
CXX_TRY_VAR (n, rpmostreecxx::treefile_delete_client_etc (), error);
if (n == 0)
{
return glnx_throw (error, "This command is not yet supported on host systems. "
"See https://github.com/coreos/rpm-ostree/issues/2326.");
g_print ("No changes to apply.\n");
}

return TRUE;
Expand Down
5 changes: 5 additions & 0 deletions tests/kolainst/nondestructive/misc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ fi
assert_file_has_content_literal err.txt 'error: Argument is invalid UTF-8'
echo "ok error on non UTF-8"

if rpm-ostree ex rebuild 2>err.txt; then
fatal "ex rebuild on host"
fi
assert_file_has_content_literal err.txt 'error: This command can only run in an OSTree container'

rpm-ostree status --jsonpath '$.deployments[0].booted' > jsonpath.txt
assert_file_has_content_literal jsonpath.txt 'true'
echo "ok jsonpath"
Expand Down

0 comments on commit ff6a16c

Please sign in to comment.