Merge branch 'jt/lazy-object-fetch-fix'
authorJunio C Hamano <gitster@pobox.com>
Mon, 24 Sep 2018 17:30:52 +0000 (10:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 24 Sep 2018 17:30:52 +0000 (10:30 -0700)
The code to backfill objects in lazily cloned repository did not
work correctly, which has been corrected.

* jt/lazy-object-fetch-fix:
fetch-object: set exact_oid when fetching
fetch-object: unify fetch_object[s] functions

1  2 
sha1-file.c
t/t0410-partial-clone.sh
unpack-trees.c
diff --combined sha1-file.c
index d85f4e93e1feb6843d61517b90acd8135ec526f7,2edf4564f6c12f33c59a101470cd2964774fcc34..a4367b8f044c6e9254d2a253149187bdb2be6dd2
@@@ -149,10 -149,10 +149,10 @@@ static struct cached_object *find_cache
        struct cached_object *co = cached_objects;
  
        for (i = 0; i < cached_object_nr; i++, co++) {
 -              if (!oidcmp(&co->oid, oid))
 +              if (oideq(&co->oid, oid))
                        return co;
        }
 -      if (!oidcmp(oid, the_hash_algo->empty_tree))
 +      if (oideq(oid, the_hash_algo->empty_tree))
                return &empty_tree;
        return NULL;
  }
@@@ -825,7 -825,7 +825,7 @@@ int check_object_signature(const struc
  
        if (map) {
                hash_object_file(map, size, type, &real_oid);
 -              return oidcmp(oid, &real_oid) ? -1 : 0;
 +              return !oideq(oid, &real_oid) ? -1 : 0;
        }
  
        st = open_istream(oid, &obj_type, &size, NULL);
        }
        the_hash_algo->final_fn(real_oid.hash, &c);
        close_istream(st);
 -      return oidcmp(oid, &real_oid) ? -1 : 0;
 +      return !oideq(oid, &real_oid) ? -1 : 0;
  }
  
  int git_open_cloexec(const char *name, int flags)
@@@ -1317,7 -1317,7 +1317,7 @@@ int oid_object_info_extended(struct rep
                         * TODO Pass a repository struct through fetch_object,
                         * such that arbitrary repositories work.
                         */
-                       fetch_object(repository_format_partial_clone, real->hash);
+                       fetch_objects(repository_format_partial_clone, real, 1);
                        already_retried = 1;
                        continue;
                }
@@@ -1671,7 -1671,7 +1671,7 @@@ static int write_loose_object(const str
                die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
                    ret);
        the_hash_algo->final_fn(parano_oid.hash, &c);
 -      if (oidcmp(oid, &parano_oid) != 0)
 +      if (!oideq(oid, &parano_oid))
                die(_("confused by unstable object source data for %s"),
                    oid_to_hex(oid));
  
