pull: Only download summary if we need it for the pull operation

If we have a commit id for all the refs we're pulling, and if we
don't need the summary to list all the refs when mirroring then the
only reason to download the summary is for the list of deltas.

With the new "indexed-deltas" property in the config file (and mirrored
to the summary file) we can detect when we don't need the summary for
deltas and completely avoid downloading it then.

2 files changed, 480 insertions, 435 deletions

diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h
index d1c5fd14..a827557a 100644
--- a/src/libostree/ostree-repo-pull-private.h
+++ b/src/libostree/ostree-repo-pull-private.h
@@ -80,6 +80,7 @@ typedef struct {
   GVariant         *summary;
   GHashTable       *summary_deltas_checksums; /* Filled from summary and delta indexes */
   gboolean          summary_has_deltas; /* True if the summary existed and had a delta index */
+  gboolean          has_indexed_deltas;
   GHashTable       *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */
   GHashTable       *verified_commits; /* Set<checksum> of commits that have been verified */
   GHashTable       *signapi_verified_commits; /* Map<checksum,verification> of commits that have been signapi verified */
diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 70cbeca2..81956d3d 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -3535,18 +3535,17 @@ initiate_request (OtPullData                 *pull_data,
         return FALSE;
     }
 
-  /* If we have a summary, we can use the newer logic */
-  if (pull_data->summary)
+  /* If we have a summary or delta index, we can use the newer logic.
+   * We prefer the index as it might have more deltas than the summary
+   * (i.e. leave some deltas out of summary to make it smaller). */
+  if (pull_data->has_indexed_deltas)
     {
-      if (!pull_data->summary_has_deltas)
-        {
-          enqueue_one_static_delta_index_request (pull_data, to_revision, delta_from_revision, ref);
-        }
-      else
-        {
-          if (!initiate_delta_request (pull_data, ref, to_revision, delta_from_revision, error))
-            return FALSE;
-        }
+      enqueue_one_static_delta_index_request (pull_data, to_revision, delta_from_revision, ref);
+    }
+  else if (pull_data->summary_has_deltas)
+    {
+      if (!initiate_delta_request (pull_data, ref, to_revision, delta_from_revision, error))
+        return FALSE;
     }
   else if (ref != NULL)
     {
@@ -3590,6 +3589,20 @@ initiate_request (OtPullData                 *pull_data,
   return TRUE;
 }
 
+static gboolean
+all_requested_refs_have_commit (GHashTable *requested_refs /* (element-type OstreeCollectionRef utf8) */)
+{
+  GLNX_HASH_TABLE_FOREACH_KV (requested_refs, const OstreeCollectionRef*, ref,
+                              const char*, override_commitid)
+    {
+      /* Note: "" override means whatever is latest */
+      if (override_commitid == NULL || *override_commitid == 0)
+        return FALSE;
+    }
+
+  return TRUE;
+}
+
 /* ------------------------------------------------------------------------------------------
  * Below is the libsoup-invariant API; these should match
  * the stub functions in the #else clause
@@ -3695,9 +3708,11 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   gboolean opt_ref_keyring_map_set = FALSE;
   gboolean disable_sign_verify = FALSE;
   gboolean disable_sign_verify_summary = FALSE;
+  gboolean need_summary = FALSE;
   const char *main_collection_id = NULL;
   const char *url_override = NULL;
   gboolean inherit_transaction = FALSE;
+  gboolean require_summary_for_mirror = FALSE;
   g_autoptr(GHashTable) updated_requested_refs_to_fetch = NULL;  /* (element-type OstreeCollectionRef utf8) */
   gsize i;
   g_autofree char **opt_localcache_repos = NULL;
@@ -3709,6 +3724,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
    */
   const char *the_ref_to_fetch = NULL;
   OstreeRepoTransactionStats tstats = { 0, };
+  gboolean remote_mode_loaded = FALSE;
 
   /* Default */
   pull_data->max_metadata_size = OSTREE_MAX_METADATA_SIZE;
@@ -4132,408 +4148,509 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   if (pull_data->is_commit_only)
     pull_data->disable_static_deltas = TRUE;
 
-  pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref);
+  /* Compute the set of collection-refs (and optional commit id) to fetch */
 
-  {
-    g_autoptr(GBytes) bytes_sig = NULL;
-    gboolean summary_sig_not_modified = FALSE;
-    g_autofree char *summary_sig_etag = NULL;
-    guint64 summary_sig_last_modified = 0;
-    gsize n;
-    g_autoptr(GVariant) refs = NULL;
-    g_autoptr(GVariant) deltas = NULL;
-    g_autoptr(GVariant) additional_metadata = NULL;
-    gboolean summary_from_cache = FALSE;
-    gboolean remote_mode_loaded = FALSE;
-    gboolean tombstone_commits = FALSE;
-
-    if (summary_sig_bytes_v)
-      {
-        /* Must both be specified */
-        g_assert (summary_bytes_v);
-
-        bytes_sig = g_variant_get_data_as_bytes (summary_sig_bytes_v);
-        bytes_summary = g_variant_get_data_as_bytes (summary_bytes_v);
+  if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set && !configured_branches)
+    {
+      require_summary_for_mirror = TRUE;
+    }
+  else if (opt_collection_refs_set)
+    {
+      const gchar *collection_id, *ref_name, *checksum;
 
-        if (!bytes_sig || !bytes_summary)
-          {
-            g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                         "summary-bytes or summary-sig-bytes set to invalid value");
+      while (g_variant_iter_loop (collection_refs_iter, "(&s&s&s)", &collection_id, &ref_name, &checksum))
+        {
+          if (!ostree_validate_rev (ref_name, error))
             goto out;
-          }
+          g_hash_table_insert (requested_refs_to_fetch,
+                               ostree_collection_ref_new (collection_id, ref_name),
+                               (*checksum != '\0') ? g_strdup (checksum) : NULL);
+        }
+    }
+  else if (refs_to_fetch != NULL)
+    {
+      char **strviter = refs_to_fetch;
+      char **commitid_strviter = override_commit_ids ?: NULL;
 
-        g_debug ("Loaded %s summary from options", remote_name_or_baseurl);
-      }
+      while (*strviter)
+        {
+          const char *branch = *strviter;
 
-    if (!bytes_sig)
-      {
-        g_autofree char *summary_sig_if_none_match = NULL;
-        guint64 summary_sig_if_modified_since = 0;
-
-        /* Load the summary.sig from the network, but send its ETag and
-         * Last-Modified from the on-disk cache (if it exists) to reduce the
-         * download size if nothing’s changed. */
-        _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig",
-                                                    &summary_sig_if_none_match, &summary_sig_if_modified_since);
-
-        g_clear_pointer (&summary_sig_etag, g_free);
-        summary_sig_last_modified = 0;
-        if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
-                                                         pull_data->meta_mirrorlist,
-                                                         "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
-                                                         summary_sig_if_none_match, summary_sig_if_modified_since,
-                                                         pull_data->n_network_retries,
-                                                         &bytes_sig,
-                                                         &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified,
-                                                         OSTREE_MAX_METADATA_SIZE,
-                                                         cancellable, error))
+          if (ostree_validate_checksum_string (branch, NULL))
+            {
+              char *key = g_strdup (branch);
+              g_hash_table_add (commits_to_fetch, key);
+            }
+          else
+            {
+              if (!ostree_validate_rev (branch, error))
+                goto out;
+              char *commitid = commitid_strviter ? g_strdup (*commitid_strviter) : NULL;
+              g_hash_table_insert (requested_refs_to_fetch,
+                                   ostree_collection_ref_new (NULL, branch), commitid);
+            }
+
+          strviter++;
+          if (commitid_strviter)
+            commitid_strviter++;
+        }
+    }
+  else
+    {
+      char **branches_iter;
+
+      branches_iter = configured_branches;
+
+      if (!(branches_iter && *branches_iter))
+        {
+          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                       "No configured branches for remote %s", remote_name_or_baseurl);
           goto out;
+        }
+      for (;branches_iter && *branches_iter; branches_iter++)
+        {
+          const char *branch = *branches_iter;
 
-        /* The server returned HTTP status 304 Not Modified, so we’re clear to
-         * load summary.sig from the cache. Also load summary, since
-         * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */
-        if (summary_sig_not_modified)
-          {
-            g_clear_pointer (&bytes_sig, g_bytes_unref);
-            g_clear_pointer (&bytes_summary, g_bytes_unref);
-            if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig",
-                                                       &bytes_sig,
-                                                       cancellable, error))
-              goto out;
+          g_hash_table_insert (requested_refs_to_fetch,
+                               ostree_collection_ref_new (NULL, branch), NULL);
+        }
+    }
 
