Merge branch 'jt/avoid-prefetch-when-able-in-diff'

"git diff" in a partial clone learned to avoid lazy loading blob
objects in more casese when they are not needed.

* jt/avoid-prefetch-when-able-in-diff:
  diff: restrict when prefetching occurs
  diff: refactor object read
  diff: make diff_populate_filespec_options struct
  promisor-remote: accept 0 as oid_nr in function

diff --combined diff.c
index 1010d806f50dac47927f6f70125647e9bf8276ef,b061f5bc70cc0715a043696462e0f656ebc21054..d1ad6a3c4ad0bc5049d1af26ca0ea33fc9f68a1e
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -573,7 -573,7 +573,7 @@@ static int fill_mmfile(struct repositor
                mf->size = 0;
                return 0;
        }
-       else if (diff_populate_filespec(r, one, 0))
+       else if (diff_populate_filespec(r, one, NULL))
                return -1;
  
        mf->ptr = one->data;
@@@ -585,9 -585,13 +585,13 @@@
  static unsigned long diff_filespec_size(struct repository *r,
                                        struct diff_filespec *one)
  {
+       struct diff_populate_filespec_options dpf_options = {
+               .check_size_only = 1,
+       };
+ 
        if (!DIFF_FILE_VALID(one))
                return 0;
-       diff_populate_filespec(r, one, CHECK_SIZE_ONLY);
+       diff_populate_filespec(r, one, &dpf_options);
        return one->size;
  }
  
@@@ -3020,6 -3024,9 +3024,9 @@@ static void show_dirstat(struct diff_op
                struct diff_filepair *p = q->queue[i];
                const char *name;
                unsigned long copied, added, damage;
+               struct diff_populate_filespec_options dpf_options = {
+                       .check_size_only = 1,
+               };
  
                name = p->two->path ? p->two->path : p->one->path;
  
@@@ -3047,19 -3054,19 +3054,19 @@@
                }
  
                if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
