pull: Fold together deltapart+fallback count for display

It's just simpler, and I'm not sure people are going to care
much about the difference by default.

We already folded in the fallback sizes into the download totals, so folding in
the count makes things consistent; previously you could see e.g.
`3/3 parts, 100MB/150MB` and be confused.

Closes: #678
Approved by: giuseppe

7 files changed, 63 insertions, 9 deletions

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 6246011f..3452fbe9 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -81,6 +81,7 @@ typedef struct {
   GHashTable       *scanned_metadata; /* Maps object name to itself */
   GHashTable       *requested_metadata; /* Maps object name to itself */
   GHashTable       *requested_content; /* Maps checksum to itself */
+  GHashTable       *requested_fallback_content; /* Maps checksum to itself */
   GHashTable       *pending_fetch_metadata; /* Map<ObjectName,FetchObjectData> */
   GHashTable       *pending_fetch_content; /* Map<checksum,FetchObjectData> */
   GHashTable       *pending_fetch_deltaparts; /* Set<FetchStaticDeltaData> */
@@ -98,6 +99,7 @@ typedef struct {
   gint              n_requested_metadata;
   gint              n_requested_content;
   guint             n_fetched_deltaparts;
+  guint             n_fetched_deltapart_fallbacks;
   guint             n_fetched_metadata;
   guint             n_fetched_content;
 
@@ -222,6 +224,8 @@ update_progress (gpointer user_data)
                                   pull_data->n_fetched_deltaparts);
   ostree_async_progress_set_uint (pull_data->progress, "total-delta-parts",
                                   pull_data->n_total_deltaparts);
+  ostree_async_progress_set_uint (pull_data->progress, "fetched-delta-fallbacks",
+                                  pull_data->n_fetched_deltapart_fallbacks);
   ostree_async_progress_set_uint (pull_data->progress, "total-delta-fallbacks",
                                   pull_data->n_total_delta_fallbacks);
   ostree_async_progress_set_uint64 (pull_data->progress, "fetched-delta-part-size",
@@ -785,6 +789,9 @@ content_fetch_on_write_complete (GObject        *object,
     }
 
   pull_data->n_fetched_content++;
+  /* Was this a delta fallback? */
+  if (g_hash_table_remove (pull_data->requested_fallback_content, expected_checksum))
+    pull_data->n_fetched_deltapart_fallbacks++;
  out:
   pull_data->n_outstanding_content_write_requests--;
   check_outstanding_requests_handle_error (pull_data, local_error);
@@ -1631,9 +1638,14 @@ process_one_static_delta_fallback (OtPullData   *pull_data,
         {
           if (!g_hash_table_lookup (pull_data->requested_content, checksum))
             {
+              /* Mark this as requested, like we do in the non-delta path */
               g_hash_table_add (pull_data->requested_content, checksum);
+              /* But also record it's a delta fallback object, so we can account
+               * for it as logically part of the delta fetch.
+               */
+              g_hash_table_add (pull_data->requested_fallback_content, g_strdup (checksum));
               enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_FILE, NULL, FALSE, FALSE);
-              checksum = NULL;  /* Transfer ownership */
+              checksum = NULL;  /* We transferred ownership to the requested_content hash */
             }
         }
     }
@@ -2604,6 +2616,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                        (GDestroyNotify)g_variant_unref, NULL);
   pull_data->requested_content = g_hash_table_new_full (g_str_hash, g_str_equal,
                                                         (GDestroyNotify)g_free, NULL);