-            if (!bytes_summary &&
-                !pull_data->remote_repo_local &&
-                !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL,
-                                                       &bytes_summary,
-                                                       cancellable, error))
-              goto out;
-          }
-      }
+  /* Deltas are necessary when mirroring or resolving a requested ref to a commit.
+   * We try to avoid loading the potentially large summary if it is not needed. */
+  need_summary = require_summary_for_mirror || !all_requested_refs_have_commit (requested_refs_to_fetch) || summary_sig_bytes_v != NULL;
 
-    if (bytes_sig &&
-        !bytes_summary &&
-        !pull_data->remote_repo_local &&
-        !_ostree_repo_load_cache_summary_if_same_sig (self,
-                                                      remote_name_or_baseurl,
-                                                      bytes_sig,
-                                                      &bytes_summary,
-                                                      cancellable,
-                                                      error))
-      goto out;
+  /* If we don't have indexed deltas, we need the summary for deltas, so check
+   * the config file for support.
+   * NOTE: Avoid download if we don't need deltas */
+  if (!need_summary && !pull_data->disable_static_deltas)
+    {
+      if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error))
+        goto out;
 
-    if (bytes_summary && !summary_bytes_v)
-      {
-        g_debug ("Loaded %s summary from cache", remote_name_or_baseurl);
-        summary_from_cache = TRUE;
-      }
+      /* Check if remote has delta indexes outside summary */
+      if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "indexed-deltas", FALSE,
+                                                &pull_data->has_indexed_deltas, error))
+        goto out;
 