-                       diff_populate_filespec(options->repo, p->one, 0);
-                       diff_populate_filespec(options->repo, p->two, 0);
+                       diff_populate_filespec(options->repo, p->one, NULL);
+                       diff_populate_filespec(options->repo, p->two, NULL);
                        diffcore_count_changes(options->repo,
                                               p->one, p->two, NULL, NULL,
                                               &copied, &added);
                        diff_free_filespec_data(p->one);
                        diff_free_filespec_data(p->two);
                } else if (DIFF_FILE_VALID(p->one)) {
-                       diff_populate_filespec(options->repo, p->one, CHECK_SIZE_ONLY);
+                       diff_populate_filespec(options->repo, p->one, &dpf_options);
                        copied = added = 0;
                        diff_free_filespec_data(p->one);
                } else if (DIFF_FILE_VALID(p->two)) {
-                       diff_populate_filespec(options->repo, p->two, CHECK_SIZE_ONLY);
+                       diff_populate_filespec(options->repo, p->two, &dpf_options);
                        copied = 0;
                        added = p->two->size;
                        diff_free_filespec_data(p->two);
@@@ -3339,13 -3346,17 +3346,17 @@@ static void emit_binary_diff(struct dif
  int diff_filespec_is_binary(struct repository *r,
                            struct diff_filespec *one)
  {
+       struct diff_populate_filespec_options dpf_options = {
+               .check_binary = 1,
+       };
+ 
        if (one->is_binary == -1) {
                diff_filespec_load_driver(one, r->index);
                if (one->driver->binary != -1)
                        one->is_binary = one->driver->binary;
                else {
                        if (!one->data && DIFF_FILE_VALID(one))
-                               diff_populate_filespec(r, one, CHECK_BINARY);
+                               diff_populate_filespec(r, one, &dpf_options);
                        if (one->is_binary == -1 && one->data)
                                one->is_binary = buffer_is_binary(one->data,
                                                one->size);
@@@ -3677,8 -3688,8 +3688,8 @@@ static void builtin_diffstat(const cha
        }
  
        else if (complete_rewrite) {
-               diff_populate_filespec(o->repo, one, 0);
-               diff_populate_filespec(o->repo, two, 0);
+               diff_populate_filespec(o->repo, one, NULL);
+               diff_populate_filespec(o->repo, two, NULL);
                data->deleted = count_lines(one->data, one->size);
                data->added = count_lines(two->data, two->size);
        }
@@@ -3914,9 -3925,10 +3925,10 @@@ static int diff_populate_gitlink(struc
   */
  int diff_populate_filespec(struct repository *r,
                           struct diff_filespec *s,
-                          unsigned int flags)
+                          const struct diff_populate_filespec_options *options)
  {
-       int size_only = flags & CHECK_SIZE_ONLY;
+       int size_only = options ? options->check_size_only : 0;
+       int check_binary = options ? options->check_binary : 0;
        int err = 0;
        int conv_flags = global_conv_flags_eol;
        /*
@@@ -3986,7 -3998,7 +3998,7 @@@
                 * opening the file and inspecting the contents, this
                 * is probably fine.
                 */
-               if ((flags & CHECK_BINARY) &&
+               if (check_binary &&
                    s->size > big_file_threshold && s->is_binary == -1) {
                        s->is_binary = 1;
                        return 0;
@@@ -4011,12 -4023,30 +4023,30 @@@
                }
        }
        else {
-               enum object_type type;
-               if (size_only || (flags & CHECK_BINARY)) {
-                       type = oid_object_info(r, &s->oid, &s->size);
-                       if (type < 0)
-                               die("unable to read %s",
-                                   oid_to_hex(&s->oid));
+               struct object_info info = {
+                       .sizep = &s->size
+               };
+ 
+               if (!(size_only || check_binary))
+                       /*
+                        * Set contentp, since there is no chance that merely
+                        * the size is sufficient.
+                        */
+                       info.contentp = &s->data;
+ 
+               if (options && options->missing_object_cb) {
+                       if (!oid_object_info_extended(r, &s->oid, &info,
+                                                     OBJECT_INFO_LOOKUP_REPLACE |
+                                                     OBJECT_INFO_SKIP_FETCH_OBJECT))
+                               goto object_read;
+                       options->missing_object_cb(options->missing_object_data);
+               }
+               if (oid_object_info_extended(r, &s->oid, &info,
+                                            OBJECT_INFO_LOOKUP_REPLACE))
+                       die("unable to read %s", oid_to_hex(&s->oid));
+ 
+ object_read:
+               if (size_only || check_binary) {
                        if (size_only)
                                return 0;
                        if (s->size > big_file_threshold && s->is_binary == -1) {
@@@ -4024,9 -4054,12 +4054,12 @@@
                                return 0;
                        }
                }
-               s->data = repo_read_object_file(r, &s->oid, &type, &s->size);
-               if (!s->data)
-                       die("unable to read %s", oid_to_hex(&s->oid));
+               if (!info.contentp) {
+                       info.contentp = &s->data;
+                       if (oid_object_info_extended(r, &s->oid, &info,
+                                                    OBJECT_INFO_LOOKUP_REPLACE))
+                               die("unable to read %s", oid_to_hex(&s->oid));
+               }
                s->should_free = 1;
        }
        return 0;
@@@ -4062,9 -4095,6 +4095,9 @@@ static void prep_temp_blob(struct index
        struct strbuf tempfile = STRBUF_INIT;
        char *path_dup = xstrdup(path);
        const char *base = basename(path_dup);
 +      struct checkout_metadata meta;
 +
 +      init_checkout_metadata(&meta, NULL, NULL, oid);
  
        /* Generate "XXXXXX_basename.ext" */
        strbuf_addstr(&tempfile, "XXXXXX_");
@@@ -4074,7 -4104,7 +4107,7 @@@
        if (!temp->tempfile)
                die_errno("unable to create temp-file");
        if (convert_to_working_tree(istate, path,
 -                      (const char *)blob, (size_t)size, &buf)) {
 +                      (const char *)blob, (size_t)size, &buf, &meta)) {
                blob = buf.buf;
                size = buf.len;
        }
@@@ -4144,7 -4174,7 +4177,7 @@@ static struct diff_tempfile *prepare_te
                return temp;
        }
        else {
-               if (diff_populate_filespec(r, one, 0))
+               if (diff_populate_filespec(r, one, NULL))
                        die("cannot read data blob for %s", one->path);
                prep_temp_blob(r->index, name, temp,
                               one->data, one->size,
@@@ -6410,9 -6440,9 +6443,9 @@@ static int diff_filespec_is_identical(s
  {
        if (S_ISGITLINK(one->mode))
                return 0;
-       if (diff_populate_filespec(r, one, 0))
+       if (diff_populate_filespec(r, one, NULL))
                return 0;
-       if (diff_populate_filespec(r, two, 0))
+       if (diff_populate_filespec(r, two, NULL))
                return 0;
        return !memcmp(one->data, two->data, one->size);
  }
@@@ -6420,6 -6450,12 +6453,12 @@@
  static int diff_filespec_check_stat_unmatch(struct repository *r,
                                            struct diff_filepair *p)
  {
+       struct diff_populate_filespec_options dpf_options = {
+               .check_size_only = 1,
+               .missing_object_cb = diff_queued_diff_prefetch,
+               .missing_object_data = r,
+       };
+ 
        if (p->done_skip_stat_unmatch)
                return p->skip_stat_unmatch_result;
  
@@@ -6442,8 -6478,8 +6481,8 @@@
            !DIFF_FILE_VALID(p->two) ||
            (p->one->oid_valid && p->two->oid_valid) ||
            (p->one->mode != p->two->mode) ||
-           diff_populate_filespec(r, p->one, CHECK_SIZE_ONLY) ||
-           diff_populate_filespec(r, p->two, CHECK_SIZE_ONLY) ||
+           diff_populate_filespec(r, p->one, &dpf_options) ||
+           diff_populate_filespec(r, p->two, &dpf_options) ||
            (p->one->size != p->two->size) ||
            !diff_filespec_is_identical(r, p->one, p->two)) /* (2) */
                p->skip_stat_unmatch_result = 1;
@@@ -6494,9 -6530,9 +6533,9 @@@ void diffcore_fix_diff_index(void
        QSORT(q->queue, q->nr, diffnamecmp);
  }
  
- static void add_if_missing(struct repository *r,
-                          struct oid_array *to_fetch,
-                          const struct diff_filespec *filespec)
+ void diff_add_if_missing(struct repository *r,
+                        struct oid_array *to_fetch,
+                        const struct diff_filespec *filespec)
  {
        if (filespec && filespec->oid_valid &&
            !S_ISGITLINK(filespec->mode) &&
@@@ -6505,30 -6541,48 +6544,48 @@@
                oid_array_append(to_fetch, &filespec->oid);
  }
  
- void diffcore_std(struct diff_options *options)
+ void diff_queued_diff_prefetch(void *repository)
  {
-       if (options->repo == the_repository && has_promisor_remote()) {
-               /*
-                * Prefetch the diff pairs that are about to be flushed.
-                */
-               int i;
-               struct diff_queue_struct *q = &diff_queued_diff;
-               struct oid_array to_fetch = OID_ARRAY_INIT;
+       struct repository *repo = repository;
+       int i;
+       struct diff_queue_struct *q = &diff_queued_diff;
+       struct oid_array to_fetch = OID_ARRAY_INIT;
  
-               for (i = 0; i < q->nr; i++) {
-                       struct diff_filepair *p = q->queue[i];
-                       add_if_missing(options->repo, &to_fetch, p->one);
-                       add_if_missing(options->repo, &to_fetch, p->two);
-               }
-               if (to_fetch.nr)
-                       /*
-                        * NEEDSWORK: Consider deduplicating the OIDs sent.
-                        */
-                       promisor_remote_get_direct(options->repo,
-                                                  to_fetch.oid, to_fetch.nr);
-               oid_array_clear(&to_fetch);
+       for (i = 0; i < q->nr; i++) {
+               struct diff_filepair *p = q->queue[i];
+               diff_add_if_missing(repo, &to_fetch, p->one);
+               diff_add_if_missing(repo, &to_fetch, p->two);
        }
  
+       /*
+        * NEEDSWORK: Consider deduplicating the OIDs sent.
+        */
+       promisor_remote_get_direct(repo, to_fetch.oid, to_fetch.nr);
+ 
+       oid_array_clear(&to_fetch);
+ }
+ 
+ void diffcore_std(struct diff_options *options)
+ {
+       int output_formats_to_prefetch = DIFF_FORMAT_DIFFSTAT |
+               DIFF_FORMAT_NUMSTAT |
+               DIFF_FORMAT_PATCH |
+               DIFF_FORMAT_SHORTSTAT |
+               DIFF_FORMAT_DIRSTAT;
+ 
+       /*
+        * Check if the user requested a blob-data-requiring diff output and/or
+        * break-rewrite detection (which requires blob data). If yes, prefetch
+        * the diff pairs.
+        *
+        * If no prefetching occurs, diffcore_rename() will prefetch if it
+        * decides that it needs inexact rename detection.
+        */
+       if (options->repo == the_repository && has_promisor_remote() &&
+           (options->output_format & output_formats_to_prefetch ||
+            options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
+               diff_queued_diff_prefetch(options->repo);
+ 
        /* NOTE please keep the following in sync with diff_tree_combined() */
        if (options->skip_stat_unmatch)
                diffcore_skip_stat_unmatch(options);
@@@ -6774,7 -6828,7 +6831,7 @@@ size_t fill_textconv(struct repository 
                        *outbuf = "";
                        return 0;
                }
-               if (diff_populate_filespec(r, df, 0))
+               if (diff_populate_filespec(r, df, NULL))
                        die("unable to read files to diff");
                *outbuf = df->data;
                return df->size;

diff --combined promisor-remote.c
index ff8721fd56680dcb3b92996ae7ea5e05655b526a,2155dfe657e87cf6b9d3832a4efe86b6f9f7fa23..baaea12fd69b3b2c86f9fe5308a06c65e2eff3eb
--- 1/promisor-remote.c
--- 2/promisor-remote.c
+++ b/promisor-remote.c
@@@ -101,7 -101,7 +101,7 @@@ static void promisor_remote_move_to_tai
  static int promisor_remote_config(const char *var, const char *value, void *data)
  {
        const char *name;
 -      int namelen;
 +      size_t namelen;
        const char *subkey;
  
        if (!strcmp(var, "core.partialclonefilter"))
@@@ -241,6 -241,9 +241,9 @@@ int promisor_remote_get_direct(struct r
        int to_free = 0;
        int res = -1;
  
+       if (oid_nr == 0)
+               return 0;
+ 
        promisor_remote_init();
  
        for (r = promisors; r; r = r->next) {

diff --combined unpack-trees.c
index f618a644efa0f942619d0ed71c8ff4d68f11a4de,01c914912102e115503ac1670e594ad8f805aab9..4c3191b9473c3ec9ea48b8a584d631f8c7c27511
--- 1/unpack-trees.c
--- 2/unpack-trees.c
+++ b/unpack-trees.c
@@@ -371,7 -371,6 +371,7 @@@ static int check_updates(struct unpack_
        state.quiet = 1;
        state.refresh_cache = 1;
        state.istate = index;
 +      clone_checkout_metadata(&state.meta, &o->meta, NULL);
  
        if (!o->update || o->dry_run) {
                remove_marked_cache_entries(index, 0);
@@@ -423,9 -422,8 +423,8 @@@
                                continue;
                        oid_array_append(&to_fetch, &ce->oid);
                }
-               if (to_fetch.nr)
-                       promisor_remote_get_direct(the_repository,
-                                                  to_fetch.oid, to_fetch.nr);
+               promisor_remote_get_direct(the_repository,
+                                          to_fetch.oid, to_fetch.nr);
                oid_array_clear(&to_fetch);
        }
        for (i = 0; i < index->cache_nr; i++) {
@@@ -1816,6 -1814,9 +1815,6 @@@ static void invalidate_ce_path(const st
  /*
   * Check that checking out ce->sha1 in subdir ce->name is not
   * going to overwrite any working files.
 - *
 - * Currently, git does not checkout subprojects during a superproject
 - * checkout, so it is not going to overwrite anything.
   */
  static int verify_clean_submodule(const char *old_sha1,
                                  const struct cache_entry *ce,
@@@ -2065,7 -2066,7 +2064,7 @@@ static int merged_entry(const struct ca
                }
                invalidate_ce_path(merge, o);
  
 -              if (submodule_from_ce(ce)) {
 +              if (submodule_from_ce(ce) && file_exists(ce->name)) {
                        int ret = check_submodule_move_head(ce, NULL,
                                                            oid_to_hex(&ce->oid),
                                                            o);
@@@ -2094,7 -2095,7 +2093,7 @@@
                        invalidate_ce_path(old, o);
                }
  
 -              if (submodule_from_ce(ce)) {
 +              if (submodule_from_ce(ce) && file_exists(ce->name)) {
                        int ret = check_submodule_move_head(ce, oid_to_hex(&old->oid),
                                                            oid_to_hex(&ce->oid),
                                                            o);