rpmostreed-transaction-types: don't use base-db when idempotent

ci/test-container.sh

@@ -13,16 +13,25 @@ versionid=$(. /usr/lib/os-release && echo $VERSION_ID)
 # These hardcoded versions can be kept until Fedora GC's them
 ignition_url_suffix=2.17.0/4.fc40/x86_64/ignition-2.17.0-4.fc40."$(arch)".rpm
 case $versionid in
+  43)
+    # We use the same as f42 for 43 as they are compatible
+    koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2681489"
+    koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2805991"
+    kver=6.17.0
+    krev=0.rc3.31
+    ;;
   42)
-    # 2.21.0-1 (this koji url must be different than above version, and different from
-    # what's in the current image)
+    # This packages need to be different from what's in the current image.
+    # 2.21.0-1
     koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2681489"
     koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2685011"
     kver=6.14.0
     krev=63
     ;;
   41)
-    # 2.19.0-2 (this koji url must be different than above version)
+    # This koji url must be different than above version, and different from
+    # what's in the current image.
+    # 2.19.0-2
     koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2495227"
     koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2571615"
     kver=6.11.4

src/daemon/rpmostree-sysroot-upgrader.cxx

@@ -111,10 +111,11 @@ struct RpmOstreeSysrootUpgrader
 
   gboolean layering_initialized; /* Whether layering_type is known */
   RpmOstreeSysrootUpgraderLayeringType layering_type;
-  gboolean layering_changed; /* Whether changes to layering should result in a new commit */
-  gboolean pkgs_imported;    /* Whether pkgs to be layered have been downloaded & imported */
-  char *base_revision;       /* Non-layered replicated commit */
-  char *final_revision;      /* Computed by layering; if NULL, only using base_revision */
+  gboolean layering_changed;        /* Whether changes to layering should result in a new commit */
+  gboolean allow_empty_transaction; /* Allow empty DNF transaction for idempotent layering */
+  gboolean pkgs_imported;           /* Whether pkgs to be layered have been downloaded & imported */
+  char *base_revision;              /* Non-layered replicated commit */
+  char *final_revision;             /* Computed by layering; if NULL, only using base_revision */
 
   char **kargs_strv; /* Kernel argument list to be written into deployment  */
 };
@@ -401,6 +402,13 @@ rpmostree_sysroot_upgrader_get_sack (RpmOstreeSysrootUpgrader *self, GError **er
   return self->rpmmd_sack;
 }
 
+void
+rpmostree_sysroot_upgrader_set_allow_empty_transaction (RpmOstreeSysrootUpgrader *self,
+                                                        gboolean allow)
+{
+  self->allow_empty_transaction = allow;
+}
+
 /*
  * Like ostree_sysroot_upgrader_pull(), but also handles the `baserefspec` we
  * use when doing layered packages.
@@ -998,10 +1006,24 @@ prep_local_assembly (RpmOstreeSysrootUpgrader *self, GCancellable *cancellable,
       self->layering_changed = strcmp (previous_state_sha512, new_state_sha512) != 0;
     }
   else
-    /* Otherwise, we're transitioning from not-layered to layered, so it
-       definitely changed */
-    self->layering_changed = TRUE;
-
+    {
+      /* Otherwise, we're transitioning from not-layered to layered, so it
+         definitely changed */
+      self->layering_changed = TRUE;
+      /* Special case for containers: if the DNF transaction is empty (all packages already in
+       * base), don't consider this a change even if transitioning from unlayered to layered. This
+       * happens with idempotent layering when packages are already in the container image. */
+      if (self->layering_type == RPMOSTREE_SYSROOT_UPGRADER_LAYERING_RPMMD_REPOS)
+        {
+          auto refspec = rpmostree_origin_get_refspec (self->computed_origin);
+          if (refspec.kind == rpmostreecxx::RefspecType::Container
+              && rpmostree_dnf_context_has_empty_transaction (
+                  rpmostree_context_get_dnf (self->ctx)))
+            {
+              self->layering_changed = FALSE;
+            }
+        }
+    }
   return TRUE;
 }
 