-    if (!pull_data->summary && !bytes_summary)
-      {
-        g_autofree char *summary_if_none_match = NULL;
-        guint64 summary_if_modified_since = 0;
+      if (!pull_data->has_indexed_deltas)
+        need_summary = TRUE;
+   }
+
+  pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref);
 
-        _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL,
-                                                    &summary_if_none_match, &summary_if_modified_since);
+  if (need_summary)
+    {
+      g_autoptr(GBytes) bytes_sig = NULL;
+      gboolean summary_sig_not_modified = FALSE;
+      g_autofree char *summary_sig_etag = NULL;
+      guint64 summary_sig_last_modified = 0;
+      gsize n;
+      g_autoptr(GVariant) refs = NULL;
+      g_autoptr(GVariant) deltas = NULL;
+      g_autoptr(GVariant) additional_metadata = NULL;
+      gboolean summary_from_cache = FALSE;
+      gboolean tombstone_commits = FALSE;
 
-        g_clear_pointer (&summary_etag, g_free);
-        summary_last_modified = 0;
-        if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
-                                                         pull_data->meta_mirrorlist,
-                                                         "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
-                                                         summary_if_none_match, summary_if_modified_since,
-                                                         pull_data->n_network_retries,
+      if (summary_sig_bytes_v)
+        {
+          /* Must both be specified */
+          g_assert (summary_bytes_v);
+
+          bytes_sig = g_variant_get_data_as_bytes (summary_sig_bytes_v);
+          bytes_summary = g_variant_get_data_as_bytes (summary_bytes_v);
+
+          if (!bytes_sig || !bytes_summary)
+            {
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "summary-bytes or summary-sig-bytes set to invalid value");
+              goto out;
+            }
+
+          g_debug ("Loaded %s summary from options", remote_name_or_baseurl);
+        }
+
+      if (!bytes_sig)
+        {
+          g_autofree char *summary_sig_if_none_match = NULL;
+          guint64 summary_sig_if_modified_since = 0;
+
+          /* Load the summary.sig from the network, but send its ETag and
+           * Last-Modified from the on-disk cache (if it exists) to reduce the
+           * download size if nothing’s changed. */
+          _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig",
+                                                      &summary_sig_if_none_match, &summary_sig_if_modified_since);
+
+          g_clear_pointer (&summary_sig_etag, g_free);
+          summary_sig_last_modified = 0;
+          if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
+                                                           pull_data->meta_mirrorlist,
+                                                           "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
+                                                           summary_sig_if_none_match, summary_sig_if_modified_since,
+                                                           pull_data->n_network_retries,
+                                                           &bytes_sig,
+                                                           &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified,
+                                                           OSTREE_MAX_METADATA_SIZE,
+                                                           cancellable, error))
+            goto out;
+
+          /* The server returned HTTP status 304 Not Modified, so we’re clear to
+           * load summary.sig from the cache. Also load summary, since
+           * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */
+          if (summary_sig_not_modified)
+            {
+              g_clear_pointer (&bytes_sig, g_bytes_unref);
+              g_clear_pointer (&bytes_summary, g_bytes_unref);
+              if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig",
+                                                         &bytes_sig,
+                                                         cancellable, error))
+                goto out;
+
+              if (!bytes_summary &&
+                  !pull_data->remote_repo_local &&
+                  !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL,
                                                          &bytes_summary,
-                                                         &summary_not_modified, &summary_etag, &summary_last_modified,
-                                                         OSTREE_MAX_METADATA_SIZE,
                                                          cancellable, error))
-          goto out;
+                goto out;
+            }
+        }
 
