comps: Make the install_or_skip() method not catch CompsError anymore

According to its docstring, the original intention of the method was to
not fail on installing an already installed group/environment.

However, the CompsError is no longer thrown when attempting to install
an already installed group or environment. It was changed to logging a
warning directly in 5210b9d and then the check was removed completely
in 217ca0f.

For the other case for which an instance of CompsError can be thrown
from the install_group() and install_environment() methods, which is
when a group or environment is not found, we certainly want to throw an
error (see the linked bugs), therefore there's no reason to catch the
exception anymore.

The install_or_skip() method is preserved as part of the API so as not
to break compatibility any more than necessary.

msg: API: Raise CompsError when group/env not found in install_group and install_environment
type: bugfix
resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1947958
related: https://bugzilla.redhat.com/show_bug.cgi?id=1943206

dnf/base.py

@@ -1687,9 +1687,7 @@ def environment_install(self, env_id, types, exclude=None, strict=True, exclude_
  if not isinstance(types, int):
  types = libdnf.transaction.listToCompsPackageType(types)
 
- trans = dnf.comps.install_or_skip(solver._environment_install,
- env_id, types, exclude or set(),
- strict, exclude_groups)
+ trans = solver._environment_install(env_id, types, exclude or set(), strict, exclude_groups)
  if not trans:
  return 0
  return self._add_comps_trans(trans)
@@ -1732,9 +1730,7 @@ def _pattern_to_pkgname(pattern):
  if not isinstance(pkg_types, int):
  pkg_types = libdnf.transaction.listToCompsPackageType(pkg_types)
 
- trans = dnf.comps.install_or_skip(solver._group_install,
- grp_id, pkg_types, exclude_pkgnames,
- strict)
+ trans = solver._group_install(grp_id, pkg_types, exclude_pkgnames, strict)
  if not trans:
  return 0
  if strict:

dnf/cli/commands/group.py

@@ -244,9 +244,9 @@ def _mark_install(self, patterns):
  types = tuple(self.base.conf.group_package_types)
  pkg_types = libdnf.transaction.listToCompsPackageType(types)
  for env_id in res.environments:
- dnf.comps.install_or_skip(solver._environment_install, env_id, pkg_types)
+ solver._environment_install(env_id, pkg_types)
  for group_id in res.groups:
- dnf.comps.install_or_skip(solver._group_install, group_id, pkg_types)
+ solver._group_install(group_id, pkg_types)
 
  def _mark_remove(self, patterns):
  q = CompsQuery(self.base.comps, self.base.history,

dnf/comps.py

@@ -93,15 +93,15 @@ def _fn_display_order(group):
 
 def install_or_skip(install_fnc, grp_or_env_id, types, exclude=None,
  strict=True, exclude_groups=None):
- """Either mark in persistor as installed given `grp_or_env` (group
-   or environment) or skip it (if it's already installed).
-  `install_fnc` has to be Solver._group_install
-  or Solver._environment_install.
-  """
- try:
-  return install_fnc(grp_or_env_id, types, exclude, strict, exclude_groups)
- except dnf.comps.CompsError as e:
-  logger.warning("%s, %s", ucd(e)[:-1], _("skipping."))
+ """
+ Installs a group or an environment identified by grp_or_env_id.
+ This method is preserved for API compatibility. It used to catch an
+ exception thrown when a gorup or env was already installed, which is no
+ longer thrown.
+ `install_fnc` has to be Solver._group_install or
+ Solver._environment_install.
+ """
+ return install_fnc(grp_or_env_id, types, exclude, strict, exclude_groups)
 
 
 class _Langs(object):
@@ -592,7 +592,7 @@ def _removable_grp(self, group_id):
  assert dnf.util.is_string_type(group_id)
  return self.history.env.is_removable_group(group_id)
 
- def _environment_install(self, env_id, pkg_types, exclude, strict=True, exclude_groups=None):
+ def _environment_install(self, env_id, pkg_types, exclude=None, strict=True, exclude_groups=None):
  assert dnf.util.is_string_type(env_id)
  comps_env = self.comps._environment_by_id(env_id)
  if not comps_env:

doc/api_base.rst

@@ -179,7 +179,7 @@
 
  .. method:: group_install(group_id, pkg_types, exclude=None, strict=True)
 
- Mark group with corresponding `group_id` installed and mark the packages in the group for installation. Return the number of packages that the operation has marked for installation. `pkg_types` is a sequence of strings determining the kinds of packages to be installed, where the respective groups can be selected by including ``"mandatory"``, ``"default"`` or ``"optional"`` in it. If `exclude` is given, it has to be an iterable of package name glob patterns: :meth:`.group_install` will then not mark the respective packages for installation whenever possible. Parameter `strict` is a boolean indicating whether group packages that exist but are non-installable due to e.g. dependency issues should be skipped (False) or cause transaction to fail to resolve (True).
+ Mark group with corresponding `group_id` installed and mark the packages in the group for installation. Return the number of packages that the operation has marked for installation. `pkg_types` is a sequence of strings determining the kinds of packages to be installed, where the respective groups can be selected by including ``"mandatory"``, ``"default"`` or ``"optional"`` in it. If `exclude` is given, it has to be an iterable of package name glob patterns: :meth:`.group_install` will then not mark the respective packages for installation whenever possible. Parameter `strict` is a boolean indicating whether group packages that exist but are non-installable due to e.g. dependency issues should be skipped (False) or cause transaction to fail to resolve (True). Raises :exc:`dnf.exceptions.CompsError` in case the group doesn't exist.
 
  .. method:: group_remove(group_id)
 
@@ -191,7 +191,7 @@
 
  .. method:: environment_install(env_id, types, exclude=None, strict=True, exclude_groups=None)
 
- Similar to :meth:`.group_install` but operates on environmental groups. `exclude_groups` is an iterable of group IDs that will not be marked as installed.
+ Similar to :meth:`.group_install` but operates on environmental groups. `exclude_groups` is an iterable of group IDs that will not be marked as installed. Raises :exc:`dnf.exceptions.CompsError` in case the group doesn't exist.
 
  .. method:: environment_remove(env_id)