@@@ -2213,7 -2213,7 +2213,7 @@@ static int check_stream_sha1(git_zstrea
        }
  
        the_hash_algo->final_fn(real_sha1, &c);
 -      if (hashcmp(expected_sha1, real_sha1)) {
 +      if (!hasheq(expected_sha1, real_sha1)) {
                error(_("sha1 mismatch for %s (expected %s)"), path,
                      sha1_to_hex(expected_sha1));
                return -1;
diff --combined t/t0410-partial-clone.sh
index b35bc89f1e82cfbfdba25d638d7c56390e2cf595,0ab02c337da4d20c9a60cfacc8fc97258b6ab61a..cfd0655ea19b0caa8d55fd41b5b76abde4de1c8b
@@@ -170,6 -170,18 +170,18 @@@ test_expect_success 'fetching of missin
        git verify-pack --verbose "$IDX" | grep "$HASH"
  '
  
+ test_expect_success 'fetching of missing objects works with ref-in-want enabled' '
+       # ref-in-want requires protocol version 2
+       git -C server config protocol.version 2 &&
+       git -C server config uploadpack.allowrefinwant 1 &&
+       git -C repo config protocol.version 2 &&
+       rm -rf repo/.git/objects/* &&
+       rm -f trace &&
+       GIT_TRACE_PACKET="$(pwd)/trace" git -C repo cat-file -p "$HASH" &&
+       grep "git< fetch=.*ref-in-want" trace
+ '
  test_expect_success 'rev-list stops traversal at missing and promised commit' '
        rm -rf repo &&
        test_create_repo repo &&
  
        git -C repo config core.repositoryformatversion 1 &&
        git -C repo config extensions.partialclone "arbitrary string" &&
 -      git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
 +      GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
        grep $(git -C repo rev-parse bar) out &&
        ! grep $FOO out
  '
diff --combined unpack-trees.c
index 65f157b9de5c75809980c1722ac67081dc2f92a1,82a83dbc67ba8410e8745c0a508bf9d0dd7b6027..51bfac6aa0bea246cdf6415490654b7ca2e4b9c2
@@@ -336,46 -336,6 +336,46 @@@ static struct progress *get_progress(st
        return start_delayed_progress(_("Checking out files"), total);
  }
  
 +static void setup_collided_checkout_detection(struct checkout *state,
 +                                            struct index_state *index)
 +{
 +      int i;
 +
 +      state->clone = 1;
 +      for (i = 0; i < index->cache_nr; i++)
 +              index->cache[i]->ce_flags &= ~CE_MATCHED;
 +}
 +
 +static void report_collided_checkout(struct index_state *index)
 +{
 +      struct string_list list = STRING_LIST_INIT_NODUP;
 +      int i;
 +
 +      for (i = 0; i < index->cache_nr; i++) {
 +              struct cache_entry *ce = index->cache[i];
 +
 +              if (!(ce->ce_flags & CE_MATCHED))
 +                      continue;
 +
 +              string_list_append(&list, ce->name);
 +              ce->ce_flags &= ~CE_MATCHED;
 +      }
 +
 +      list.cmp = fspathcmp;
 +      string_list_sort(&list);
 +
 +      if (list.nr) {
 +              warning(_("the following paths have collided (e.g. case-sensitive paths\n"
 +                        "on a case-insensitive filesystem) and only one from the same\n"
 +                        "colliding group is in the working tree:\n"));
 +
 +              for (i = 0; i < list.nr; i++)
 +                      fprintf(stderr, "  '%s'\n", list.items[i].string);
 +      }
 +
 +      string_list_clear(&list, 0);
 +}
 +
  static int check_updates(struct unpack_trees_options *o)
  {
        unsigned cnt = 0;
        struct checkout state = CHECKOUT_INIT;
        int i;
  
 +      trace_performance_enter();
        state.force = 1;
        state.quiet = 1;
        state.refresh_cache = 1;
        state.istate = index;
  
 +      if (o->clone)
 +              setup_collided_checkout_detection(&state, index);
 +
        progress = get_progress(o);
  
        if (o->update)
                }
                if (to_fetch.nr)
                        fetch_objects(repository_format_partial_clone,
-                                     &to_fetch);
+                                     to_fetch.oid, to_fetch.nr);
                fetch_if_missing = fetch_if_missing_store;
                oid_array_clear(&to_fetch);
        }
        errs |= finish_delayed_checkout(&state);
        if (o->update)
                git_attr_set_direction(GIT_ATTR_CHECKIN);
 +
 +      if (o->clone)
 +              report_collided_checkout(index);
 +
 +      trace_performance_leave("check_updates");
        return errs != 0;
  }
  
@@@ -679,114 -630,7 +679,114 @@@ static int switch_cache_bottom(struct t
  
  static inline int are_same_oid(struct name_entry *name_j, struct name_entry *name_k)
  {
 -      return name_j->oid && name_k->oid && !oidcmp(name_j->oid, name_k->oid);
 +      return name_j->oid && name_k->oid && oideq(name_j->oid, name_k->oid);
 +}
 +
 +static int all_trees_same_as_cache_tree(int n, unsigned long dirmask,
 +                                      struct name_entry *names,
 +                                      struct traverse_info *info)
 +{
 +      struct unpack_trees_options *o = info->data;
 +      int i;
 +
 +      if (!o->merge || dirmask != ((1 << n) - 1))
 +              return 0;
 +
 +      for (i = 1; i < n; i++)
 +              if (!are_same_oid(names, names + i))
 +                      return 0;
 +
 +      return cache_tree_matches_traversal(o->src_index->cache_tree, names, info);
 +}
 +
 +static int index_pos_by_traverse_info(struct name_entry *names,
 +                                    struct traverse_info *info)
 +{
 +      struct unpack_trees_options *o = info->data;
 +      int len = traverse_path_len(info, names);
 +      char *name = xmalloc(len + 1 /* slash */ + 1 /* NUL */);
 +      int pos;
 +
 +      make_traverse_path(name, info, names);
 +      name[len++] = '/';
 +      name[len] = '\0';
 +      pos = index_name_pos(o->src_index, name, len);
 +      if (pos >= 0)
 +              BUG("This is a directory and should not exist in index");
 +      pos = -pos - 1;
 +      if (!starts_with(o->src_index->cache[pos]->name, name) ||
 +          (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name)))
 +              BUG("pos must point at the first entry in this directory");
 +      free(name);
 +      return pos;
 +}
 +
 +/*
 + * Fast path if we detect that all trees are the same as cache-tree at this
 + * path. We'll walk these trees in an iterative loop using cache-tree/index
 + * instead of ODB since we already know what these trees contain.
 + */
 +static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names,
 +                                struct name_entry *names,
 +                                struct traverse_info *info)
 +{
 +      struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
 +      struct unpack_trees_options *o = info->data;
 +      struct cache_entry *tree_ce = NULL;
 +      int ce_len = 0;
 +      int i, d;
 +
 +      if (!o->merge)
 +              BUG("We need cache-tree to do this optimization");
 +
 +      /*
 +       * Do what unpack_callback() and unpack_nondirectories() normally
 +       * do. But we walk all paths in an iterative loop instead.
 +       *
 +       * D/F conflicts and higher stage entries are not a concern
 +       * because cache-tree would be invalidated and we would never
 +       * get here in the first place.
 +       */
 +      for (i = 0; i < nr_entries; i++) {
 +              int new_ce_len, len, rc;
 +
 +              src[0] = o->src_index->cache[pos + i];
 +
 +              len = ce_namelen(src[0]);
 +              new_ce_len = cache_entry_size(len);
 +
 +              if (new_ce_len > ce_len) {
 +                      new_ce_len <<= 1;
 +                      tree_ce = xrealloc(tree_ce, new_ce_len);
 +                      memset(tree_ce, 0, new_ce_len);
 +                      ce_len = new_ce_len;
 +
 +                      tree_ce->ce_flags = create_ce_flags(0);
 +
 +                      for (d = 1; d <= nr_names; d++)
 +                              src[d] = tree_ce;
 +              }
 +
 +              tree_ce->ce_mode = src[0]->ce_mode;
 +              tree_ce->ce_namelen = len;
 +              oidcpy(&tree_ce->oid, &src[0]->oid);
 +              memcpy(tree_ce->name, src[0]->name, len + 1);
 +
 +              rc = call_unpack_fn((const struct cache_entry * const *)src, o);
 +              if (rc < 0) {
 +                      free(tree_ce);
 +                      return rc;
 +              }
 +
 +              mark_ce_used(src[0], o);
 +      }
 +      free(tree_ce);
 +      if (o->debug_unpack)
 +              printf("Unpacked %d entries from %s to %s using cache-tree\n",
 +                     nr_entries,
 +                     o->src_index->cache[pos]->name,
 +                     o->src_index->cache[pos + nr_entries - 1]->name);
 +      return 0;
  }
  
  static int traverse_trees_recursive(int n, unsigned long dirmask,
        void *buf[MAX_UNPACK_TREES];
        struct traverse_info newinfo;
        struct name_entry *p;
 +      int nr_entries;
 +
 +      nr_entries = all_trees_same_as_cache_tree(n, dirmask, names, info);
 +      if (nr_entries > 0) {
 +              struct unpack_trees_options *o = info->data;
 +              int pos = index_pos_by_traverse_info(names, info);
 +
 +              if (!o->merge || df_conflicts)
 +                      BUG("Wrong condition to get here buddy");
 +
 +              /*
 +               * All entries up to 'pos' must have been processed
 +               * (i.e. marked CE_UNPACKED) at this point. But to be safe,
 +               * save and restore cache_bottom anyway to not miss
 +               * unprocessed entries before 'pos'.
 +               */
 +              bottom = o->cache_bottom;
 +              ret = traverse_by_cache_tree(pos, nr_entries, n, names, info);
 +              o->cache_bottom = bottom;
 +              return ret;
 +      }
  
        p = names;
        while (!p->mode)
@@@ -987,11 -810,6 +987,11 @@@ static struct cache_entry *create_ce_en
        return ce;
  }
  
 +/*
 + * Note that traverse_by_cache_tree() duplicates some logic in this function
 + * without actually calling it. If you change the logic here you may need to
 + * check and change there as well.
 + */
  static int unpack_nondirectories(int n, unsigned long mask,
                                 unsigned long dirmask,
                                 struct cache_entry **src,
@@@ -1184,11 -1002,6 +1184,11 @@@ static void debug_unpack_callback(int n
                debug_name_entry(i, names + i);
  }
  
 +/*
 + * Note that traverse_by_cache_tree() duplicates some logic in this function
 + * without actually calling it. If you change the logic here you may need to
 + * check and change there as well.
 + */
  static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info)
  {
        struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
@@@ -1476,7 -1289,6 +1476,7 @@@ int unpack_trees(unsigned len, struct t
        if (len > MAX_UNPACK_TREES)
                die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
  
 +      trace_performance_enter();
        memset(&el, 0, sizeof(el));
        if (!core_apply_sparse_checkout || !o->update)
                o->skip_sparse_checkout = 1;
                        }
                }
  
 -              if (traverse_trees(len, t, &info) < 0)
 +              trace_performance_enter();
 +              ret = traverse_trees(len, t, &info);
 +              trace_performance_leave("traverse_trees");
 +              if (ret < 0)
                        goto return_failed;
        }
  
  
        ret = check_updates(o) ? (-2) : 0;
        if (o->dst_index) {
 +              move_index_extensions(&o->result, o->src_index);
                if (!ret) {
 +                      if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0))
 +                              cache_tree_verify(&o->result);
                        if (!o->result.cache_tree)
                                o->result.cache_tree = cache_tree();
                        if (!cache_tree_fully_valid(o->result.cache_tree))
                                                  WRITE_TREE_SILENT |
                                                  WRITE_TREE_REPAIR);
                }
 -              move_index_extensions(&o->result, o->src_index);
                discard_index(o->dst_index);
                *o->dst_index = o->result;
        } else {
        o->src_index = NULL;
  
  done:
 +      trace_performance_leave("unpack_trees");
        clear_exclude_list(&el);
        return ret;
  
@@@ -1678,7 -1484,7 +1678,7 @@@ static int same(const struct cache_entr
        if ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)
                return 0;
        return a->ce_mode == b->ce_mode &&
 -             !oidcmp(&a->oid, &b->oid);
 +             oideq(&a->oid, &b->oid);
  }
  
  
@@@ -1810,7 -1616,7 +1810,7 @@@ static int verify_clean_subdirectory(co
                 * If we are not going to update the submodule, then
                 * we don't care.
                 */
 -              if (!sub_head && !oidcmp(&oid, &ce->oid))
 +              if (!sub_head && oideq(&oid, &ce->oid))
                        return 0;
                return verify_clean_submodule(sub_head ? NULL : oid_to_hex(&oid),
                                              ce, error_type, o);
                        if (verify_uptodate(ce2, o))
                                return -1;
                        add_entry(o, ce2, CE_REMOVE, 0);
 +                      invalidate_ce_path(ce, o);
                        mark_ce_used(ce2, o);
                }
                cnt++;
@@@ -2098,8 -1903,6 +2098,8 @@@ static int keep_entry(const struct cach
                      struct unpack_trees_options *o)
  {
        add_entry(o, ce, 0, 0);
 +      if (ce_stage(ce))
 +              invalidate_ce_path(ce, o);
        return 1;
  }