-        /* The server returned HTTP status 304 Not Modified, so we’re clear to
-         * load summary from the cache. */
-        if (summary_not_modified)
-          {
-            g_clear_pointer (&bytes_summary, g_bytes_unref);
-            if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL,
-                                                       &bytes_summary,
-                                                       cancellable, error))
-              goto out;
-          }
-      }
+      if (bytes_sig &&
+          !bytes_summary &&
+          !pull_data->remote_repo_local &&
+          !_ostree_repo_load_cache_summary_if_same_sig (self,
+                                                        remote_name_or_baseurl,
+                                                        bytes_sig,
+                                                        &bytes_summary,
+                                                        cancellable,
+                                                        error))
+        goto out;
+
+      if (bytes_summary && !summary_bytes_v)
+        {
+          g_debug ("Loaded %s summary from cache", remote_name_or_baseurl);
+          summary_from_cache = TRUE;
+        }
+
+      if (!pull_data->summary && !bytes_summary)
+        {
+          g_autofree char *summary_if_none_match = NULL;
+          guint64 summary_if_modified_since = 0;
+
+          _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL,
+                                                      &summary_if_none_match, &summary_if_modified_since);
+
+          g_clear_pointer (&summary_etag, g_free);
+          summary_last_modified = 0;
+
+          if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
+                                                           pull_data->meta_mirrorlist,
+                                                           "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
+                                                           summary_if_none_match, summary_if_modified_since,
+                                                           pull_data->n_network_retries,
+                                                           &bytes_summary,
+                                                           &summary_not_modified, &summary_etag, &summary_last_modified,
+                                                           OSTREE_MAX_METADATA_SIZE,
+                                                           cancellable, error))
+            goto out;
+
+          /* The server returned HTTP status 304 Not Modified, so we’re clear to
+           * load summary from the cache. */
+          if (summary_not_modified)
+            {
+              g_clear_pointer (&bytes_summary, g_bytes_unref);
+              if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL,
+                                                         &bytes_summary,
+                                                         cancellable, error))
+                goto out;
+            }
+        }
 
 #ifndef OSTREE_DISABLE_GPGME
-    if (!bytes_summary && pull_data->gpg_verify_summary)
-      {
-        g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                     "GPG verification enabled, but no summary found (use gpg-verify-summary=false in remote config to disable)");
-        goto out;
-      }
+      if (!bytes_summary && pull_data->gpg_verify_summary)
+        {
+          g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                       "GPG verification enabled, but no summary found (use gpg-verify-summary=false in remote config to disable)");
+          goto out;
+        }
 #endif /* OSTREE_DISABLE_GPGME */
 
-    if (!bytes_summary && pull_data->require_static_deltas)
-      {
-        g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                     "Fetch configured to require static deltas, but no summary found");
-        goto out;
-      }
+      if (!bytes_summary && require_summary_for_mirror)
+        {
+          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                       "Fetching all refs was requested in mirror mode, but remote repository does not have a summary");
+          goto out;
+        }
 
 #ifndef OSTREE_DISABLE_GPGME
-    if (!bytes_sig && pull_data->gpg_verify_summary)
-      {
-        g_set_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_NO_SIGNATURE,
-                     "GPG verification enabled, but no summary.sig found (use gpg-verify-summary=false in remote config to disable)");
-        goto out;
-      }
+      if (!bytes_sig && pull_data->gpg_verify_summary)
+        {
+          g_set_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_NO_SIGNATURE,
+                       "GPG verification enabled, but no summary.sig found (use gpg-verify-summary=false in remote config to disable)");
+          goto out;
+        }
 
-    if (pull_data->gpg_verify_summary && bytes_summary && bytes_sig)
-      {
-        g_autoptr(OstreeGpgVerifyResult) result = NULL;
-        g_autoptr(GError) temp_error = NULL;
+      if (pull_data->gpg_verify_summary && bytes_summary && bytes_sig)
+        {
+          g_autoptr(OstreeGpgVerifyResult) result = NULL;
+          g_autoptr(GError) temp_error = NULL;
 
-        result = ostree_repo_verify_summary (self, pull_data->remote_name,
-                                             bytes_summary, bytes_sig,
-                                             cancellable, &temp_error);
-        if (!ostree_gpg_verify_result_require_valid_signature (result, &temp_error))
-          {
-            if (summary_from_cache)
-              {
-                /* The cached summary doesn't match, fetch a new one and verify again.
-                 * Don’t set the cache headers in the HTTP request, to force a
-                 * full download. */
-                if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0)
-                  {
-                    g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                                 "Remote %s cached summary invalid and "
-                                 "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified",
-                                 pull_data->remote_name);
+          result = ostree_repo_verify_summary (self, pull_data->remote_name,
+                                               bytes_summary, bytes_sig,
+                                               cancellable, &temp_error);
+          if (!ostree_gpg_verify_result_require_valid_signature (result, &temp_error))
+            {
+              if (summary_from_cache)
+                {
+                  /* The cached summary doesn't match, fetch a new one and verify again.
+                   * Don’t set the cache headers in the HTTP request, to force a
+                   * full download. */
+                  if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0)
+                    {
+                      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                   "Remote %s cached summary invalid and "
+                                   "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified",
+                                   pull_data->remote_name);
+                      goto out;
+                    }
+                  else
+                    g_debug ("Remote %s cached summary invalid, pulling new version",
+                             pull_data->remote_name);
+
+                  summary_from_cache = FALSE;
+                  g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref);
+                  g_clear_pointer (&summary_etag, g_free);
+                  summary_last_modified = 0;
+                  if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
+                                                                   pull_data->meta_mirrorlist,
+                                                                   "summary",
+                                                                   OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
+                                                                   NULL, 0,  /* no cache headers */
+                                                                   pull_data->n_network_retries,
+                                                                   &bytes_summary,
+                                                                   &summary_not_modified, &summary_etag, &summary_last_modified,
+                                                                   OSTREE_MAX_METADATA_SIZE,
+                                                                   cancellable, error))
                     goto out;