@@ -1021,6 +1043,7 @@ perform_local_assembly (RpmOstreeSysrootUpgrader *self, GCancellable *cancellabl
 
   rpmostree_context_set_devino_cache (self->ctx, self->devino_cache);
   rpmostree_context_set_tmprootfs_dfd (self->ctx, self->tmprootfs_dfd);
+  rpmostree_context_set_allow_empty_transaction (self->ctx, self->allow_empty_transaction);
 
   if (self->layering_type == RPMOSTREE_SYSROOT_UPGRADER_LAYERING_RPMMD_REPOS)
     {

src/daemon/rpmostree-sysroot-upgrader.h

@@ -98,6 +98,9 @@ const char *rpmostree_sysroot_upgrader_get_base (RpmOstreeSysrootUpgrader *self)
 
 DnfSack *rpmostree_sysroot_upgrader_get_sack (RpmOstreeSysrootUpgrader *self, GError **error);
 
+void rpmostree_sysroot_upgrader_set_allow_empty_transaction (RpmOstreeSysrootUpgrader *self,
+                                                             gboolean allow);
+
 gboolean rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self,
                                                const char *dir_to_pull, OstreeRepoPullFlags flags,
                                                OstreeAsyncProgress *progress, gboolean *out_changed,

src/daemon/rpmostreed-transaction-types.cxx

@@ -1128,6 +1128,8 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca
   if (upgrader == NULL)
     return FALSE;
 
+  rpmostree_sysroot_upgrader_set_allow_empty_transaction (upgrader, idempotent_layering);
+
   g_autoptr (RpmOstreeOrigin) origin = rpmostree_sysroot_upgrader_dup_origin (upgrader);
 
   /* Handle local repo remotes immediately; the idea is that the remote is "transient"
@@ -1191,6 +1193,7 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca
     }
 
   gboolean changed = FALSE;
+  gboolean remove_changed = FALSE;
   if (no_initramfs
       && (rpmostree_origin_get_regenerate_initramfs (origin)
           || rpmostree_origin_has_initramfs_etc_files (origin)))
@@ -1214,7 +1217,10 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca
   if (no_layering)
     {
       if (rpmostree_origin_remove_all_packages (origin))
-        changed = TRUE;
+        {
+          changed = TRUE;
+          remove_changed = TRUE;
+        }
     }
   else
     {
@@ -1223,43 +1229,60 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca
        * create a new deployment; see https://github.com/projectatomic/rpm-ostree/issues/753 */
       if (!rpmostree_origin_remove_packages (origin,
                                              util::rust_stringvec_from_strv (uninstall_pkgs),
-                                             idempotent_layering, &changed, error))
+                                             idempotent_layering, &remove_changed, error))
         return FALSE;
+      if (remove_changed)
+        changed = TRUE;
     }
 
   /* lazily loaded cache that's used in a few conditional blocks */
   g_autoptr (RpmOstreeRefSack) base_rsack = NULL;
 
+  /* Check if we should skip the base-db check:
+   * When booted from a container image on an ostree host,
+   * we skip the base commit rpmdb check because container images don't have
+   * meaningful base-db separation like traditional ostree commits do. */
+  gboolean skip_base_check = FALSE;
+  CXX_TRY_VAR (host_type, rpmostreecxx::get_system_host_type (), error);
+  if (host_type == rpmostreecxx::SystemHostType::OstreeHost)
+    {
+      const auto refspec = rpmostree_origin_get_refspec (origin);
+      skip_base_check = (refspec.kind == rpmostreecxx::RefspecType::Container);
+    }
+
   if (install_pkgs)
     {
       /* we run a special check here; let's just not allow trying to install a pkg that will
        * right away become inactive because it's already installed */
 
-      if (!base_rsack)
+      if (!skip_base_check)
         {
-          const char *base = rpmostree_sysroot_upgrader_get_base (upgrader);
-          base_rsack = rpmostree_get_refsack_for_commit (repo, base, cancellable, error);
-          if (base_rsack == NULL)
-            return FALSE;
-        }
+          if (!base_rsack)
+            {
+              const char *base = rpmostree_sysroot_upgrader_get_base (upgrader);
+              base_rsack = rpmostree_get_refsack_for_commit (repo, base, cancellable, error);
+              if (base_rsack == NULL)
+                return FALSE;
+            }
 
-      for (char **it = install_pkgs; it && *it; it++)
-        {
-          const char *pkg = *it;
-          g_autoptr (GPtrArray) pkgs = rpmostree_get_matching_packages (base_rsack->sack, pkg);
-          if (pkgs->len > 0 && !allow_inactive)
+          for (char **it = install_pkgs; it && *it; it++)
             {
-              g_autoptr (GString) pkgnames = g_string_new ("");
-              for (guint i = 0; i < pkgs->len; i++)
+              const char *pkg = *it;
+              g_autoptr (GPtrArray) pkgs = rpmostree_get_matching_packages (base_rsack->sack, pkg);
+              if (pkgs->len > 0 && !allow_inactive)
                 {
-                  auto p = static_cast<DnfPackage *> (pkgs->pdata[i]);
-                  g_string_append_printf (pkgnames, " %s", dnf_package_get_nevra (p));
+                  g_autoptr (GString) pkgnames = g_string_new ("");
+                  for (guint i = 0; i < pkgs->len; i++)
+                    {
+                      auto p = static_cast<DnfPackage *> (pkgs->pdata[i]);
+                      g_string_append_printf (pkgnames, " %s", dnf_package_get_nevra (p));
+                    }
+                  return glnx_throw (error,
+                                     "\"%s\" is already provided by:%s. Use "
+                                     "--allow-inactive to explicitly "
+                                     "require it.",
+                                     pkg, pkgnames->str);
                 }
-              return glnx_throw (error,
-                                 "\"%s\" is already provided by:%s. Use "
-                                 "--allow-inactive to explicitly "
-                                 "require it.",
-                                 pkg, pkgnames->str);
             }
         }
 
@@ -1565,6 +1588,15 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca
     return FALSE;
   changed = changed || layering_changed;
 
+  /* When using containers and nothing changed after prep_layering, return early.
+   * This happens when all requested packages are already present in the container image.
+   * For idempotent mode, this avoids the "No packages in transaction" error.
+   * However, if packages were removed from the origin, we need to deploy to update the origin
+   * even if no layering is needed. Similarly, if we're rebasing (refspec) or deploying a
+   * specific revision, we need to deploy. */
+  if (skip_base_check && !layering_changed && !remove_changed && !self->refspec && !self->revision)
+    return TRUE;
+
   if (dry_run)
     /* Note early return here; we printed the transaction already */
     return TRUE;

src/libpriv/rpmostree-core-private.h

@@ -40,6 +40,7 @@ struct _RpmOstreeContext
   std::optional<rust::Box<rpmostreecxx::Treefile>> treefile_owned;
   rpmostreecxx::Treefile *treefile_rs; /* For composes for now */
   gboolean empty;
+  gboolean allow_empty_transaction;
   gboolean disable_selinux;
   char *ref;
 

src/libpriv/rpmostree-core.cxx

@@ -381,6 +381,12 @@ rpmostree_context_set_is_empty (RpmOstreeContext *self)
   self->empty = TRUE;
 }
 