+  pull_data->requested_fallback_content = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                                 (GDestroyNotify)g_free, NULL);
   pull_data->requested_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
                                                          (GDestroyNotify)g_variant_unref, NULL);
   pull_data->pending_fetch_content = g_hash_table_new_full (g_str_hash, g_str_equal,
@@ -3324,6 +3338,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   g_clear_pointer (&pull_data->scanned_metadata, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->summary_deltas_checksums, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref);
+  g_clear_pointer (&pull_data->requested_fallback_content, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_metadata, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->pending_fetch_content, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->pending_fetch_metadata, (GDestroyNotify) g_hash_table_unref);
diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c
index 1ab11ed7..21ed081d 100644
--- a/src/libostree/ostree-repo-static-delta-core.c
+++ b/src/libostree/ostree-repo-static-delta-core.c
@@ -937,7 +937,7 @@ _ostree_repo_static_delta_dump (OstreeRepo                    *self,
       {
         guint64 size, usize;
         g_autoptr(GVariant) checksum_v = NULL;
-        char checksum[65];
+        char checksum[OSTREE_SHA256_STRING_LEN+1];
         g_variant_get_child (fallback, i, "(y@aytt)", NULL, &checksum_v, &size, &usize);
         ostree_checksum_inplace_from_bytes (ostree_checksum_bytes_peek (checksum_v), checksum);
         size = maybe_swap_endian_u64 (swap_endian, size);
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 98abd926..dc0eb575 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3884,6 +3884,8 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
   guint n_scanned_metadata;
   guint fetched_delta_parts;
   guint total_delta_parts;
+  guint fetched_delta_part_fallbacks;
+  guint total_delta_part_fallbacks;
 
   buf = g_string_new ("");
 
@@ -3895,6 +3897,8 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
   n_scanned_metadata = ostree_async_progress_get_uint (progress, "scanned-metadata");
   fetched_delta_parts = ostree_async_progress_get_uint (progress, "fetched-delta-parts");
   total_delta_parts = ostree_async_progress_get_uint (progress, "total-delta-parts");
+  fetched_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "fetched-delta-fallbacks");
+  total_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "total-delta-fallbacks");
 
   if (status)
     {
@@ -3932,10 +3936,15 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
       if (total_delta_parts > 0)
         {
           guint64 fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
-          g_autofree char *formatted_fetched =
-            g_format_size (fetched_delta_part_size);
-          g_autofree char *formatted_total =
-            g_format_size (total_delta_part_size);
+          g_autofree char *formatted_fetched = NULL;
+          g_autofree char *formatted_total = NULL;
+
+          /* Here we merge together deltaparts + fallbacks to avoid bloating the text UI */
+          fetched_delta_parts += fetched_delta_part_fallbacks;
+          total_delta_parts += total_delta_part_fallbacks;
+
+          formatted_fetched = g_format_size (fetched_delta_part_size);
+          formatted_total = g_format_size (total_delta_part_size);
 
           if (bytes_sec > 0)
             {
diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index 7df48002..170a3a91 100644
--- a/src/ostree/ot-builtin-pull.c
+++ b/src/ostree/ot-builtin-pull.c
@@ -80,14 +80,24 @@ dry_run_console_progress_changed (OstreeAsyncProgress *progress,
                                   gpointer             user_data)
 {
   guint fetched_delta_parts, total_delta_parts;
+  guint fetched_delta_part_fallbacks, total_delta_part_fallbacks;
   guint64 fetched_delta_part_size, total_delta_part_size, total_delta_part_usize;
   GString *buf;
 
   g_assert (!printed_console_progress);
   printed_console_progress = TRUE;
 
+  /* Number of parts */
   fetched_delta_parts = ostree_async_progress_get_uint (progress, "fetched-delta-parts");
   total_delta_parts = ostree_async_progress_get_uint (progress, "total-delta-parts");
+  fetched_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "fetched-delta-fallbacks");
+  total_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "total-delta-fallbacks");
+  /* Fold the count of deltaparts + fallbacks for simplicity; if changing this,
+   * please change ostree_repo_pull_default_console_progress_changed() first.
+   */
+  fetched_delta_parts += fetched_delta_part_fallbacks;
+  total_delta_parts += total_delta_part_fallbacks;
+  /* Size variables */
   fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
   total_delta_part_size = ostree_async_progress_get_uint64 (progress, "total-delta-part-size");
   total_delta_part_usize = ostree_async_progress_get_uint64 (progress, "total-delta-part-usize");
diff --git a/tests/libtest.sh b/tests/libtest.sh
index 1c603d66..ce41668f 100755
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -313,6 +313,10 @@ setup_exampleos_repo() {
         nfiles=10
         while [ $nfiles -gt 0 ]; do
             echo file-$ndirs-$nfiles > f$ndirs-$nfiles
+            # Make an unreadable file to trigger https://github.com/ostreedev/ostree/pull/634
+            if [ $(($x % 10)) -eq 0 ]; then
+                chmod 0600 f$ndirs-$nfiles
+            fi
             nfiles=$((nfiles-1))
         done
         ndirs=$((ndirs-1))
diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 56b24a0c..693eac06 100644
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -144,7 +144,8 @@ cd ${test_tmpdir}
 repo_init
 ${CMD_PREFIX} ostree --repo=repo pull origin main@${prev_rev}
 ${CMD_PREFIX} ostree --repo=repo pull --dry-run --require-static-deltas origin main >dry-run-pull.txt
-assert_file_has_content dry-run-pull.txt 'Delta update: 0/1 parts'
+# Compression can vary, so we support 400-699
+assert_file_has_content dry-run-pull.txt 'Delta update: 0/1 parts, 0 bytes/[456][0-9][0-9] bytes, 455 bytes total uncompressed'
 rev=$(${CMD_PREFIX} ostree --repo=repo rev-parse origin:main)
 assert_streq "${prev_rev}" "${rev}"
 ${CMD_PREFIX} ostree --repo=repo fsck
diff --git a/tests/test-pull-many.sh b/tests/test-pull-many.sh
index f39c8e6e..73245a6b 100755
--- a/tests/test-pull-many.sh
+++ b/tests/test-pull-many.sh
@@ -23,15 +23,30 @@ set -euo pipefail
 
 setup_exampleos_repo
 
-echo '1..1'
+echo '1..3'
 
 cd ${test_tmpdir}
 set -x
 
 echo "$(date): Pulling content..."
 rev=$(${CMD_PREFIX} ostree --repo=ostree-srv/exampleos/repo rev-parse ${REF})
-${CMD_PREFIX} ostree --repo=repo pull --disable-static-deltas --mirror origin ${REF}
+${CMD_PREFIX} ostree --repo=repo pull --disable-static-deltas origin ${REF}
 ${CMD_PREFIX} ostree --repo=repo fsck
 assert_streq ${rev} $(${CMD_PREFIX} ostree --repo=repo rev-parse ${REF})
 
+echo "ok without deltas"
+
+previous=$(${CMD_PREFIX} ostree --repo=repo rev-parse ${rev}^)
+rm repo/refs/{heads,remotes}/* -rf
+${CMD_PREFIX} ostree --repo=repo prune --refs-only
+${CMD_PREFIX} ostree --repo=repo pull origin ${REF}@${previous}
+${CMD_PREFIX} ostree --repo=repo pull --dry-run --require-static-deltas origin ${REF} > output.txt
+assert_file_has_content output.txt 'Delta update: 0/1 parts, 0 bytes/1.[012] MB, 1.[345] MB total uncompressed'
+
+echo "ok delta dry-run"
+
+${CMD_PREFIX} ostree --repo=repo pull  --require-static-deltas origin ${REF}
+assert_streq $(${CMD_PREFIX} ostree --repo=repo rev-parse ${REF}) ${rev}
+${CMD_PREFIX} ostree --repo=repo fsck
+
 echo "ok"