-                  }
-                else
-                  g_debug ("Remote %s cached summary invalid, pulling new version",
-                           pull_data->remote_name);
-
-                summary_from_cache = FALSE;
-                g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref);
-                g_clear_pointer (&summary_etag, g_free);
-                summary_last_modified = 0;
-                if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
-                                                                 pull_data->meta_mirrorlist,
-                                                                 "summary",
-                                                                 OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
-                                                                 NULL, 0,  /* no cache headers */
-                                                                 pull_data->n_network_retries,
-                                                                 &bytes_summary,
-                                                                 &summary_not_modified, &summary_etag, &summary_last_modified,
-                                                                 OSTREE_MAX_METADATA_SIZE,
-                                                                 cancellable, error))
-                  goto out;
 
-                g_autoptr(OstreeGpgVerifyResult) retry =
-                  ostree_repo_verify_summary (self, pull_data->remote_name,
-                                              bytes_summary, bytes_sig,
-                                              cancellable, error);
-                if (!ostree_gpg_verify_result_require_valid_signature (retry, error))
+                  g_autoptr(OstreeGpgVerifyResult) retry =
+                    ostree_repo_verify_summary (self, pull_data->remote_name,
+                                                bytes_summary, bytes_sig,
+                                                cancellable, error);
+                  if (!ostree_gpg_verify_result_require_valid_signature (retry, error))
+                    goto out;
+                }
+              else
+                {
+                  g_propagate_error (error, g_steal_pointer (&temp_error));
                   goto out;
-              }
-            else
-              {
-                g_propagate_error (error, g_steal_pointer (&temp_error));
-                goto out;
-              }
-          }
-      }
+                }
+            }
+        }
 #endif /* OSTREE_DISABLE_GPGME */
 
-    if (pull_data->signapi_summary_verifiers)
-      {
-        if (!bytes_sig && pull_data->signapi_summary_verifiers)
-          {
-            g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                         "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)");
-            goto out;
-          }
-        if (bytes_summary && bytes_sig)
-          {
-            g_autoptr(GVariant) signatures = NULL;
-            g_autoptr(GError) temp_error = NULL;
-
-            signatures = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT,
-                                                   bytes_sig, FALSE);
-
-
-            g_assert (pull_data->signapi_summary_verifiers);
-            if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, &temp_error))
-              {
-                if (summary_from_cache)
-                  {
-                    /* The cached summary doesn't match, fetch a new one and verify again.
-                     * Don’t set the cache headers in the HTTP request, to force a
-                     * full download. */
-                    if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0)
-                      {
-                        g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                                     "Remote %s cached summary invalid and "
-                                     "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified",
-                                     pull_data->remote_name);
+      if (pull_data->signapi_summary_verifiers)
+        {
+          if (!bytes_sig && pull_data->signapi_summary_verifiers)
+            {
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)");
+              goto out;
+            }
+          if (bytes_summary && bytes_sig)
+            {
+              g_autoptr(GVariant) signatures = NULL;
+              g_autoptr(GError) temp_error = NULL;
+
+              signatures = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT,
+                                                     bytes_sig, FALSE);
+
+              g_assert (pull_data->signapi_summary_verifiers);
+              if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, &temp_error))
+                {
+                  if (summary_from_cache)
+                    {
+                      /* The cached summary doesn't match, fetch a new one and verify again.
+                       * Don’t set the cache headers in the HTTP request, to force a
+                       * full download. */
+                      if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0)
+                        {
+                          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                       "Remote %s cached summary invalid and "
+                                       "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified",
+                                       pull_data->remote_name);
+                          goto out;
+                        }
+                      else
+                        g_debug ("Remote %s cached summary invalid, pulling new version",
+                                 pull_data->remote_name);
+
+                      summary_from_cache = FALSE;
+                      g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref);
+                      g_clear_pointer (&summary_etag, g_free);
+                      summary_last_modified = 0;
+                      if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
+                                                                       pull_data->meta_mirrorlist,
+                                                                       "summary",
+                                                                       OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
+                                                                       NULL, 0,  /* no cache headers */
+                                                                       pull_data->n_network_retries,
+                                                                       &bytes_summary,
+                                                                       &summary_not_modified, &summary_etag, &summary_last_modified,
+                                                                       OSTREE_MAX_METADATA_SIZE,
+                                                                       cancellable, error))
                         goto out;
