Re: [PATCH v9 0/3] push: add "--[no-]force-if-includes"

Hello,

On 10/01/2020 10:12, Junio C Hamano wrote:

I didn't want want to cause a delay with this patch. Since the new option was seemingly working without it, I decided to disable it here and added a "TODO" in the comments to remove "toggle_commit_graph()" in the future. We can definitely put this on hold and wait until the with and without "commit-graph" result disparities are clarified.

Thanks, I appreciate it!
 

:)
 

Ah, this will keep a record of the behavior in the tests; nice, will update in the next set.

I looked around a little bit trying to understand what was happening here. As mentioned before [1], "repo_in_merge_bases_many()" returns early because of this:

  for (i = 0; i < nr_reference; i++) {
	  if (repo_parse_commit(r, reference[i]))
		  return ret;

	  generation = commit_graph_generation(reference[i]);
	  if (generation < min_generation)
		  min_generation = generation;
	  fprintf(stderr,
		  "[%s]: (local): generation: %u, min_generation: %u\n",
		  oid_to_hex(&reference[i]->object.oid),
		  generation,
		  min_generation);
  }

  generation = commit_graph_generation(commit);
  fprintf(stderr, "[%s]: (remote) generation: %u, min_generation: %u\n",
	  oid_to_hex(&commit->object.oid), generation, min_generation);
  if (generation > min_generation) {
	  return ret;
  }

I made some changes locally to display the generation numbers of all the commits that were collected during the "reflog" traversal.

In the beginning we get the minimum generation number of the list of the commits in "reference[]" to see if the "commit" is reachable from one of the items in "reference[]". Since in this case, the generation number of "commit" is higher than minimum amongst the members of "reference", does it mean it cannot be reached?

When "GIT_TEST_COMMIT_GRAPH" is turned off, all of the commits in "reference" as well as "commit" have "GENERATION_NUMBER_INFINITY", and the path moves forward to "paint_down_to_common()" and correctly identifies the reachability.

I ran the test again, but this time with running "git-commit-graph" right before pushing:

  (a) git commit-graph write --reachable, and the commit's generation
      number was "GENERATION_NUMBER_INFINITY".

  (b) git-show-ref -s | git commit-graph write --stdin-commits, and
      the commit's generation number was 5.

and since generation number of "commit" was always higher than the minimum -- it returns that it is not reachable from any of "reference".

One of the failing tests in t5533 was modified (for debugging), and the following shows the difference in behavior of "repo_in_merge_bases_many()". The test checks if the remote tip is reachable from the reflog entries of the local branch after "git pull --rebase" is run.

  $ git log --graph --oneline # (before "pull --rebase")
   * be2465f J
   * 157d38b C
   * f9a2dac B
   * 112d1ac A

  $ git log --graph --oneline # (after "pull --rebase")
   * 7c16010 J
   * b995a30 G
   * 00b6961 F
   * 157d38b C
   * f9a2dac B
   * 112d1ac A

  $ git reflog master
  7c16010 master@{0}: pull --rebase origin master (finish): refs/heads/master onto b995a30227dd14b34b6f7728201aefac8cc12296
  be2465f master@{1}: commit: J
  157d38b master@{2}: commit: C
  f9a2dac master@{3}: commit: B
  112d1ac master@{4}: commit (initial): A

  - (a) is run.
    $ git push --force-if-includes --force-with-lease=master
    [7c16010bad84d8b53873875c2e242890920360f2]: (local):  generation: 4294967295, min_generation: 4294967295
    [be2465f6a155acb8f5eb9ee876bad730e0f656cf]: (local):  generation: 4, min_generation: 4
    [157d38b4112d457e6645c7c4e9a486e6189be435]: (local):  generation: 3, min_generation: 3
    [f9a2dac17e4f8cafaa9655d40cb86c53094da8d6]: (local):  generation: 2, min_generation: 2
    [112d1ac551b908f10b995d7e41456f4cd8f071c5]: (local):  generation: 1, min_generation: 1
    [b995a30227dd14b34b6f7728201aefac8cc12296]: (remote): generation: 4294967295, min_generation: 1
    [...] fail.

  - "git fetch --all" and (b) is run.
    $ git push --force-if-includes --force-with-lease=master
    [7c16010bad84d8b53873875c2e242890920360f2]: (local):  generation: 4294967295, min_generation: 4294967295
    [be2465f6a155acb8f5eb9ee876bad730e0f656cf]: (local):  generation: 4, min_generation: 4
    [157d38b4112d457e6645c7c4e9a486e6189be435]: (local):  generation: 3, min_generation: 3
    [f9a2dac17e4f8cafaa9655d40cb86c53094da8d6]: (local):  generation: 2, min_generation: 2
    [112d1ac551b908f10b995d7e41456f4cd8f071c5]: (local):  generation: 1, min_generation: 1
    [b995a30227dd14b34b6f7728201aefac8cc12296]: (remote): generation: 5, min_generation: 1
    [...] fail.

  - neither (a), nor (b) are run, and "core.commitGraph" is disabled.
    $ git push --force-if-includes --force-with-lease=master
    [7c16010bad84d8b53873875c2e242890920360f2]: (local):  generation: 4294967295, min_generation: 4294967295
    [be2465f6a155acb8f5eb9ee876bad730e0f656cf]: (local):  generation: 4294967295, min_generation: 4294967295
    [157d38b4112d457e6645c7c4e9a486e6189be435]: (local):  generation: 4294967295, min_generation: 4294967295
    [f9a2dac17e4f8cafaa9655d40cb86c53094da8d6]: (local):  generation: 4294967295, min_generation: 4294967295
    [112d1ac551b908f10b995d7e41456f4cd8f071c5]: (local):  generation: 4294967295, min_generation: 4294967295
    [b995a30227dd14b34b6f7728201aefac8cc12296]: (remote): generation: 4294967295, min_generation: 4294967295
    [...] pass.

It looks like G (b995a30) is reachable from J (7c16010), but "repo_in_merge_bases_many()" disagrees when "GIT_TEST_COMMIT_GRAPH" is enabled. I hope this information helps a little to understand why this happens and whether it is a bug or not.

[1] https://public-inbox.org/git/20200928193400.GA88208@mail.clickyotomy.dev

Thanks.