+void
+rpmostree_context_set_allow_empty_transaction (RpmOstreeContext *self, gboolean allow)
+{
+  self->allow_empty_transaction = allow;
+}
+
 void
 rpmostree_context_disable_selinux (RpmOstreeContext *self)
 {
@@ -784,7 +790,8 @@ rpmostree_context_setup (RpmOstreeContext *self, const char *install_root, const
        * we're only installing local RPMs. Missing deps will cause the regular
        * 'not found' error from libdnf. */
       auto pkgs = self->treefile_rs->get_packages ();
-      if (!pkgs.empty ())
+      auto local_pkgs = self->treefile_rs->get_local_packages ();
+      if (!pkgs.empty () && local_pkgs.empty ())
         return glnx_throw (error, "No enabled repositories");
     }
 
@@ -4229,8 +4236,11 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
   g_autoptr (GPtrArray) overrides_remove = dnf_goal_get_packages (
       dnf_context_get_goal (dnfctx), DNF_PACKAGE_INFO_REMOVE, DNF_PACKAGE_INFO_OBSOLETE, -1);
 
-  if (overlays->len == 0 && overrides_remove->len == 0 && overrides_replace->len == 0)
-    return glnx_throw (error, "No packages in transaction");
+  if (rpmostree_dnf_context_has_empty_transaction (dnfctx))
+    {
+      if (!self->allow_empty_transaction)
+        return glnx_throw (error, "No packages in transaction");
+    }
 
   /* Sort the packages as rpmtsOrder() only reorder to satisfy dependencies
    * but doesn't impose any ordering to packages with the same dependencies.

src/libpriv/rpmostree-core.h

@@ -109,6 +109,7 @@ void rpmostree_context_configure_from_deployment (RpmOstreeContext *self, Ostree
                                                   OstreeDeployment *cfg_deployment);
 
 void rpmostree_context_set_is_empty (RpmOstreeContext *self);
+void rpmostree_context_set_allow_empty_transaction (RpmOstreeContext *self, gboolean allow);
 void rpmostree_context_disable_selinux (RpmOstreeContext *self);
 const char *rpmostree_context_get_ref (RpmOstreeContext *self);
 

src/libpriv/rpmostree-rpm-util.cxx

@@ -1048,6 +1048,21 @@ rpmostree_print_transaction (DnfContext *dnfctx)
     rpmostree_output_message ("Empty transaction");
 }
 
+/* Helper function to check if a DNF transaction has any packages to install, remove, or update */
+gboolean
+rpmostree_dnf_context_has_empty_transaction (DnfContext *dnfctx)
+{
+  HyGoal goal = dnf_context_get_goal (dnfctx);
+
+  g_autoptr (GPtrArray) install_pkgs = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_INSTALL, -1);
+  g_autoptr (GPtrArray) remove_pkgs
+      = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_REMOVE, DNF_PACKAGE_INFO_OBSOLETE, -1);
+  g_autoptr (GPtrArray) update_pkgs
+      = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_UPDATE, DNF_PACKAGE_INFO_DOWNGRADE, -1);
+
+  return (install_pkgs->len == 0 && remove_pkgs->len == 0 && update_pkgs->len == 0);
+}
+
 G_DEFINE_AUTO_CLEANUP_FREE_FUNC (cap_t, cap_free, NULL)
 
 GVariant *

src/libpriv/rpmostree-rpm-util.h

@@ -117,6 +117,8 @@ gint rpmostree_pkg_array_compare (DnfPackage **p_pkg1, DnfPackage **p_pkg2);
 
 void rpmostree_print_transaction (DnfContext *context);
 
+gboolean rpmostree_dnf_context_has_empty_transaction (DnfContext *dnfctx);
+
 GVariant *rpmostree_fcap_to_ostree_xattr (const char *fcap, GError **error);
 GVariant *rpmostree_fcap_to_xattr_variant (const char *fcap, GError **error);