-                      }
-                    else
-                      g_debug ("Remote %s cached summary invalid, pulling new version",
-                               pull_data->remote_name);
-
-                    summary_from_cache = FALSE;
-                    g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref);
-                    g_clear_pointer (&summary_etag, g_free);
-                    summary_last_modified = 0;
-                    if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher,
-                                                                     pull_data->meta_mirrorlist,
-                                                                     "summary",
-                                                                     OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT,
-                                                                     NULL, 0,  /* no cache headers */
-                                                                     pull_data->n_network_retries,
-                                                                     &bytes_summary,
-                                                                     &summary_not_modified, &summary_etag, &summary_last_modified,
-                                                                     OSTREE_MAX_METADATA_SIZE,
-                                                                     cancellable, error))
-                      goto out;
 
-                    if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, error))
+                      if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, error))
                         goto out;
-                  }
-                else
-                  {
-                    g_propagate_error (error, g_steal_pointer (&temp_error));
-                    goto out;
-                  }
-              }
-          }
-      }
+                    }
+                  else
+                    {
+                      g_propagate_error (error, g_steal_pointer (&temp_error));
+                      goto out;
+                    }
+                }
+            }
+        }
 
-    if (bytes_summary)
-      {
-        pull_data->summary_data = g_bytes_ref (bytes_summary);
-        pull_data->summary_etag = g_strdup (summary_etag);
-        pull_data->summary_last_modified = summary_last_modified;
-        pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE);
+      if (bytes_summary)
+        {
+          pull_data->summary_data = g_bytes_ref (bytes_summary);
+          pull_data->summary_etag = g_strdup (summary_etag);
+          pull_data->summary_last_modified = summary_last_modified;
+          pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE);
 
-        if (!g_variant_is_normal_form (pull_data->summary))
-          {
-            g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                                 "Not normal form");
-            goto out;
-          }
-        if (!g_variant_is_of_type (pull_data->summary, OSTREE_SUMMARY_GVARIANT_FORMAT))
-          {
-            g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                         "Doesn't match variant type '%s'",
-                         (char *)OSTREE_SUMMARY_GVARIANT_FORMAT);
+          if (!g_variant_is_normal_form (pull_data->summary))
+            {
+              g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                   "Not normal form");
+              goto out;
+            }
+          if (!g_variant_is_of_type (pull_data->summary, OSTREE_SUMMARY_GVARIANT_FORMAT))
+            {
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Doesn't match variant type '%s'",
+                           (char *)OSTREE_SUMMARY_GVARIANT_FORMAT);
+              goto out;
+            }
+
+          if (bytes_sig)
+            {
+              pull_data->summary_data_sig = g_bytes_ref (bytes_sig);
+              pull_data->summary_sig_etag = g_strdup (summary_sig_etag);
+              pull_data->summary_sig_last_modified = summary_sig_last_modified;
+            }
+        }
+
+      if (!summary_from_cache && bytes_summary && bytes_sig)
+        {
+          if (!pull_data->remote_repo_local &&
+              !_ostree_repo_cache_summary (self,
+                                           remote_name_or_baseurl,
+                                           bytes_summary,
+                                           summary_etag, summary_last_modified,
+                                           bytes_sig,
+                                           summary_sig_etag, summary_sig_last_modified,
+                                           cancellable,
+                                           error))
             goto out;
-          }
+        }
 
