Skip to content

RFC Fix commit-graph/graft/replace/shallow combo

Derrick Stolee git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 RFC Latest6 patches 11 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/6DO NOT MERGE: compute commit-graph on every commitbuiltin/commit.c builtin/gc.c environment.c+7 -21 reply
2/6DO NOT MERGE: write commit-graph on every fetchbuiltin/fetch.c+4 -00 replies
3/6commit-graph: enable replace-object and graftsbuiltin/replace.c commit-graph.c commit-graph.h +1 more+36 -11 reply
4/6commit-graph: avoid writing when repo is shallowcommit-graph.c+12 -05 replies
5/6fetch: destroy commit graph on shallow parametersbuiltin/fetch.c+6 -01 reply
6/6commit-graph: revert to odb on missing parentscommit-graph.c+30 -30 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [RFC PATCH 0/6] Fix commit-graph/graft/replace/shallow combo

The commit-graph file stores a condensed version of the commit history. This helps speed up several operations involving commit walks. This feature does not work well if those commits "change" using features like commit grafts, replace objects, or shallow clones.

Since the commit-graph feature is optional, hidden behind the 'core.commitGraph' config option, and requires manual maintenance (until ds/commit-graph-fsck' is merged), these issues only arise for expert users who decided to opt-in.

This RFC is a first attempt at rectify the issues that come up when these features interact. I have not succeeded entirely, as I will discuss below.

The first two "DO NOT MERGE" commits are not intended to be part of the main product, but are ways to help the full test suite run while computing and consuming commit-graph files. This is acheived by calling write_commit_graph_reachable() from git fetch and git commit. I believe this is the only dependence on ds/commit-graph-fsck. The other commits should only depend on ds/commit-graph-lockfile-fix.

Running the full test suite after these DO NO MERGE commits results in the following test failures, which I categorize by type of failure.

The following tests are red herrings. Most work by deleting a commit from the object database and trying to demonstrate a failure. Others rely on how certain objects are loaded. These are not bugs, but will add noise if you run the tests suite with this patch.

t0410-partial-clone.sh Failed tests: 5, 8
t5307-pack-missing-commit.sh Failed tests: 3-4
t6011-rev-list-with-bad-commit.sh Failed test: 4
t6024-recursive-merge.sh Failed test: 4

The following tests are fixed in "commit-graph: enable replace-object and grafts".

t6001-rev-list-graft.sh Failed tests: 3, 5, 7, 9, 11, 13
t6050-replace.sh Failed tests: 11-15, 23-24, 29

The following tests involve shallow clones.

t5500-fetch-pack.sh Failed tests: 30-31, 34, 348-349
t5537-fetch-shallow.sh Failed tests: 4-7, 9
t5802-connect-helper.sh Failed test: 3

These tests are mostly fixed by three commits:

* commit-graph: avoid writing when repo is shallow
* fetch: destroy commit graph on shallow parameters
* commit-graph: revert to odb on missing parents

Each of these cases cover a different interaction that occurs with shallow clones. Some are due to a commit becoming shallow, while others are due to a commit becoming unshallow (and hence invalidating its generation number).

After these changes, there is one test case that still fails, and I cannot understand why:

t5500-fetch-pack.sh Failed test: 348

This test fails when performing a git fetch --shallow-exclude. When I halt the test using t5500-fetch-pack.sh -d -i and navigate to the directory to replay the fetch it performs as expected. After struggling with it for a while, I figured I should just put this series up for discussion. Maybe someone with more experience in shallow clones can point out the obvious issues I'm having.

Thanks,
-Stolee

Derrick Stolee (6):
DO NOT MERGE: compute commit-graph on every commit DO NOT MERGE: write commit-graph on every fetch
commit-graph: enable replace-object and grafts
commit-graph: avoid writing when repo is shallow
fetch: destroy commit graph on shallow parameters
commit-graph: revert to odb on missing parents

 builtin/commit.c  |  5 +++++
 builtin/fetch.c   | 10 +++++++++
 builtin/gc.c      |  2 +-
 builtin/replace.c |  3 +++
 commit-graph.c    | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 commit-graph.h    |  9 ++++++++
 commit.c          |  5 +++++
 environment.c     |  2 +-
 8 files changed, 95 insertions(+), 6 deletions(-)
Signature · 1 line
2.16.2.338.gcfe06ae955