Fix pruning of partial commits

If we have a partial commit it is not an error for a dirmeta to be
missing (in fact, that is likely), so instead of returning a not-found
error from ostree_repo_traverse_commit() we ignore the error and
continue.

In particular, this means we don't stop early at the first
missing dirmeta, which previously caused ostree_repo_prune() to
thing the dirmetas after that to be unreached and thus purged.

Also, we remove the special casing in ostree_repo_prune() to
not report errors for commitpartial, because these should not
be reported anymore.

This fixes https://github.com/ostreedev/ostree/issues/541

Closes: #542
Approved by: cgwalters

src/libostree/ostree-repo-prune.c

@@ -311,29 +311,11 @@ ostree_repo_prune (OstreeRepo        *self,
       while (g_hash_table_iter_next (&hash_iter, &key, &value))
         {
           const char *checksum = value;
-          OstreeRepoCommitState commitstate;
-          GError *local_error = NULL;
-
-          if (!ostree_repo_load_commit (self, checksum, NULL, &commitstate,
-                                        error))
-            goto out;
 
           g_debug ("Finding objects to keep for commit %s", checksum);
           if (!ostree_repo_traverse_commit_union (self, checksum, depth, data.reachable,
-                                                  cancellable, &local_error))
-            {
-              /* Don't fail traversing a partial commit */
-              if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0 &&
-                  g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
-                {
-                  g_clear_error (&local_error);
-                }
-              else
-                {
-                  g_propagate_error (error, local_error);
-                  goto out;
-                }
-            }
+                                                  cancellable, error))
+            goto out;
         }
     }
 
@@ -349,34 +331,16 @@ ostree_repo_prune (OstreeRepo        *self,
           GVariant *serialized_key = key;
           const char *checksum;
           OstreeObjectType objtype;
-          OstreeRepoCommitState commitstate;
-          GError *local_error = NULL;
 
           ostree_object_name_deserialize (serialized_key, &checksum, &objtype);
 
           if (objtype != OSTREE_OBJECT_TYPE_COMMIT)
             continue;
 
-          if (!ostree_repo_load_commit (self, checksum, NULL, &commitstate,
-                                        error))
-            goto out;
-
           g_debug ("Finding objects to keep for commit %s", checksum);
           if (!ostree_repo_traverse_commit_union (self, checksum, depth, data.reachable,
-                                                  cancellable, &local_error))
-            {
-              /* Don't fail traversing a partial commit */
-              if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0 &&
-                  g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
-                {
-                  g_clear_error (&local_error);
-                }
-              else
-                {
-                  g_propagate_error (error, local_error);
-                  goto out;
-                }
-            }
+                                                  cancellable, error))
+            goto out;
         }
     }
 

src/libostree/ostree-repo-traverse.c

@@ -301,6 +301,7 @@ static gboolean
 traverse_dirtree (OstreeRepo           *repo,
                   const char           *checksum,
                   GHashTable           *inout_reachable,
+                  gboolean              ignore_missing_dirs,
                   GCancellable         *cancellable,
                   GError              **error);
 
@@ -308,6 +309,7 @@ static gboolean
 traverse_iter (OstreeRepo                          *repo,
                OstreeRepoCommitTraverseIter        *iter,
                GHashTable                          *inout_reachable,
+               gboolean                             ignore_missing_dirs,
                GCancellable                        *cancellable,
                GError                             **error)
 {
@@ -316,11 +318,26 @@ traverse_iter (OstreeRepo                          *repo,
   while (TRUE)
     {
       g_autoptr(GVariant) key = NULL;
+      g_autoptr(GError) local_error = NULL;
       OstreeRepoCommitIterResult iterres =
-        ostree_repo_commit_traverse_iter_next (iter, cancellable, error);
-          
+        ostree_repo_commit_traverse_iter_next (iter, cancellable, &local_error);
+
       if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_ERROR)
-        goto out;
+        {
+          /* There is only one kind of not-found error, which is
+             failing to load the dirmeta itself, if so, we ignore that
+             (and the whole subtree) if told to. */
+          if (ignore_missing_dirs &&
+              g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
+            {
+              g_debug ("Ignoring not-found dirmeta");
+              ret = TRUE;
+            }
+          else
+            g_propagate_error (error, g_steal_pointer (&local_error));
+
+          goto out;
+        }
       else if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_END)
         break;
       else if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_FILE)
@@ -357,7 +374,7 @@ traverse_iter (OstreeRepo                          *repo,
               key = NULL;
 
               if (!traverse_dirtree (repo, content_checksum, inout_reachable,
-                                     cancellable, error))
+                                     ignore_missing_dirs, cancellable, error))
                 goto out;
             }
         }