-        if (bytes_sig)
-          {
-            pull_data->summary_data_sig = g_bytes_ref (bytes_sig);
-            pull_data->summary_sig_etag = g_strdup (summary_sig_etag);
-            pull_data->summary_sig_last_modified = summary_sig_last_modified;
-          }
-      }
+      if (pull_data->summary)
+        {
+          additional_metadata = g_variant_get_child_value (pull_data->summary, 1);
 
-    if (!summary_from_cache && bytes_summary && bytes_sig)
-      {
-        if (!pull_data->remote_repo_local &&
-            !_ostree_repo_cache_summary (self,
-                                         remote_name_or_baseurl,
-                                         bytes_summary,
-                                         summary_etag, summary_last_modified,
-                                         bytes_sig,
-                                         summary_sig_etag, summary_sig_last_modified,
-                                         cancellable,
-                                         error))
-          goto out;
-      }
+          if (!g_variant_lookup (additional_metadata, OSTREE_SUMMARY_COLLECTION_ID, "&s", &main_collection_id))
+            main_collection_id = NULL;
+          else if (!ostree_validate_collection_id (main_collection_id, error))
+            goto out;
 
-    if (pull_data->summary)
-      {
-        additional_metadata = g_variant_get_child_value (pull_data->summary, 1);
+          refs = g_variant_get_child_value (pull_data->summary, 0);
+          for (i = 0, n = g_variant_n_children (refs); i < n; i++)
+            {
+              const char *refname;
+              g_autoptr(GVariant) ref = g_variant_get_child_value (refs, i);
 
-        if (!g_variant_lookup (additional_metadata, OSTREE_SUMMARY_COLLECTION_ID, "&s", &main_collection_id))
-          main_collection_id = NULL;
-        else if (!ostree_validate_collection_id (main_collection_id, error))
-          goto out;
+              g_variant_get_child (ref, 0, "&s", &refname);
 
-        refs = g_variant_get_child_value (pull_data->summary, 0);
-        for (i = 0, n = g_variant_n_children (refs); i < n; i++)
-          {
-            const char *refname;
-            g_autoptr(GVariant) ref = g_variant_get_child_value (refs, i);
+              if (!ostree_validate_rev (refname, error))
+                goto out;
 
-            g_variant_get_child (ref, 0, "&s", &refname);
+              if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set)
+                {
+                  g_hash_table_insert (requested_refs_to_fetch,
+                                       ostree_collection_ref_new (main_collection_id, refname), NULL);
+                }
+            }
 
-            if (!ostree_validate_rev (refname, error))
-              goto out;
+          g_autoptr(GVariant) collection_map = NULL;
+          collection_map = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_COLLECTION_MAP, G_VARIANT_TYPE ("a{sa(s(taya{sv}))}"));
+          if (collection_map != NULL)
+            {
+              GVariantIter collection_map_iter;
+              const char *collection_id;
+              g_autoptr(GVariant) collection_refs = NULL;
 
-            if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set)
-              {
-                g_hash_table_insert (requested_refs_to_fetch,
-                                     ostree_collection_ref_new (main_collection_id, refname), NULL);
-              }
-          }
+              g_variant_iter_init (&collection_map_iter, collection_map);
 
-        g_autoptr(GVariant) collection_map = NULL;
-        collection_map = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_COLLECTION_MAP, G_VARIANT_TYPE ("a{sa(s(taya{sv}))}"));
-        if (collection_map != NULL)
-          {
-            GVariantIter collection_map_iter;
-            const char *collection_id;
-            g_autoptr(GVariant) collection_refs = NULL;
+              while (g_variant_iter_loop (&collection_map_iter, "{&s@a(s(taya{sv}))}", &collection_id, &collection_refs))
+                {
+                  if (!ostree_validate_collection_id (collection_id, error))
+                    goto out;
 
-            g_variant_iter_init (&collection_map_iter, collection_map);
+                  for (i = 0, n = g_variant_n_children (collection_refs); i < n; i++)
+                    {
+                      const char *refname;
+                      g_autoptr(GVariant) ref = g_variant_get_child_value (collection_refs, i);
 
-            while (g_variant_iter_loop (&collection_map_iter, "{&s@a(s(taya{sv}))}", &collection_id, &collection_refs))
-              {
-                if (!ostree_validate_collection_id (collection_id, error))
-                  goto out;
+                      g_variant_get_child (ref, 0, "&s", &refname);
 
-                for (i = 0, n = g_variant_n_children (collection_refs); i < n; i++)
-                  {
-                    const char *refname;
-                    g_autoptr(GVariant) ref = g_variant_get_child_value (collection_refs, i);
+                      if (!ostree_validate_rev (refname, error))
+                        goto out;
 
-                    g_variant_get_child (ref, 0, "&s", &refname);
+                      if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set)
+                        {
+                          g_hash_table_insert (requested_refs_to_fetch,
+                                               ostree_collection_ref_new (collection_id, refname), NULL);
+                        }
+                    }
+                }
+            }
 
-                    if (!ostree_validate_rev (refname, error))
-                      goto out;
+          deltas = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}"));
+          pull_data->summary_has_deltas = deltas != NULL && g_variant_n_children (deltas) > 0;
+          if (!collect_available_deltas_for_pull (pull_data, deltas, error))
+            goto out;
 
-                    if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set)
-                      {
-                        g_hash_table_insert (requested_refs_to_fetch,
-                                             ostree_collection_ref_new (collection_id, refname), NULL);
-                      }
-                  }
-              }
-          }
+          g_variant_lookup (additional_metadata, OSTREE_SUMMARY_INDEXED_DELTAS, "b", &pull_data->has_indexed_deltas);
+        }
 
