Merge branch 'md/list-lazy-objects-fix'
authorJunio C Hamano <gitster@pobox.com>
Mon, 14 Jan 2019 23:29:28 +0000 (15:29 -0800)
committerJunio C Hamano <gitster@pobox.com>
Mon, 14 Jan 2019 23:29:28 +0000 (15:29 -0800)
"git rev-list --exclude-promisor-objects" had to take an object
that does not exist locally (and is lazily available) from the
command line without barfing, but the code dereferenced NULL.

* md/list-lazy-objects-fix:
list-objects.c: don't segfault for missing cmdline objects

1  2 
revision.c
t/t0410-partial-clone.sh
diff --combined revision.c
index 13e0519c0241635c0c1fd14a7ff12e9478c70bd4,07dc0e28b60ce0072a53fcfd86061016f5708bdd..293303b67d0d876b24e678e34afb1c56160bc128
@@@ -25,8 -25,6 +25,8 @@@
  #include "worktree.h"
  #include "argv-array.h"
  #include "commit-reach.h"
 +#include "commit-graph.h"
 +#include "prio-queue.h"
  
  volatile show_early_output_fn_t show_early_output;
  
@@@ -179,6 -177,7 +179,6 @@@ static void add_pending_object_with_pat
                strbuf_release(&buf);
                return; /* do not add the commit itself */
        }
 -      obj->flags |= USER_GIVEN;
        add_object_array_with_path(obj, name, &revs->pending, mode, path);
  }
  
@@@ -769,8 -768,8 +769,8 @@@ static void commit_list_insert_by_date_
                *cache = new_entry;
  }
  
 -static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 -                  struct commit_list **list, struct commit_list **cache_ptr)
 +static int process_parents(struct rev_info *revs, struct commit *commit,
 +                         struct commit_list **list, struct commit_list **cache_ptr)
  {
        struct commit_list *parent = commit->parents;
        unsigned left_flag;
                        if (p->object.flags & SEEN)
                                continue;
                        p->object.flags |= SEEN;
 -                      commit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);
 +                      if (list)
 +                              commit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);
                }
                return 0;
        }
                p->object.flags |= left_flag;
                if (!(p->object.flags & SEEN)) {
                        p->object.flags |= SEEN;
 -                      commit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);
 +                      if (list)
 +                              commit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);
                }
                if (revs->first_parent_only)
                        break;
