core: Accumulate all missing pkgs into a single error

Accumulate the list of requested packages that were not found and error
out with the full list rather than failing early. This fixes a small UX
papercut in certain situations.

Closes: #1540

Closes: #1541
Approved by: cgwalters
This commit is contained in:
Jonathan Lebon 2018-09-08 17:19:15 -04:00 committed by Atomic Bot
parent f6c0788b4e
commit 651a9e01f4
2 changed files with 36 additions and 9 deletions

View File

@ -1397,13 +1397,12 @@ sort_packages (RpmOstreeContext *self,
/* Set @error with a string containing an error relating to @pkgs */
static gboolean
throw_package_list (GError **error, const char *suffix, GPtrArray *pkgs)
throw_package_list (GError **error, const char *prefix, GPtrArray *pkgs)
{
if (!error)
return FALSE; /* Note early simultaneously happy and sad return */
g_autoptr(GString) msg = g_string_new ("The following base packages ");
g_string_append (msg, suffix);
g_autoptr(GString) msg = g_string_new (prefix);
g_string_append (msg, ": ");
gboolean first = TRUE;
@ -1480,7 +1479,7 @@ check_goal_solution (RpmOstreeContext *self,
}
if (forbidden->len > 0)
return throw_package_list (error, "would be removed", forbidden);
return throw_package_list (error, "Base packages would be removed", forbidden);
}
/* check that all the pkgs we expect to remove are marked for removal */
@ -1494,7 +1493,7 @@ check_goal_solution (RpmOstreeContext *self,
}
if (forbidden->len > 0)
return throw_package_list (error, "are not marked to be removed", forbidden);
return throw_package_list (error, "Base packages not marked to be removed", forbidden);
}
/* REINSTALLs should never happen since it doesn't make sense in the rpm-ostree flow, and
@ -1572,7 +1571,7 @@ check_goal_solution (RpmOstreeContext *self,
}
if (forbidden->len > 0)
return throw_package_list (error, "are not marked to be installed", forbidden);
return throw_package_list (error, "Base packages not marked to be installed", forbidden);
}
return TRUE;
@ -1828,14 +1827,31 @@ rpmostree_context_prepare (RpmOstreeContext *self,
}
/* Loop over each named package, and tell libdnf to add it to the goal */
g_autoptr(GPtrArray) missing_pkgs = NULL;
for (char **it = pkgnames; it && *it; it++)
{
const char *pkgname = *it;
g_autoptr(GError) local_error = NULL;
g_assert (!self->rojig_pure);
if (!dnf_context_install (dnfctx, pkgname, error))
return FALSE;
if (!dnf_context_install (dnfctx, pkgname, &local_error))
{
/* Only keep going if it's ENOENT, so we coalesce into one msg at the end */
if (!g_error_matches (local_error, DNF_ERROR, DNF_ERROR_PACKAGE_NOT_FOUND))
{
g_propagate_error (error, g_steal_pointer (&local_error));
return FALSE;
}
/* lazy init since it's unlikely in the common case (e.g. upgrades) */
if (!missing_pkgs)
missing_pkgs = g_ptr_array_new ();
g_ptr_array_add (missing_pkgs, (gpointer)pkgname);
}
}
if (missing_pkgs && missing_pkgs->len > 0)
return throw_package_list (error, "Packages not found", missing_pkgs);
if (self->rojig_spec)
{
if (!setup_rojig_state (self, error))

View File

@ -59,6 +59,17 @@ assert_file_has_content err.txt "See https://github.com/projectatomic/rpm-ostree
echo "ok failed to install in /opt and /usr/local"
# Check that trying to install multiple nonexistent pkgs at once provides an
# error including all of them at once
fakes="foobar barbaz bazboo"
if vm_rpmostree install $fakes &> err.txt; then
assert_not_reached "successfully layered non-existent pkgs $fakes?"
fi
assert_file_has_content_literal err.txt "Packages not found:"
# ordering can be different, so check one at a time
for pkg in $fakes; do assert_file_has_content_literal err.txt $pkg; done
echo "ok one error for multiple missing pkgs"
# Explicit epoch of 0 as it's a corner case:
# https://github.com/projectatomic/rpm-ostree/issues/349
vm_build_rpm foo epoch 0
@ -163,7 +174,7 @@ vm_build_rpm bar conflicts foo
if vm_rpmostree install bar &> err.txt; then
assert_not_reached "successfully layered conflicting pkg bar?"
fi
assert_file_has_content err.txt "The following base packages would be removed"
assert_file_has_content err.txt "Base packages would be removed"
assert_file_has_content err.txt "foo-1.0-1.x86_64"
vm_rpmostree cleanup -p
vm_cmd ostree reset vmcheck $(vm_cmd ostree rev-parse "vmcheck^")