@@ -374,6 +391,7 @@ static gboolean
 traverse_dirtree (OstreeRepo           *repo,
                   const char           *checksum,
                   GHashTable           *inout_reachable,
+                  gboolean              ignore_missing_dirs,
                   GCancellable         *cancellable,
                   GError              **error)
 {
@@ -381,10 +399,22 @@ traverse_dirtree (OstreeRepo           *repo,
   g_autoptr(GVariant) dirtree = NULL;
   ostree_cleanup_repo_commit_traverse_iter
     OstreeRepoCommitTraverseIter iter = { 0, };
+  g_autoptr(GError) local_error = NULL;
 
   if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_DIR_TREE, checksum,
-                                 &dirtree, error))
-    goto out;
+                                 &dirtree, &local_error))
+    {
+      if (ignore_missing_dirs &&
+          g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
+        {
+          g_print ("Ignoring not-found dirmeta %s", checksum);
+          ret = TRUE;
+        }
+      else
+        g_propagate_error (error, g_steal_pointer (&local_error));
+
+      goto out;
+    }
 
   g_debug ("Traversing dirtree %s", checksum);
   if (!ostree_repo_commit_traverse_iter_init_dirtree (&iter, repo, dirtree,
@@ -392,7 +422,7 @@ traverse_dirtree (OstreeRepo           *repo,
                                                       error))
     goto out;
 
-  if (!traverse_iter (repo, &iter, inout_reachable, cancellable, error))
+  if (!traverse_iter (repo, &iter, inout_reachable, ignore_missing_dirs, cancellable, error))
     goto out;
 
   ret = TRUE;
@@ -430,6 +460,8 @@ ostree_repo_traverse_commit_union (OstreeRepo      *repo,
       g_autoptr(GVariant) commit = NULL;
       ostree_cleanup_repo_commit_traverse_iter
         OstreeRepoCommitTraverseIter iter = { 0, };
+      OstreeRepoCommitState commitstate;
+      gboolean ignore_missing_dirs = FALSE;
 
       key = ostree_object_name_serialize (commit_checksum, OSTREE_OBJECT_TYPE_COMMIT);
 
@@ -440,13 +472,21 @@ ostree_repo_traverse_commit_union (OstreeRepo      *repo,
                                                commit_checksum, &commit,
                                                error))
         goto out;
-        
+
       /* Just return if the parent isn't found; we do expect most
        * people to have partial repositories.
        */
       if (!commit)
         break;
 
+      /* See if the commit is partial, if so it's not an error to lack objects */
+      if (!ostree_repo_load_commit (repo, commit_checksum, NULL, &commitstate,
+                                    error))
+        goto out;
+
+      if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) != 0)
+        ignore_missing_dirs = TRUE;
+
       g_hash_table_add (inout_reachable, key);
       key = NULL;
 
@@ -456,9 +496,9 @@ ostree_repo_traverse_commit_union (OstreeRepo      *repo,
                                                          error))
         goto out;
 
-      if (!traverse_iter (repo, &iter, inout_reachable, cancellable, error))
+      if (!traverse_iter (repo, &iter, inout_reachable, ignore_missing_dirs, cancellable, error))
         goto out;
-      
+
       if (maxdepth == -1 || maxdepth > 0)
         {
           g_free (tmp_checksum);

tests/test-pull-subpath.sh

@@ -23,7 +23,7 @@ set -euo pipefail
 
 setup_fake_remote_repo1 "archive-z2"
 
-echo '1..2'
+echo '1..4'
 
 repopath=${test_tmpdir}/ostree-srv/gnomerepo
 cp -a ${repopath} ${repopath}.orig
@@ -54,5 +54,26 @@ ${CMD_PREFIX} ostree --repo=repo ls origin:main /firstfile
 ${CMD_PREFIX} ostree --repo=repo pull origin main
 assert_not_has_file repo/state/${rev}.commitpartial
 ${CMD_PREFIX} ostree --repo=repo fsck
-echo "ok"
+echo "ok subpaths"
+
+rm -rf repo
+
+${CMD_PREFIX} ostree --repo=repo init
+${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin ${remoteurl}
+
+# Pull a directory which is not the first in the commit (/baz/another is before)
+${CMD_PREFIX} ostree --repo=repo pull --subpath=/baz/deeper origin main
+
+# Ensure it is there
+${CMD_PREFIX} ostree --repo=repo ls origin:main /baz/deeper
+
+# Now prune, this should not prune the /baz/deeper dirmeta even if the
+# /baz/another dirmeta is not in the repo.
+${CMD_PREFIX} ostree --repo=repo prune --refs-only
+
+# Ensure it is still there
+${CMD_PREFIX} ostree --repo=repo ls origin:main /baz/deeper
+
+echo "ok prune with commitpartial"
+
 done