@@@ -1094,7 -1091,7 +1094,7 @@@ static int limit_list(struct rev_info *
  
                if (revs->max_age != -1 && (commit->date < revs->max_age))
                        obj->flags |= UNINTERESTING;
 -              if (add_parents_to_list(revs, commit, &list, NULL) < 0)
 +              if (process_parents(revs, commit, &list, NULL) < 0)
                        return -1;
                if (obj->flags & UNINTERESTING) {
                        mark_parents_uninteresting(commit);
@@@ -1181,7 -1178,7 +1181,7 @@@ struct all_refs_cb 
        int warned_bad_reflog;
        struct rev_info *all_revs;
        const char *name_for_errormsg;
 -      struct ref_store *refs;
 +      struct worktree *wt;
  };
  
  int ref_excluded(struct string_list *ref_excludes, const char *path)
@@@ -1218,7 -1215,7 +1218,7 @@@ static void init_all_refs_cb(struct all
        cb->all_revs = revs;
        cb->all_flags = flags;
        revs->rev_input_given = 1;
 -      cb->refs = NULL;
 +      cb->wt = NULL;
  }
  
  void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@@ -1281,20 -1278,14 +1281,20 @@@ static int handle_one_reflog_ent(struc
        return 0;
  }
  
 -static int handle_one_reflog(const char *path, const struct object_id *oid,
 +static int handle_one_reflog(const char *refname_in_wt,
 +                           const struct object_id *oid,
                             int flag, void *cb_data)
  {
        struct all_refs_cb *cb = cb_data;
 +      struct strbuf refname = STRBUF_INIT;
 +
        cb->warned_bad_reflog = 0;
 -      cb->name_for_errormsg = path;
 -      refs_for_each_reflog_ent(cb->refs, path,
 +      strbuf_worktree_ref(cb->wt, &refname, refname_in_wt);
 +      cb->name_for_errormsg = refname.buf;
 +      refs_for_each_reflog_ent(get_main_ref_store(the_repository),
 +                               refname.buf,
                                 handle_one_reflog_ent, cb_data);
 +      strbuf_release(&refname);
        return 0;
  }
  
@@@ -1309,8 -1300,8 +1309,8 @@@ static void add_other_reflogs_to_pendin
                if (wt->is_current)
                        continue;
  
 -              cb->refs = get_worktree_ref_store(wt);
 -              refs_for_each_reflog(cb->refs,
 +              cb->wt = wt;
 +              refs_for_each_reflog(get_worktree_ref_store(wt),
                                     handle_one_reflog,
                                     cb);
        }
@@@ -1323,7 -1314,7 +1323,7 @@@ void add_reflogs_to_pending(struct rev_
  
        cb.all_revs = revs;
        cb.all_flags = flags;
 -      cb.refs = get_main_ref_store(revs->repo);
 +      cb.wt = NULL;
        for_each_reflog(handle_one_reflog, &cb);
  
        if (!revs->single_worktree)
  }
  
  static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
 -                         struct strbuf *path)
 +                         struct strbuf *path, unsigned int flags)
  {
        size_t baselen = path->len;
        int i;
  
        if (it->entry_count >= 0) {
                struct tree *tree = lookup_tree(revs->repo, &it->oid);
 +              tree->object.flags |= flags;
                add_pending_object_with_path(revs, &tree->object, "",
                                             040000, path->buf);
        }
        for (i = 0; i < it->subtree_nr; i++) {
                struct cache_tree_sub *sub = it->down[i];
                strbuf_addf(path, "%s%s", baselen ? "/" : "", sub->name);
 -              add_cache_tree(sub->cache_tree, revs, path);
 +              add_cache_tree(sub->cache_tree, revs, path, flags);
                strbuf_setlen(path, baselen);
        }
  
  }
  
  static void do_add_index_objects_to_pending(struct rev_info *revs,
 -                                          struct index_state *istate)
 +                                          struct index_state *istate,
 +                                          unsigned int flags)
  {
        int i;
  
                blob = lookup_blob(revs->repo, &ce->oid);
                if (!blob)
                        die("unable to add index blob to traversal");
 +              blob->object.flags |= flags;
                add_pending_object_with_path(revs, &blob->object, "",
                                             ce->ce_mode, ce->name);
        }
  
        if (istate->cache_tree) {
                struct strbuf path = STRBUF_INIT;
 -              add_cache_tree(istate->cache_tree, revs, &path);
 +              add_cache_tree(istate->cache_tree, revs, &path, flags);
                strbuf_release(&path);
        }
  }
@@@ -1385,7 -1373,7 +1385,7 @@@ void add_index_objects_to_pending(struc
        struct worktree **worktrees, **p;
  
        read_index(revs->repo->index);
 -      do_add_index_objects_to_pending(revs, revs->repo->index);
 +      do_add_index_objects_to_pending(revs, revs->repo->index, flags);
  
        if (revs->single_worktree)
                return;
                if (read_index_from(&istate,
                                    worktree_git_path(wt, "index"),
                                    get_worktree_git_dir(wt)) > 0)
 -                      do_add_index_objects_to_pending(revs, &istate);
 +                      do_add_index_objects_to_pending(revs, &istate, flags);
                discard_index(&istate);
        }
        free_worktrees(worktrees);
@@@ -1729,6 -1717,8 +1729,8 @@@ int handle_revision_arg(const char *arg
        if (!cant_be_filename)
                verify_non_filename(revs->prefix, arg);
        object = get_reference(revs, arg, &oid, flags ^ local_flags);
+       if (!object)
+               return revs->ignore_missing ? 0 : -1;
        add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
        add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);
        free(oc.path);
