Merge branch 'ds/commit-graph-on-fetch'

Regression fix.

* ds/commit-graph-on-fetch:
  commit-graph: fix writing first commit-graph during fetch
  t5510-fetch.sh: demonstrate fetch.writeCommitGraph bug

diff --combined commit-graph.c
index fc4a43b8d6eb112f1c845882b32411c37a402e3b,a23b9d95f7404f70139f0877ece7a5a4dffb4a98..0aea7b2ae5263899b2b93e5acbcd1f391814e0f3
--- 1/commit-graph.c
--- 2/commit-graph.c
+++ b/commit-graph.c
@@@ -41,6 -41,9 +41,9 @@@
  #define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
                        + GRAPH_FANOUT_SIZE + the_hash_algo->rawsz)
  
+ /* Remember to update object flag allocation in object.h */
+ #define REACHABLE       (1u<<15)
+ 
  char *get_commit_graph_filename(const char *obj_dir)
  {
        char *filename = xstrfmt("%s/info/commit-graph", obj_dir);
@@@ -434,7 -437,6 +437,7 @@@ static struct commit_graph *load_commit
  
        free(oids);
        fclose(fp);
 +      strbuf_release(&line);
  
        return graph_chain;
  }
@@@ -468,21 -470,14 +471,21 @@@ static int prepare_commit_graph(struct 
  {
        struct object_directory *odb;
  
 -      if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 -              die("dying as requested by the '%s' variable on commit-graph load!",
 -                  GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
 +      /*
 +       * This must come before the "already attempted?" check below, because
 +       * we want to disable even an already-loaded graph file.
 +       */
 +      if (r->commit_graph_disabled)
 +              return 0;
  
        if (r->objects->commit_graph_attempted)
                return !!r->objects->commit_graph;
        r->objects->commit_graph_attempted = 1;
  
 +      if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 +              die("dying as requested by the '%s' variable on commit-graph load!",
 +                  GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
 +
        prepare_repo_settings(r);
  
        if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
@@@ -790,8 -785,7 +793,8 @@@ struct write_commit_graph_context 
  
        unsigned append:1,
                 report_progress:1,
 -               split:1;
 +               split:1,
 +               check_oids:1;
  
        const struct split_commit_graph_opts *split_opts;
  };
@@@ -846,19 -840,12 +849,19 @@@ static void write_graph_chunk_data(stru
  
        while (list < last) {
                struct commit_list *parent;
 +              struct object_id *tree;
                int edge_value;
                uint32_t packedDate[2];
                display_progress(ctx->progress, ++ctx->progress_cnt);
  
 -              parse_commit_no_graph(*list);
 -              hashwrite(f, get_commit_tree_oid(*list)->hash, hash_len);
 +              if (parse_commit_no_graph(*list))
 +                      die(_("unable to parse commit %s"),
 +                              oid_to_hex(&(*list)->object.oid));
 +              tree = get_commit_tree_oid(*list);
 +              if (!tree)
 +                      die(_("unable to get tree for %s"),
 +                              oid_to_hex(&(*list)->object.oid));
 +              hashwrite(f, tree->hash, hash_len);
  
                parent = (*list)->parents;
  
@@@ -1030,11 -1017,11 +1033,11 @@@ static void add_missing_parents(struct 
  {
        struct commit_list *parent;
        for (parent = commit->parents; parent; parent = parent->next) {
-               if (!(parent->item->object.flags & UNINTERESTING)) {
+               if (!(parent->item->object.flags & REACHABLE)) {
                        ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
                        oidcpy(&ctx->oids.list[ctx->oids.nr], &(parent->item->object.oid));
                        ctx->oids.nr++;
-                       parent->item->object.flags |= UNINTERESTING;
+                       parent->item->object.flags |= REACHABLE;
                }
        }
  }
@@@ -1052,7 -1039,7 +1055,7 @@@ static void close_reachable(struct writ
                display_progress(ctx->progress, i + 1);
                commit = lookup_commit(ctx->r, &ctx->oids.list[i]);
                if (commit)
-                       commit->object.flags |= UNINTERESTING;
+                       commit->object.flags |= REACHABLE;
        }
        stop_progress(&ctx->progress);
  
@@@ -1064,7 -1051,7 +1067,7 @@@
        if (ctx->report_progress)
                ctx->progress = start_delayed_progress(
                                        _("Expanding reachable commits in commit graph"),
 -                                      ctx->oids.nr);
 +                                      0);
        for (i = 0; i < ctx->oids.nr; i++) {
                display_progress(ctx->progress, i + 1);
                commit = lookup_commit(ctx->r, &ctx->oids.list[i]);
@@@ -1089,7 -1076,7 +1092,7 @@@
                commit = lookup_commit(ctx->r, &ctx->oids.list[i]);
  
                if (commit)
-                       commit->object.flags &= ~UNINTERESTING;
+                       commit->object.flags &= ~REACHABLE;
        }
        stop_progress(&ctx->progress);
  }
@@@ -1149,8 -1136,7 +1152,8 @@@ static int add_ref_to_list(const char *
        return 0;
  }
  
 -int write_commit_graph_reachable(const char *obj_dir, unsigned int flags,
 +int write_commit_graph_reachable(const char *obj_dir,
 +                               enum commit_graph_write_flags flags,
                                 const struct split_commit_graph_opts *split_opts)
  {
        struct string_list list = STRING_LIST_INIT_DUP;
@@@ -1203,14 -1189,14 +1206,14 @@@ static int fill_oids_from_packs(struct 
        }
  
        stop_progress(&ctx->progress);
 -      strbuf_reset(&progress_title);
 +      strbuf_release(&progress_title);
        strbuf_release(&packname);
  
        return 0;
  }
  
 -static void fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
 -                                    struct string_list *commit_hex)
 +static int fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
 +                                   struct string_list *commit_hex)
  {
        uint32_t i;
        struct strbuf progress_title = STRBUF_INIT;
@@@ -1231,21 -1217,20 +1234,21 @@@
                struct commit *result;
  
                display_progress(ctx->progress, i + 1);
 -              if (commit_hex->items[i].string &&
 -                  parse_oid_hex(commit_hex->items[i].string, &oid, &end))
 -                      continue;
 -
 -              result = lookup_commit_reference_gently(ctx->r, &oid, 1);
 -
 -              if (result) {
 +              if (!parse_oid_hex(commit_hex->items[i].string, &oid, &end) &&
 +                  (result = lookup_commit_reference_gently(ctx->r, &oid, 1))) {
                        ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
                        oidcpy(&ctx->oids.list[ctx->oids.nr], &(result->object.oid));
                        ctx->oids.nr++;
 +              } else if (ctx->check_oids) {
 +                      error(_("invalid commit object id: %s"),
 +                          commit_hex->items[i].string);
 +                      return -1;
                }
        }
        stop_progress(&ctx->progress);
        strbuf_release(&progress_title);
 +
 +      return 0;
  }
  
  static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
@@@ -1293,6 -1278,7 +1296,6 @@@ static uint32_t count_distinct_commits(
  static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
  {
        uint32_t i;
 -      struct commit_list *parent;
  
        ctx->num_extra_edges = 0;
        if (ctx->report_progress)
@@@ -1300,8 -1286,7 +1303,8 @@@
                        _("Finding extra edges in commit graph"),
                        ctx->oids.nr);
        for (i = 0; i < ctx->oids.nr; i++) {
 -              int num_parents = 0;
 +              unsigned int num_parents;
 +
                display_progress(ctx->progress, i + 1);
                if (i > 0 && oideq(&ctx->oids.list[i - 1], &ctx->oids.list[i]))
                        continue;
@@@ -1315,7 -1300,10 +1318,7 @@@
  
                parse_commit_no_graph(ctx->commits.list[ctx->commits.nr]);
  
 -              for (parent = ctx->commits.list[ctx->commits.nr]->parents;
 -                   parent; parent = parent->next)
 -                      num_parents++;
 -
 +              num_parents = commit_list_count(ctx->commits.list[ctx->commits.nr]->parents);
                if (num_parents > 2)
                        ctx->num_extra_edges += num_parents - 1;
  
@@@ -1533,8 -1521,8 +1536,8 @@@ static int write_commit_graph_file(stru
  
  static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
  {
 -      struct commit_graph *g = ctx->r->objects->commit_graph;
 -      uint32_t num_commits = ctx->commits.nr;
 +      struct commit_graph *g;
 +      uint32_t num_commits;
        uint32_t i;
  
        int max_commits = 0;
@@@ -1546,7 -1534,6 +1549,7 @@@
        }
  
        g = ctx->r->objects->commit_graph;
 +      num_commits = ctx->commits.nr;
        ctx->num_commit_graphs_after = ctx->num_commit_graphs_before + 1;
  
        while (g && (g->num_commits <= size_mult * num_commits ||
@@@ -1628,7 -1615,8 +1631,7 @@@ static int commit_compare(const void *_
  
  static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)
  {
 -      uint32_t i, num_parents;
 -      struct commit_list *parent;
 +      uint32_t i;
  
        if (ctx->report_progress)
                ctx->progress = start_delayed_progress(
@@@ -1646,11 -1634,12 +1649,11 @@@
                        die(_("unexpected duplicate commit id %s"),
                            oid_to_hex(&ctx->commits.list[i]->object.oid));
                } else {
 -                      num_parents = 0;
 -                      for (parent = ctx->commits.list[i]->parents; parent; parent = parent->next)
 -                              num_parents++;
 +                      unsigned int num_parents;
  
 +                      num_parents = commit_list_count(ctx->commits.list[i]->parents);
                        if (num_parents > 2)
 -                              ctx->num_extra_edges += num_parents - 2;
 +                              ctx->num_extra_edges += num_parents - 1;
                }
        }
  
@@@ -1727,8 -1716,10 +1730,8 @@@ static void expire_commit_graphs(struc
        strbuf_addstr(&path, "/info/commit-graphs");
        dir = opendir(path.buf);
  
 -      if (!dir) {
 -              strbuf_release(&path);
 -              return;
 -      }
 +      if (!dir)
 +              goto out;
  
        strbuf_addch(&path, '/');
        dirnamelen = path.len;
@@@ -1757,15 -1748,12 +1760,15 @@@
                if (!found)
                        unlink(path.buf);
        }
 +
 +out:
 +      strbuf_release(&path);
  }
  
  int write_commit_graph(const char *obj_dir,
                       struct string_list *pack_indexes,
                       struct string_list *commit_hex,
 -                     unsigned int flags,
 +                     enum commit_graph_write_flags flags,
                       const struct split_commit_graph_opts *split_opts)
  {
        struct write_commit_graph_context *ctx;
@@@ -1786,10 -1774,9 +1789,10 @@@
        if (len && ctx->obj_dir[len - 1] == '/')
                ctx->obj_dir[len - 1] = 0;
  
 -      ctx->append = flags & COMMIT_GRAPH_APPEND ? 1 : 0;
 -      ctx->report_progress = flags & COMMIT_GRAPH_PROGRESS ? 1 : 0;
 -      ctx->split = flags & COMMIT_GRAPH_SPLIT ? 1 : 0;
 +      ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
 +      ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
 +      ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
 +      ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
        ctx->split_opts = split_opts;
  
        if (ctx->split) {
@@@ -1844,10 -1831,8 +1847,10 @@@
                        goto cleanup;
        }
  
 -      if (commit_hex)
 -              fill_oids_from_commit_hex(ctx, commit_hex);
 +      if (commit_hex) {
 +              if ((res = fill_oids_from_commit_hex(ctx, commit_hex)))
 +                      goto cleanup;
 +      }
  
        if (!pack_indexes && !commit_hex)
                fill_oids_from_all_packs(ctx);
@@@ -2002,10 -1987,8 +2005,10 @@@ int verify_commit_graph(struct reposito
        if (verify_commit_graph_error & ~VERIFY_COMMIT_GRAPH_ERROR_HASH)
                return verify_commit_graph_error;
  
 -      progress = start_progress(_("Verifying commits in commit graph"),
 -                                g->num_commits);
 +      if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
 +              progress = start_progress(_("Verifying commits in commit graph"),
 +                                      g->num_commits);
 +
        for (i = 0; i < g->num_commits; i++) {
                struct commit *graph_commit, *odb_commit;
                struct commit_list *graph_parents, *odb_parents;
@@@ -2113,8 -2096,3 +2116,8 @@@ void free_commit_graph(struct commit_gr
        free(g->filename);
        free(g);
  }
 +
 +void disable_commit_graph(struct repository *r)
 +{
 +      r->commit_graph_disabled = 1;
 +}

diff --combined t/t5510-fetch.sh
index ecabbe1616d8a5c597fc32a354b053bc38a40334,8248923df2cd878d0a8fb9771a60f5f93f7f6ae4..4b602826895e21154376391333c6517b4ec1ec54
--- 1/t/t5510-fetch.sh
--- 2/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@@ -583,6 -583,22 +583,22 @@@ test_expect_success 'fetch.writeCommitG
        )
  '
  
+ test_expect_success 'fetch.writeCommitGraph with submodules' '
+       git clone dups super &&
+       (
+               cd super &&
+               git submodule add "file://$TRASH_DIRECTORY/three" &&
+               git commit -m "add submodule"
+       ) &&
+       git clone "super" super-clone &&
+       (
+               cd super-clone &&
+               rm -rf .git/objects/info &&
+               git -c fetch.writeCommitGraph=true fetch origin &&
+               test_path_is_file .git/objects/info/commit-graphs/commit-graph-chain
+       )
+ '
+ 
  # configured prune tests
  
  set_config_tristate () {
@@@ -915,29 -931,6 +931,29 @@@ test_expect_success C_LOCALE_OUTPUT 'fe
        test_cmp expect actual
  '
  
 +test_expect_success '--no-show-forced-updates' '
 +      mkdir forced-updates &&
 +      (
 +              cd forced-updates &&
 +              git init &&
 +              test_commit 1 &&
 +              test_commit 2
 +      ) &&
 +      git clone forced-updates forced-update-clone &&
 +      git clone forced-updates no-forced-update-clone &&
 +      git -C forced-updates reset --hard HEAD~1 &&
 +      (
 +              cd forced-update-clone &&
 +              git fetch --show-forced-updates origin 2>output &&
 +              test_i18ngrep "(forced update)" output
 +      ) &&
 +      (
 +              cd no-forced-update-clone &&
 +              git fetch --no-show-forced-updates origin 2>output &&
 +              test_i18ngrep ! "(forced update)" output
 +      )
 +'
 +
  setup_negotiation_tip () {
        SERVER="$1"
        URL="$2"
@@@ -1014,7 -1007,27 +1030,7 @@@ test_expect_success '--negotiation-tip 
        check_negotiation_tip
  '
  
 -test_expect_success '--no-show-forced-updates' '
 -      mkdir forced-updates &&
 -      (
 -              cd forced-updates &&
 -              git init &&
 -              test_commit 1 &&
 -              test_commit 2
 -      ) &&
 -      git clone forced-updates forced-update-clone &&
 -      git clone forced-updates no-forced-update-clone &&
 -      git -C forced-updates reset --hard HEAD~1 &&
 -      (
 -              cd forced-update-clone &&
 -              git fetch --show-forced-updates origin 2>output &&
 -              test_i18ngrep "(forced update)" output
 -      ) &&
 -      (
 -              cd no-forced-update-clone &&
 -              git fetch --no-show-forced-updates origin 2>output &&
 -              ! test_i18ngrep "(forced update)" output
 -      )
 -'
 +# DO NOT add non-httpd-specific tests here, because the last part of this
 +# test script is only executed when httpd is available and enabled.
  
  test_done