From: Derrick Stolee Date: Tue, 21 Aug 2018 18:35:48 GMT Subject: Re: [PATCH v2 0/8] Clarify commit-graph and grafts/replace/shallow incompatibilities Message-ID: <76ef06b5-87cb-ff40-a424-01a3b0ed435a@gmail.com> In-Reply-To: On 8/21/2018 1:51 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> On Mon, Aug 20, 2018 at 11:24 AM Derrick Stolee wrote: >>> Because of the change of base, it is hard to provide a side-by-side diff >>> from v1. >> I thought that was the whole point of range-diff. ;-) > I thought so, too. Was there any change that confused range-diff > machinery? I've just been out of the loop and didn't realize what range-diff did. Here is the range-diff: 1:  43ddcc9ef9 = 1:  c8edae4179 refs.c: migrate internal ref iteration to pass thru repository argument 2:  22dc9ce836 = 2:  f89451e884 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback 3:  5e90d36f84 ! 3:  d95aa3472c commit-graph: update design document     @@ -19,7 +19,7 @@         so a future change of hash algorithm does not require a change in format.      +- Commit grafts and replace objects can change the shape of the commit     -+  history. These can also be enabled/disabled on the fly using     ++  history. The latter can also be enabled/disabled on the fly using      +  `--no-replace-objects`. This leads to difficultly storing both possible      +  interpretations of a commit id, especially when computing generation      +  numbers. The commit-graph will not be read or written when 4:  88711a3cf4 = 4:  ec89c88e14 test-repository: properly init repo 5:  7f596c1718 ! 5:  0321a3cf10 commit-graph: not compatible with replace objects     @@ -6,10 +6,34 @@          repository r is compatible with the commit-graph feature. Fill the          method with a check to see if replace-objects exist. Test this          interaction succeeds, including ignoring an existing commit-graph and     -    failing to write a new commit-graph.     +    failing to write a new commit-graph. However, we do ensure that     +    we write a new commit-graph by setting read_replace_refs to 0, thereby     +    ignoring the replace refs.          Signed-off-by: Derrick Stolee     +diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c     +--- a/builtin/commit-graph.c     ++++ b/builtin/commit-graph.c     +@@     +   return 0;     + }     +     ++extern int read_replace_refs;     ++     + static int graph_write(int argc, const char **argv)     + {     +   struct string_list *pack_indexes = NULL;     +@@     +   if (!opts.obj_dir)     +           opts.obj_dir = get_object_directory();     +     ++  read_replace_refs = 0;     ++     +   if (opts.reachable) {     +           write_commit_graph_reachable(opts.obj_dir, opts.append);     +           return 0;     +      diff --git a/commit-graph.c b/commit-graph.c      --- a/commit-graph.c      +++ b/commit-graph.c     @@ -26,11 +50,15 @@         return g;       }     ++extern int read_replace_refs;     ++      +static int commit_graph_compatible(struct repository *r)      +{     -+  prepare_replace_object(r);     -+  if (hashmap_get_size(&r->objects->replace_map->map))     -+          return 0;     ++  if (read_replace_refs) {     ++          prepare_replace_object(r);     ++          if (hashmap_get_size(&r->objects->replace_map->map))     ++                  return 0;     ++  }      +      +  return 1;      +}     @@ -110,7 +138,7 @@      +          test_cmp expect actual &&      +          rm -rf .git/objects/info/commit-graph &&      +          git commit-graph write --reachable &&     -+          test_path_is_missing .git/objects/info/commit-graph     ++          test_path_is_file .git/objects/info/commit-graph      +  )      +'      + 6:  94dd91ac35 ! 6:  d18ecc0124 commit-graph: not compatible with grafts     @@ -7,24 +7,25 @@          situations we ignore existing commit-graph files and we do not write new          commit-graph files.     +    Helped-by: Jakub Narebski          Signed-off-by: Derrick Stolee      diff --git a/commit-graph.c b/commit-graph.c      --- a/commit-graph.c      +++ b/commit-graph.c      @@     +                   return 0;     +   }     - static int commit_graph_compatible(struct repository *r)     - {      +  prepare_commit_graft(r);      +  if (r->parsed_objects && r->parsed_objects->grafts_nr)      +          return 0;      +  if (is_repository_shallow(r))      +          return 0;      +     -   prepare_replace_object(r);     -   if (hashmap_get_size(&r->objects->replace_map->map))     -           return 0;     +   return 1;     + }     +      diff --git a/commit.c b/commit.c      --- a/commit.c     @@ -66,7 +67,9 @@      +          cd graft &&      +          git commit-graph write --reachable &&      +          test_path_is_file .git/objects/info/commit-graph &&     -+          git replace --graft HEAD~1 HEAD~3 &&     ++          H1=$(git rev-parse --verify HEAD~1) &&     ++          H3=$(git rev-parse --verify HEAD~3) &&     ++          echo "$H1 $H3" >.git/info/grafts &&      +          git -c core.commitGraph=false log >expect &&      +          git -c core.commitGraph=true log >actual &&      +          test_cmp expect actual && 7:  5314a5a93d ! 7:  87d3397700 commit-graph: not compatible with uninitialized repo     @@ -14,6 +14,6 @@      +  if (!r->gitdir)      +          return 0;      +     -   prepare_commit_graft(r);     -   if (r->parsed_objects && r->parsed_objects->grafts_nr)     -           return 0;     +   if (read_replace_refs) {     +           prepare_replace_object(r);     +           if (hashmap_get_size(&r->objects->replace_map->map)) 8:  f4ab234ed2 ! 8:  a26c175897 commit-graph: close_commit_graph before shallow walk     @@ -2,8 +2,6 @@          commit-graph: close_commit_graph before shallow walk     -    Make close_commit_graph() work for arbitrary repositories.     -          Call close_commit_graph() when about to start a rev-list walk that          includes shallow commits. This is necessary in code paths that "fake"          shallow commits for the sake of fetch. Specifically, test 351 in