-        deltas = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}"));
-        pull_data->summary_has_deltas = deltas != NULL && g_variant_n_children (deltas) > 0;
-        if (!collect_available_deltas_for_pull (pull_data, deltas, error))
-          goto out;
-      }
+      if (pull_data->summary &&
+          g_variant_lookup (additional_metadata, OSTREE_SUMMARY_MODE, "s", &remote_mode_str) &&
+          g_variant_lookup (additional_metadata, OSTREE_SUMMARY_TOMBSTONE_COMMITS, "b", &tombstone_commits))
+        {
+          if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error))
+            goto out;
+          pull_data->has_tombstone_commits = tombstone_commits;
+          remote_mode_loaded = TRUE;
+        }
+    }
 
-    if (pull_data->summary &&
-        g_variant_lookup (additional_metadata, OSTREE_SUMMARY_MODE, "s", &remote_mode_str) &&
-        g_variant_lookup (additional_metadata, OSTREE_SUMMARY_TOMBSTONE_COMMITS, "b", &tombstone_commits))
-      {
-        if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error))
-          goto out;
-        pull_data->has_tombstone_commits = tombstone_commits;
-        remote_mode_loaded = TRUE;
-      }
-    else if (pull_data->remote_repo_local == NULL)
+  if (pull_data->require_static_deltas && !pull_data->has_indexed_deltas && !pull_data->summary_has_deltas)
+    {
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                   "Fetch configured to require static deltas, but no summary deltas or delta index found");
+      goto out;
+    }
+
+  if (remote_mode_loaded && pull_data->remote_repo_local == NULL)
       {
         /* Fall-back path which loads the necessary config from the remote’s
-         * `config` file. Doing so is deprecated since it means an
+         * `config` file (unless we already read it above). Doing so is deprecated since it means an
          * additional round trip to the remote for each pull. No need to do
          * it for local pulls. */
-        if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error))
+        if (remote_config == NULL &&
+            !load_remote_repo_config (pull_data, &remote_config, cancellable, error))
           goto out;
 
         if (!ot_keyfile_get_value_with_default (remote_config, "core", "mode", "bare",
@@ -4550,85 +4667,12 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
         remote_mode_loaded = TRUE;
       }
 
-    if (remote_mode_loaded && pull_data->remote_repo_local == NULL && pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE)
-      {
-        g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                     "Can't pull from archives with mode \"%s\"",
-                     remote_mode_str);
-        goto out;
-      }
-  }
-
-  if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set && !configured_branches)
-    {
-      if (!bytes_summary)
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                       "Fetching all refs was requested in mirror mode, but remote repository does not have a summary");
-          goto out;
-        }
-
-    }
-  else if (opt_collection_refs_set)
-    {
-      const gchar *collection_id, *ref_name, *checksum;
-
-      while (g_variant_iter_loop (collection_refs_iter, "(&s&s&s)", &collection_id, &ref_name, &checksum))
-        {
-          if (!ostree_validate_rev (ref_name, error))
-            goto out;
-          g_hash_table_insert (requested_refs_to_fetch,
-                               ostree_collection_ref_new (collection_id, ref_name),
-                               (*checksum != '\0') ? g_strdup (checksum) : NULL);
-        }
-    }
-  else if (refs_to_fetch != NULL)
-    {
-      char **strviter = refs_to_fetch;
-      char **commitid_strviter = override_commit_ids ?: NULL;
-
-      while (*strviter)
-        {
-          const char *branch = *strviter;
-
-          if (ostree_validate_checksum_string (branch, NULL))
-            {
-              char *key = g_strdup (branch);
-              g_hash_table_add (commits_to_fetch, key);
-            }
-          else
-            {
-              if (!ostree_validate_rev (branch, error))
-                goto out;
-              char *commitid = commitid_strviter ? g_strdup (*commitid_strviter) : NULL;
-              g_hash_table_insert (requested_refs_to_fetch,
-                                   ostree_collection_ref_new (NULL, branch), commitid);
-            }
-
-          strviter++;
-          if (commitid_strviter)
-            commitid_strviter++;
-        }
-    }
-  else
+  if (remote_mode_loaded && pull_data->remote_repo_local == NULL && pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE)
     {
-      char **branches_iter;
-
-      branches_iter = configured_branches;
-
-      if (!(branches_iter && *branches_iter))
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                       "No configured branches for remote %s", remote_name_or_baseurl);
-          goto out;
-        }
-      for (;branches_iter && *branches_iter; branches_iter++)
-        {
-          const char *branch = *branches_iter;
-
-          g_hash_table_insert (requested_refs_to_fetch,
-                               ostree_collection_ref_new (NULL, branch), NULL);
-        }
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                   "Can't pull from archives with mode \"%s\"",
+                   remote_mode_str);
+      goto out;
     }
 
   /* Resolve the checksum for each ref. This has to be done into a new hash table,