]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'sg/commit-graph-progress-fix' into master
authorJunio C Hamano <gitster@pobox.com>
Wed, 15 Jul 2020 23:29:43 +0000 (16:29 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 15 Jul 2020 23:29:43 +0000 (16:29 -0700)
The code to produce progress output from "git commit-graph --write"
had a few breakages, which have been fixed.

* sg/commit-graph-progress-fix:
  commit-graph: fix "Writing out commit graph" progress counter
  commit-graph: fix progress of reachable commits

1  2 
commit-graph.c

diff --combined commit-graph.c
index 328ab06fd417e171945e104f2f90861958f784f0,65cf32637cb875616035ad1fb6bcbae67a36e300..1af68c297d2e267b45fc5a1e820b11936dff906f
@@@ -18,7 -18,6 +18,7 @@@
  #include "progress.h"
  #include "bloom.h"
  #include "commit-slab.h"
 +#include "shallow.h"
  
  void git_test_write_commit_graph_or_die(void)
  {
@@@ -87,69 -86,15 +87,69 @@@ static int commit_pos_cmp(const void *v
               commit_pos_at(&commit_pos, b);
  }
  
 +define_commit_slab(commit_graph_data_slab, struct commit_graph_data);
 +static struct commit_graph_data_slab commit_graph_data_slab =
 +      COMMIT_SLAB_INIT(1, commit_graph_data_slab);
 +
 +uint32_t commit_graph_position(const struct commit *c)
 +{
 +      struct commit_graph_data *data =
 +              commit_graph_data_slab_peek(&commit_graph_data_slab, c);
 +
 +      return data ? data->graph_pos : COMMIT_NOT_FROM_GRAPH;
 +}
 +
 +uint32_t commit_graph_generation(const struct commit *c)
 +{
 +      struct commit_graph_data *data =
 +              commit_graph_data_slab_peek(&commit_graph_data_slab, c);
 +
 +      if (!data)
 +              return GENERATION_NUMBER_INFINITY;
 +      else if (data->graph_pos == COMMIT_NOT_FROM_GRAPH)
 +              return GENERATION_NUMBER_INFINITY;
 +
 +      return data->generation;
 +}
 +
 +static struct commit_graph_data *commit_graph_data_at(const struct commit *c)
 +{
 +      unsigned int i, nth_slab;
 +      struct commit_graph_data *data =
 +              commit_graph_data_slab_peek(&commit_graph_data_slab, c);
 +
 +      if (data)
 +              return data;
 +
 +      nth_slab = c->index / commit_graph_data_slab.slab_size;
 +      data = commit_graph_data_slab_at(&commit_graph_data_slab, c);
 +
 +      /*
 +       * commit-slab initializes elements with zero, overwrite this with
 +       * COMMIT_NOT_FROM_GRAPH for graph_pos.
 +       *
 +       * We avoid initializing generation with checking if graph position
 +       * is not COMMIT_NOT_FROM_GRAPH.
 +       */
 +      for (i = 0; i < commit_graph_data_slab.slab_size; i++) {
 +              commit_graph_data_slab.slab[nth_slab][i].graph_pos =
 +                      COMMIT_NOT_FROM_GRAPH;
 +      }
 +
 +      return data;
 +}
 +
  static int commit_gen_cmp(const void *va, const void *vb)
  {
        const struct commit *a = *(const struct commit **)va;
        const struct commit *b = *(const struct commit **)vb;
  
 +      uint32_t generation_a = commit_graph_generation(a);
 +      uint32_t generation_b = commit_graph_generation(b);
        /* lower generation commits first */
 -      if (a->generation < b->generation)
 +      if (generation_a < generation_b)
                return -1;
 -      else if (a->generation > b->generation)
 +      else if (generation_a > generation_b)
                return 1;
  
        /* use date as a heuristic when generations are equal */
@@@ -203,8 -148,7 +203,8 @@@ static int commit_graph_compatible(stru
        }
  
        prepare_commit_graft(r);
 -      if (r->parsed_objects && r->parsed_objects->grafts_nr)
 +      if (r->parsed_objects &&
 +          (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent))
                return 0;
        if (is_repository_shallow(r))
                return 0;
@@@ -337,7 -281,8 +337,7 @@@ struct commit_graph *parse_commit_graph
                if (data + graph_size - chunk_lookup <
                    GRAPH_CHUNKLOOKUP_WIDTH) {
                        error(_("commit-graph chunk lookup table entry missing; file may be incomplete"));
 -                      free(graph);
 -                      return NULL;
 +                      goto free_and_return;
                }
  
                chunk_id = get_be32(chunk_lookup + 0);
                if (chunk_offset > graph_size - the_hash_algo->rawsz) {
                        error(_("commit-graph improper chunk offset %08x%08x"), (uint32_t)(chunk_offset >> 32),
                              (uint32_t)chunk_offset);
 -                      free(graph);
 -                      return NULL;
 +                      goto free_and_return;
                }
  
                switch (chunk_id) {
  
                if (chunk_repeated) {
                        error(_("commit-graph chunk id %08x appears multiple times"), chunk_id);
 -                      free(graph);
 -                      return NULL;
 +                      goto free_and_return;
                }
  
                if (last_chunk_id == GRAPH_CHUNKID_OIDLOOKUP)
                /* We need both the bloom chunks to exist together. Else ignore the data */
                graph->chunk_bloom_indexes = NULL;
                graph->chunk_bloom_data = NULL;
 -              graph->bloom_filter_settings = NULL;
 +              FREE_AND_NULL(graph->bloom_filter_settings);
        }
  
        hashcpy(graph->oid.hash, graph->data + graph->data_len - graph->hash_len);
  
 -      if (verify_commit_graph_lite(graph)) {
 -              free(graph);
 -              return NULL;
 -      }
 +      if (verify_commit_graph_lite(graph))
 +              goto free_and_return;
  
        return graph;
 +
 +free_and_return:
 +      free(graph->bloom_filter_settings);
 +      free(graph);
 +      return NULL;
  }
  
  static struct commit_graph *load_commit_graph_one(const char *graph_file,
@@@ -725,14 -669,13 +725,14 @@@ static struct commit_list **insert_pare
        c = lookup_commit(r, &oid);
        if (!c)
                die(_("could not find commit %s"), oid_to_hex(&oid));
 -      c->graph_pos = pos;
 +      commit_graph_data_at(c)->graph_pos = pos;
        return &commit_list_insert(c, pptr)->next;
  }
  
  static void fill_commit_graph_info(struct commit *item, struct commit_graph *g, uint32_t pos)
  {
        const unsigned char *commit_data;
 +      struct commit_graph_data *graph_data;
        uint32_t lex_index;
  
        while (pos < g->num_commits_in_base)
  
        lex_index = pos - g->num_commits_in_base;
        commit_data = g->chunk_commit_data + GRAPH_DATA_WIDTH * lex_index;
 -      item->graph_pos = pos;
 -      item->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
 +
 +      graph_data = commit_graph_data_at(item);
 +      graph_data->graph_pos = pos;
 +      graph_data->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
  }
  
  static inline void set_commit_tree(struct commit *c, struct tree *t)
@@@ -759,7 -700,6 +759,7 @@@ static int fill_commit_in_graph(struct 
        uint32_t *parent_data_ptr;
        uint64_t date_low, date_high;
        struct commit_list **pptr;
 +      struct commit_graph_data *graph_data;
        const unsigned char *commit_data;
        uint32_t lex_index;
  
         * Store the "full" position, but then use the
         * "local" position for the rest of the calculation.
         */
 -      item->graph_pos = pos;
 +      graph_data = commit_graph_data_at(item);
 +      graph_data->graph_pos = pos;
        lex_index = pos - g->num_commits_in_base;
  
        commit_data = g->chunk_commit_data + (g->hash_len + 16) * lex_index;
        date_low = get_be32(commit_data + g->hash_len + 12);
        item->date = (timestamp_t)((date_high << 32) | date_low);
  
 -      item->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
 +      graph_data->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
  
        pptr = &item->parents;
  
  
  static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uint32_t *pos)
  {
 -      if (item->graph_pos != COMMIT_NOT_FROM_GRAPH) {
 -              *pos = item->graph_pos;
 +      uint32_t graph_pos = commit_graph_position(item);
 +      if (graph_pos != COMMIT_NOT_FROM_GRAPH) {
 +              *pos = graph_pos;
                return 1;
        } else {
                struct commit_graph *cur_g = g;
@@@ -876,13 -814,12 +876,13 @@@ static struct tree *load_tree_for_commi
  {
        struct object_id oid;
        const unsigned char *commit_data;
 +      uint32_t graph_pos = commit_graph_position(c);
  
 -      while (c->graph_pos < g->num_commits_in_base)
 +      while (graph_pos < g->num_commits_in_base)
                g = g->base_graph;
  
        commit_data = g->chunk_commit_data +
 -                      GRAPH_DATA_WIDTH * (c->graph_pos - g->num_commits_in_base);
 +                      GRAPH_DATA_WIDTH * (graph_pos - g->num_commits_in_base);
  
        hashcpy(oid.hash, commit_data);
        set_commit_tree(c, lookup_tree(r, &oid));
@@@ -896,7 -833,7 +896,7 @@@ static struct tree *get_commit_tree_in_
  {
        if (c->maybe_tree)
                return c->maybe_tree;
 -      if (c->graph_pos == COMMIT_NOT_FROM_GRAPH)
 +      if (commit_graph_position(c) == COMMIT_NOT_FROM_GRAPH)
                BUG("get_commit_tree_in_graph_one called from non-commit-graph commit");
  
        return load_tree_for_commit(r, g, (struct commit *)c);
@@@ -1082,7 -1019,7 +1082,7 @@@ static void write_graph_chunk_data(stru
                else
                        packedDate[0] = 0;
  
 -              packedDate[0] |= htonl((*list)->generation << 2);
 +              packedDate[0] |= htonl(commit_graph_data_at(*list)->generation << 2);
  
                packedDate[1] = htonl((*list)->date);
                hashwrite(f, packedDate, 8);
@@@ -1149,23 -1086,14 +1149,14 @@@ static void write_graph_chunk_bloom_ind
        struct commit **list = ctx->commits.list;
        struct commit **last = ctx->commits.list + ctx->commits.nr;
        uint32_t cur_pos = 0;
-       struct progress *progress = NULL;
-       int i = 0;
-       if (ctx->report_progress)
-               progress = start_delayed_progress(
-                       _("Writing changed paths Bloom filters index"),
-                       ctx->commits.nr);
  
        while (list < last) {
                struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);
                cur_pos += filter->len;
-               display_progress(progress, ++i);
+               display_progress(ctx->progress, ++ctx->progress_cnt);
                hashwrite_be32(f, cur_pos);
                list++;
        }
-       stop_progress(&progress);
  }
  
  static void write_graph_chunk_bloom_data(struct hashfile *f,
  {
        struct commit **list = ctx->commits.list;
        struct commit **last = ctx->commits.list + ctx->commits.nr;
-       struct progress *progress = NULL;
-       int i = 0;
-       if (ctx->report_progress)
-               progress = start_delayed_progress(
-                       _("Writing changed paths Bloom filters data"),
-                       ctx->commits.nr);
  
        hashwrite_be32(f, settings->hash_version);
        hashwrite_be32(f, settings->num_hashes);
  
        while (list < last) {
                struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);
-               display_progress(progress, ++i);
+               display_progress(ctx->progress, ++ctx->progress_cnt);
                hashwrite(f, filter->data, filter->len * sizeof(unsigned char));
                list++;
        }
-       stop_progress(&progress);
  }
  
  static int oid_compare(const void *_a, const void *_b)
@@@ -1281,7 -1200,7 +1263,7 @@@ static void close_reachable(struct writ
                        continue;
                if (ctx->split) {
                        if ((!parse_commit(commit) &&
 -                           commit->graph_pos == COMMIT_NOT_FROM_GRAPH) ||
 +                           commit_graph_position(commit) == COMMIT_NOT_FROM_GRAPH) ||
                            flags == COMMIT_GRAPH_SPLIT_REPLACE)
                                add_missing_parents(ctx, commit);
                } else if (!parse_commit_no_graph(commit))
@@@ -1313,11 -1232,9 +1295,11 @@@ static void compute_generation_numbers(
                                        _("Computing commit graph generation numbers"),
                                        ctx->commits.nr);
        for (i = 0; i < ctx->commits.nr; i++) {
 +              uint32_t generation = commit_graph_data_at(ctx->commits.list[i])->generation;
 +
                display_progress(ctx->progress, i + 1);
 -              if (ctx->commits.list[i]->generation != GENERATION_NUMBER_INFINITY &&
 -                  ctx->commits.list[i]->generation != GENERATION_NUMBER_ZERO)
 +              if (generation != GENERATION_NUMBER_INFINITY &&
 +                  generation != GENERATION_NUMBER_ZERO)
                        continue;
  
                commit_list_insert(ctx->commits.list[i], &list);
                        uint32_t max_generation = 0;
  
                        for (parent = current->parents; parent; parent = parent->next) {
 -                              if (parent->item->generation == GENERATION_NUMBER_INFINITY ||
 -                                  parent->item->generation == GENERATION_NUMBER_ZERO) {
 +                              generation = commit_graph_data_at(parent->item)->generation;
 +
 +                              if (generation == GENERATION_NUMBER_INFINITY ||
 +                                  generation == GENERATION_NUMBER_ZERO) {
                                        all_parents_computed = 0;
                                        commit_list_insert(parent->item, &list);
                                        break;
 -                              } else if (parent->item->generation > max_generation) {
 -                                      max_generation = parent->item->generation;
 +                              } else if (generation > max_generation) {
 +                                      max_generation = generation;
                                }
                        }
  
                        if (all_parents_computed) {
 -                              current->generation = max_generation + 1;
 +                              struct commit_graph_data *data = commit_graph_data_at(current);
 +
 +                              data->generation = max_generation + 1;
                                pop_commit(&list);
  
 -                              if (current->generation > GENERATION_NUMBER_MAX)
 -                                      current->generation = GENERATION_NUMBER_MAX;
 +                              if (data->generation > GENERATION_NUMBER_MAX)
 +                                      data->generation = GENERATION_NUMBER_MAX;
                        }
                }
        }
@@@ -1423,12 -1336,13 +1405,13 @@@ int write_commit_graph_reachable(struc
                        _("Collecting referenced commits"), 0);
  
        for_each_ref(add_ref_to_set, &data);
+       stop_progress(&data.progress);
        result = write_commit_graph(odb, NULL, &commits,
                                    flags, split_opts);
  
        oidset_clear(&commits);
-       if (data.progress)
-               stop_progress(&data.progress);
        return result;
  }
  
@@@ -1526,7 -1440,7 +1509,7 @@@ static uint32_t count_distinct_commits(
                        if (ctx->split) {
                                struct commit *c = lookup_commit(ctx->r, &ctx->oids.list[i]);
  
 -                              if (!c || c->graph_pos != COMMIT_NOT_FROM_GRAPH)
 +                              if (!c || commit_graph_position(c) != COMMIT_NOT_FROM_GRAPH)
                                        continue;
                        }
  
@@@ -1560,7 -1474,7 +1543,7 @@@ static void copy_oids_to_commits(struc
                ctx->commits.list[ctx->commits.nr] = lookup_commit(ctx->r, &ctx->oids.list[i]);
  
                if (ctx->split && flags != COMMIT_GRAPH_SPLIT_REPLACE &&
 -                  ctx->commits.list[ctx->commits.nr]->graph_pos != COMMIT_NOT_FROM_GRAPH)
 +                  commit_graph_position(ctx->commits.list[ctx->commits.nr]) != COMMIT_NOT_FROM_GRAPH)
                        continue;
  
                if (ctx->split && flags == COMMIT_GRAPH_SPLIT_REPLACE)
@@@ -1638,8 -1552,7 +1621,8 @@@ static int write_commit_graph_file(stru
        if (ctx->split) {
                char *lock_name = get_chain_filename(ctx->odb);
  
 -              hold_lock_file_for_update(&lk, lock_name, LOCK_DIE_ON_ERROR);
 +              hold_lock_file_for_update_mode(&lk, lock_name,
 +                                             LOCK_DIE_ON_ERROR, 0444);
  
                fd = git_mkstemp_mode(ctx->graph_name, 0444);
                if (fd < 0) {
                        return -1;
                }
  
 +              if (adjust_shared_perm(ctx->graph_name)) {
 +                      error(_("unable to adjust shared permissions for '%s'"),
 +                            ctx->graph_name);
 +                      return -1;
 +              }
 +
                f = hashfd(fd, ctx->graph_name);
        } else {
 -              hold_lock_file_for_update(&lk, ctx->graph_name, LOCK_DIE_ON_ERROR);
 +              hold_lock_file_for_update_mode(&lk, ctx->graph_name,
 +                                             LOCK_DIE_ON_ERROR, 0444);
                fd = lk.tempfile->fd;
                f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);
        }
@@@ -2309,7 -2215,6 +2292,7 @@@ int verify_commit_graph(struct reposito
                struct commit *graph_commit, *odb_commit;
                struct commit_list *graph_parents, *odb_parents;
                uint32_t max_generation = 0;
 +              uint32_t generation;
  
                display_progress(progress, i + 1);
                hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
                                             oid_to_hex(&graph_parents->item->object.oid),
                                             oid_to_hex(&odb_parents->item->object.oid));
  
 -                      if (graph_parents->item->generation > max_generation)
 -                              max_generation = graph_parents->item->generation;
 +                      generation = commit_graph_generation(graph_parents->item);
 +                      if (generation > max_generation)
 +                              max_generation = generation;
  
                        graph_parents = graph_parents->next;
                        odb_parents = odb_parents->next;
                        graph_report(_("commit-graph parent list for commit %s terminates early"),
                                     oid_to_hex(&cur_oid));
  
 -              if (!graph_commit->generation) {
 +              if (!commit_graph_generation(graph_commit)) {
                        if (generation_zero == GENERATION_NUMBER_EXISTS)
                                graph_report(_("commit-graph has generation number zero for commit %s, but non-zero elsewhere"),
                                             oid_to_hex(&cur_oid));
                if (max_generation == GENERATION_NUMBER_MAX)
                        max_generation--;
  
 -              if (graph_commit->generation != max_generation + 1)
 +              generation = commit_graph_generation(graph_commit);
 +              if (generation != max_generation + 1)
                        graph_report(_("commit-graph generation for commit %s is %u != %u"),
                                     oid_to_hex(&cur_oid),
 -                                   graph_commit->generation,
 +                                   generation,
                                     max_generation + 1);
  
                if (graph_commit->date != odb_commit->date)