@@@ -2151,8 -2141,7 +2153,8 @@@ static int handle_revision_opt(struct r
                revs->limited = 1;
        } else if (!strcmp(arg, "--ignore-missing")) {
                revs->ignore_missing = 1;
 -      } else if (!strcmp(arg, "--exclude-promisor-objects")) {
 +      } else if (revs->allow_exclude_promisor_objects_opt &&
 +                 !strcmp(arg, "--exclude-promisor-objects")) {
                if (fetch_if_missing)
                        BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
                revs->exclude_promisor_objects = 1;
@@@ -2472,7 -2461,7 +2474,7 @@@ int setup_revisions(int argc, const cha
        if (revs->diffopt.objfind)
                revs->simplify_history = 0;
  
 -      if (revs->topo_order)
 +      if (revs->topo_order && !generation_numbers_enabled(the_repository))
                revs->limited = 1;
  
        if (revs->prune_data.nr) {
@@@ -2911,217 -2900,6 +2913,217 @@@ static int mark_uninteresting(const str
        return 0;
  }
  
 +define_commit_slab(indegree_slab, int);
 +define_commit_slab(author_date_slab, timestamp_t);
 +
 +struct topo_walk_info {
 +      uint32_t min_generation;
 +      struct prio_queue explore_queue;
 +      struct prio_queue indegree_queue;
 +      struct prio_queue topo_queue;
 +      struct indegree_slab indegree;
 +      struct author_date_slab author_date;
 +};
 +
 +static inline void test_flag_and_insert(struct prio_queue *q, struct commit *c, int flag)
 +{
 +      if (c->object.flags & flag)
 +              return;
 +
 +      c->object.flags |= flag;
 +      prio_queue_put(q, c);
 +}
 +
 +static void explore_walk_step(struct rev_info *revs)
 +{
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +      struct commit_list *p;
 +      struct commit *c = prio_queue_get(&info->explore_queue);
 +
 +      if (!c)
 +              return;
 +
 +      if (parse_commit_gently(c, 1) < 0)
 +              return;
 +
 +      if (revs->sort_order == REV_SORT_BY_AUTHOR_DATE)
 +              record_author_date(&info->author_date, c);
 +
 +      if (revs->max_age != -1 && (c->date < revs->max_age))
 +              c->object.flags |= UNINTERESTING;
 +
 +      if (process_parents(revs, c, NULL, NULL) < 0)
 +              return;
 +
 +      if (c->object.flags & UNINTERESTING)
 +              mark_parents_uninteresting(c);
 +
 +      for (p = c->parents; p; p = p->next)
 +              test_flag_and_insert(&info->explore_queue, p->item, TOPO_WALK_EXPLORED);
 +}
 +
 +static void explore_to_depth(struct rev_info *revs,
 +                           uint32_t gen_cutoff)
 +{
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +      struct commit *c;
 +      while ((c = prio_queue_peek(&info->explore_queue)) &&
 +             c->generation >= gen_cutoff)
 +              explore_walk_step(revs);
 +}
 +
 +static void indegree_walk_step(struct rev_info *revs)
 +{
 +      struct commit_list *p;
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +      struct commit *c = prio_queue_get(&info->indegree_queue);
 +
 +      if (!c)
 +              return;
 +
 +      if (parse_commit_gently(c, 1) < 0)
 +              return;
 +
 +      explore_to_depth(revs, c->generation);
 +
 +      for (p = c->parents; p; p = p->next) {
 +              struct commit *parent = p->item;
 +              int *pi = indegree_slab_at(&info->indegree, parent);
 +
 +              if (*pi)
 +                      (*pi)++;
 +              else
 +                      *pi = 2;
 +
 +              test_flag_and_insert(&info->indegree_queue, parent, TOPO_WALK_INDEGREE);
 +
 +              if (revs->first_parent_only)
 +                      return;
 +      }
 +}
 +
 +static void compute_indegrees_to_depth(struct rev_info *revs,
 +                                     uint32_t gen_cutoff)
 +{
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +      struct commit *c;
 +      while ((c = prio_queue_peek(&info->indegree_queue)) &&
 +             c->generation >= gen_cutoff)
 +              indegree_walk_step(revs);
 +}
 +
 +static void init_topo_walk(struct rev_info *revs)
 +{
 +      struct topo_walk_info *info;
 +      struct commit_list *list;
 +      revs->topo_walk_info = xmalloc(sizeof(struct topo_walk_info));
 +      info = revs->topo_walk_info;
 +      memset(info, 0, sizeof(struct topo_walk_info));
 +
 +      init_indegree_slab(&info->indegree);
 +      memset(&info->explore_queue, 0, sizeof(info->explore_queue));
 +      memset(&info->indegree_queue, 0, sizeof(info->indegree_queue));
 +      memset(&info->topo_queue, 0, sizeof(info->topo_queue));
 +
 +      switch (revs->sort_order) {
 +      default: /* REV_SORT_IN_GRAPH_ORDER */
 +              info->topo_queue.compare = NULL;
 +              break;
 +      case REV_SORT_BY_COMMIT_DATE:
 +              info->topo_queue.compare = compare_commits_by_commit_date;
 +              break;
 +      case REV_SORT_BY_AUTHOR_DATE:
 +              init_author_date_slab(&info->author_date);
 +              info->topo_queue.compare = compare_commits_by_author_date;
 +              info->topo_queue.cb_data = &info->author_date;
 +              break;
 +      }
 +
 +      info->explore_queue.compare = compare_commits_by_gen_then_commit_date;
 +      info->indegree_queue.compare = compare_commits_by_gen_then_commit_date;
 +
 +      info->min_generation = GENERATION_NUMBER_INFINITY;
 +      for (list = revs->commits; list; list = list->next) {
 +              struct commit *c = list->item;
 +
 +              if (parse_commit_gently(c, 1))
 +                      continue;
 +
 +              test_flag_and_insert(&info->explore_queue, c, TOPO_WALK_EXPLORED);
 +              test_flag_and_insert(&info->indegree_queue, c, TOPO_WALK_INDEGREE);
 +
 +              if (c->generation < info->min_generation)
 +                      info->min_generation = c->generation;
 +
 +              *(indegree_slab_at(&info->indegree, c)) = 1;
 +
 +              if (revs->sort_order == REV_SORT_BY_AUTHOR_DATE)
 +                      record_author_date(&info->author_date, c);
 +      }
 +      compute_indegrees_to_depth(revs, info->min_generation);
 +
 +      for (list = revs->commits; list; list = list->next) {
 +              struct commit *c = list->item;
 +
 +              if (*(indegree_slab_at(&info->indegree, c)) == 1)
 +                      prio_queue_put(&info->topo_queue, c);
 +      }
 +
 +      /*
 +       * This is unfortunate; the initial tips need to be shown
 +       * in the order given from the revision traversal machinery.
 +       */
 +      if (revs->sort_order == REV_SORT_IN_GRAPH_ORDER)
 +              prio_queue_reverse(&info->topo_queue);
 +}
 +
 +static struct commit *next_topo_commit(struct rev_info *revs)
 +{
 +      struct commit *c;
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +
 +      /* pop next off of topo_queue */
 +      c = prio_queue_get(&info->topo_queue);
 +
 +      if (c)
 +              *(indegree_slab_at(&info->indegree, c)) = 0;
 +
 +      return c;
 +}
 +
 +static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
 +{
 +      struct commit_list *p;
 +      struct topo_walk_info *info = revs->topo_walk_info;
 +      if (process_parents(revs, commit, NULL, NULL) < 0) {
 +              if (!revs->ignore_missing_links)
 +                      die("Failed to traverse parents of commit %s",
 +                          oid_to_hex(&commit->object.oid));
 +      }
 +
 +      for (p = commit->parents; p; p = p->next) {
 +              struct commit *parent = p->item;
 +              int *pi;
 +
 +              if (parse_commit_gently(parent, 1) < 0)
 +                      continue;
 +
 +              if (parent->generation < info->min_generation) {
 +                      info->min_generation = parent->generation;
 +                      compute_indegrees_to_depth(revs, info->min_generation);
 +              }
 +
 +              pi = indegree_slab_at(&info->indegree, parent);
 +
 +              (*pi)--;
 +              if (*pi == 1)
 +                      prio_queue_put(&info->topo_queue, parent);
 +
 +              if (revs->first_parent_only)
 +                      return;
 +      }
 +}
 +
  int prepare_revision_walk(struct rev_info *revs)
  {
        int i;
                commit_list_sort_by_date(&revs->commits);
        if (revs->no_walk)
                return 0;
 -      if (revs->limited)
 +      if (revs->limited) {
                if (limit_list(revs) < 0)
                        return -1;
 -      if (revs->topo_order)
 -              sort_in_topological_order(&revs->commits, revs->sort_order);
 +              if (revs->topo_order)
 +                      sort_in_topological_order(&revs->commits, revs->sort_order);
 +      } else if (revs->topo_order)
 +              init_topo_walk(revs);
        if (revs->line_level_traverse)
                line_log_filter(revs);
        if (revs->simplify_merges)
@@@ -3181,7 -2957,7 +3183,7 @@@ static enum rewrite_result rewrite_one(
        for (;;) {
                struct commit *p = *pp;
                if (!revs->limited)
 -                      if (add_parents_to_list(revs, p, &revs->commits, &cache) < 0)
 +                      if (process_parents(revs, p, &revs->commits, &cache) < 0)
                                return rewrite_one_error;
                if (p->object.flags & UNINTERESTING)
                        return rewrite_one_ok;
@@@ -3489,8 -3265,6 +3491,8 @@@ static struct commit *get_revision_1(st
  
                if (revs->reflog_info)
                        commit = next_reflog_entry(revs->reflog_info);
 +              else if (revs->topo_walk_info)
 +                      commit = next_topo_commit(revs);
                else
                        commit = pop_commit(&revs->commits);
  
  
                        if (revs->reflog_info)
                                try_to_simplify_commit(revs, commit);
 -                      else if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
 +                      else if (revs->topo_walk_info)
 +                              expand_topo_walk(revs, commit);
 +                      else if (process_parents(revs, commit, &revs->commits, NULL) < 0) {
                                if (!revs->ignore_missing_links)
                                        die("Failed to traverse parents of commit %s",
                                                oid_to_hex(&commit->object.oid));
diff --combined t/t0410-partial-clone.sh
index ba3887f178b03a71b1ee1d2149429e2ee76814a6,d446021bab237df9ad5403e5bdc61fc35cb7dbb9..169f7f10a7332c1e9c71c09165d90dd7b7377d9e
@@@ -239,51 -239,6 +239,51 @@@ test_expect_success 'rev-list stops tra
        ! grep $FOO out
  '
  
 +test_expect_success 'missing tree objects with --missing=allow-promisor and --exclude-promisor-objects' '
 +      rm -rf repo &&
 +      test_create_repo repo &&
 +      test_commit -C repo foo &&
 +      test_commit -C repo bar &&
 +      test_commit -C repo baz &&
 +
 +      promise_and_delete $(git -C repo rev-parse bar^{tree}) &&
 +      promise_and_delete $(git -C repo rev-parse foo^{tree}) &&
 +
 +      git -C repo config core.repositoryformatversion 1 &&
 +      git -C repo config extensions.partialclone "arbitrary string" &&
 +
 +      git -C repo rev-list --missing=allow-promisor --objects HEAD >objs 2>rev_list_err &&
 +      test_must_be_empty rev_list_err &&
 +      # 3 commits, 3 blobs, and 1 tree
 +      test_line_count = 7 objs &&
 +
 +      # Do the same for --exclude-promisor-objects, but with all trees gone.
 +      promise_and_delete $(git -C repo rev-parse baz^{tree}) &&
 +      git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs 2>rev_list_err &&
 +      test_must_be_empty rev_list_err &&
 +      # 3 commits, no blobs or trees
 +      test_line_count = 3 objs
 +'
 +
 +test_expect_success 'missing non-root tree object and rev-list' '
 +      rm -rf repo &&
 +      test_create_repo repo &&
 +      mkdir repo/dir &&
 +      echo foo >repo/dir/foo &&
 +      git -C repo add dir/foo &&
 +      git -C repo commit -m "commit dir/foo" &&
 +
 +      promise_and_delete $(git -C repo rev-parse HEAD:dir) &&
 +
 +      git -C repo config core.repositoryformatversion 1 &&
 +      git -C repo config extensions.partialclone "arbitrary string" &&
 +
 +      git -C repo rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err &&
 +      test_must_be_empty rev_list_err &&
 +      # 1 commit and 1 tree
 +      test_line_count = 2 objs
 +'
 +
  test_expect_success 'rev-list stops traversal at missing and promised tree' '
        rm -rf repo &&
        test_create_repo repo &&
@@@ -349,7 -304,7 +349,7 @@@ test_expect_success 'rev-list stops tra
        grep $(git -C repo rev-parse bar) out  # sanity check that some walking was done
  '
  
- test_expect_success 'rev-list accepts missing and promised objects on command line' '
+ test_expect_success 'rev-list dies for missing objects on cmd line' '
        rm -rf repo &&
        test_create_repo repo &&
        test_commit -C repo foo &&
  
        git -C repo config core.repositoryformatversion 1 &&
        git -C repo config extensions.partialclone "arbitrary string" &&
-       git -C repo rev-list --exclude-promisor-objects --objects "$COMMIT" "$TREE" "$BLOB"
+       for OBJ in "$COMMIT" "$TREE" "$BLOB"; do
+               test_must_fail git -C repo rev-list --objects \
+                       --exclude-promisor-objects "$OBJ" &&
+               test_must_fail git -C repo rev-list --objects-edge-aggressive \
+                       --exclude-promisor-objects "$OBJ" &&
+               # Do not die or crash when --ignore-missing is passed.
+               git -C repo rev-list --ignore-missing --objects \
+                       --exclude-promisor-objects "$OBJ" &&
+               git -C repo rev-list --ignore-missing --objects-edge-aggressive \
+                       --exclude-promisor-objects "$OBJ"
+       done
  '
  
  test_expect_success 'gc repacks promisor objects separately from non-promisor objects' '