On 1/24/2019 4:40 AM, Ævar Arnfjörð Bjarmason wrote:
On Wed, Jan 23 2019, Derrick Stolee via GitGitGadget wrote:
The new format modifies the header of the commit-graph to solve
these problems. We use the 4-byte hash format id, freeing up a byte
in our 32-bit alignment to introduce a reachability index version.
We can also fail to read the commit-graph if the eighth byte is
non-zero.
I haven't tested, but it looks from the patch like we can transparently
read existing v1 data and then will write v2 the next time. Would be
helpful for reviewers if this was noted explicitly in the commit
message.
Can do.
Should there be a GIT_TEST_COMMIT_GRAPH_VERSION=[12] going forward to
test the non-default version, or do you feel confident the tests added
here test the upgrade path & old code well enough?
You're right that we should have an explicit "upgrade" test:
1. Write a v1 commit-graph
2. Add a commit
3. Write a v2 commit-graph
As for a new GIT_TEST_ variable, we should only need to continue relying
on GIT_TEST_COMMIT_GRAPH to test v2. I can add a 'graph_git_behavior'
call on an explicitly v1 commit-graph file to get most of the coverage
we need.
The 'git commit-graph read' subcommand needs updating to show the
new data.
Let's say "The ... subcommand has been updated to show the new
data". This sounds like a later patch is going to do that, but in fact
it's done here.
Will clean up.
Set the default file format version to 2, and adjust the tests to
expect the new 'git commit-graph read' output.
Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
---
.../technical/commit-graph-format.txt | 26 +++++++++-
builtin/commit-graph.c | 9 ++++
commit-graph.c | 47 ++++++++++++++++---
commit-graph.h | 1 +
t/t5318-commit-graph.sh | 9 +++-
5 files changed, 83 insertions(+), 9 deletions(-)
diff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt
index 16452a0504..e367aa94b1 100644
--- a/Documentation/technical/commit-graph-format.txt
+++ b/Documentation/technical/commit-graph-format.txt
@@ -31,13 +31,22 @@ and hash type.
All 4-byte numbers are in network order.
+There are two versions available, 1 and 2. These currently differ only in
+the header.
Shouldn't this be s/currently/ / ? Won't we add a version 3 if we make
new changes?
When we add a new reachability index version, then the content of the
data chunk will change. Since we have a separate byte for versioning
that data, we do not need a v3 for the file format as a whole. A similar
statement applies to the unused byte reserved for the incremental file
format: we won't need to increase the file format version as we will
make that number non-zero and add a chunk with extra data.
Thanks,
-Stolee
