Re: [PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Junio C Hamano]

Derrick Stolee  writes:

> On 5/29/2018 12:27 AM, Junio C Hamano wrote:
>> Derrick Stolee  writes:
>>
>>> Thanks for all the feedback on v2. I've tried to make this round's
>>> review a bit easier by splitting up the commits into smaller pieces.
>>> Also, the test script now has less boilerplate and uses variables and
>>> clear arithmetic to explain which bytes are being modified.
>>>
>>> One other change worth mentioning: in "commit-graph: add '--reachable'
>>> option" I put the ref-iteration into a new external
>>> 'write_commit_graph_reachable()' method inside commit-graph.c. This
>>> makes the 'gc: automatically write commit-graph files' a simpler change.
>> I finally managed to find time to resolve conflicts this topic has
>> with other topics (of your own included, if I am not mistaken).
>> Please double check the resolution when I push out the day's
>> integration result later today.
>
> Sorry about the confusion. I was operating on my copy of
> ds/generation-numbers (34fdd43339) but fetching just now I see you
> updated that branch to 1472978ec6. I reset my local branch to
> ds/commit-graph-fsk (53dd1e6600). The only diff I see between my v3
> branch and that commit are the changes from
> ds/commit-graph-lockfile-fix (33286dcd6d).

Sorry for a confusing/confused comment.  The topic I had in mind
that I saw interactions with this one was the per-in-core-repo
allocation (sb/object-store-alloc), which was not yours.

In any case, what I wanted to see sanity checked was the result of
the merge into 'pu' (which needed some semantic adjustment), not
individual patches on the topic branch.  Relative to what we see at
the tip of 'pu' right now, what I'll be pushing out in 3 or 4 hours
will gain yet another semantic adjustment, so you may want to wait
a bit more to avoid duplicated work.

Thanks.

Re: [PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Derrick Stolee]

On 5/29/2018 12:27 AM, Junio C Hamano wrote:

Derrick Stolee  writes:


Thanks for all the feedback on v2. I've tried to make this round's
review a bit easier by splitting up the commits into smaller pieces.
Also, the test script now has less boilerplate and uses variables and
clear arithmetic to explain which bytes are being modified.

One other change worth mentioning: in "commit-graph: add '--reachable'
option" I put the ref-iteration into a new external
'write_commit_graph_reachable()' method inside commit-graph.c. This
makes the 'gc: automatically write commit-graph files' a simpler change.

I finally managed to find time to resolve conflicts this topic has
with other topics (of your own included, if I am not mistaken).
Please double check the resolution when I push out the day's
integration result later today.


Sorry about the confusion. I was operating on my copy of 
ds/generation-numbers (34fdd43339) but fetching just now I see you 
updated that branch to 1472978ec6. I reset my local branch to 
ds/commit-graph-fsk (53dd1e6600). The only diff I see between my v3 
branch and that commit are the changes from ds/commit-graph-lockfile-fix 
(33286dcd6d).


Thanks,
-Stolee

Re: [PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Junio C Hamano]

Derrick Stolee  writes:

> Thanks for all the feedback on v2. I've tried to make this round's
> review a bit easier by splitting up the commits into smaller pieces.
> Also, the test script now has less boilerplate and uses variables and
> clear arithmetic to explain which bytes are being modified.
>
> One other change worth mentioning: in "commit-graph: add '--reachable'
> option" I put the ref-iteration into a new external
> 'write_commit_graph_reachable()' method inside commit-graph.c. This
> makes the 'gc: automatically write commit-graph files' a simpler change.

I finally managed to find time to resolve conflicts this topic has
with other topics (of your own included, if I am not mistaken).
Please double check the resolution when I push out the day's
integration result later today.

Thanks.

Re: [PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Junio C Hamano]

Derrick Stolee  writes:

> One other change worth mentioning: in "commit-graph: add '--reachable'
> option" I put the ref-iteration into a new external
> 'write_commit_graph_reachable()' method inside commit-graph.c. This
> makes the 'gc: automatically write commit-graph files' a simpler change.

;-).

Re: [PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Ævar Arnfjörð Bjarmason]

On Thu, May 24 2018, Derrick Stolee wrote:

> Thanks for all the feedback on v2. I've tried to make this round's
> review a bit easier by splitting up the commits into smaller pieces.
> Also, the test script now has less boilerplate and uses variables and
> clear arithmetic to explain which bytes are being modified.

Thanks. it's a lot easier.

> One other change worth mentioning: in "commit-graph: add '--reachable'
> option" I put the ref-iteration into a new external
> 'write_commit_graph_reachable()' method inside commit-graph.c. This
> makes the 'gc: automatically write commit-graph files' a simpler change.

Maybe you want this, maybe not, but I came up with this to squash:

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9a3abd87e7..2665522385 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -900,7 +900,8 @@ the `GIT_NOTES_REF` environment variable.  See 
linkgit:git-notes[1].

 core.commitGraph::
Enable git commit graph feature. Allows reading from the
-   commit-graph file.
+   commit-graph file. See `gc.commitGraph` for automatically
+   maintaining the file.

 core.sparseCheckout::
Enable "sparse checkout" feature. See section "Sparse checkout" in
@@ -1554,10 +1555,10 @@ gc.autoDetach::
if the system supports it. Default is true.

 gc.commitGraph::
-   If true, then gc will rewrite the commit-graph file after any
-   change to the object database. If '--auto' is used, then the
-   commit-graph will not be updated unless the threshold is met.
-   See linkgit:git-commit-graph[1] for details.
+   If true, then gc will rewrite the commit-graph file when
+   linkgit:git-gc[1] is run. When using linkgit:git-gc[1]
+   '--auto' the commit-graph will be updated if housekeeping is
+   required. See linkgit:git-commit-graph[1] for details.

 gc.logExpiry::
If the file gc.log exists, then `git gc --auto` won't run

I.e. let's mention the new gc.commitGraph in core.commitGraph, and I
think the "any change to the object database" line in gc.commitGraph is
needlessly confusing, let's just say "when git-gc is run".

[PATCH v3 00/20] Integrate commit-graph into 'fsck' and 'gc'

[Derrick Stolee]

Thanks for all the feedback on v2. I've tried to make this round's
review a bit easier by splitting up the commits into smaller pieces.
Also, the test script now has less boilerplate and uses variables and
clear arithmetic to explain which bytes are being modified.

One other change worth mentioning: in "commit-graph: add '--reachable'
option" I put the ref-iteration into a new external
'write_commit_graph_reachable()' method inside commit-graph.c. This
makes the 'gc: automatically write commit-graph files' a simpler change.

Thanks,
-Stolee

Derrick Stolee (20):
  commit-graph: UNLEAK before die()
  commit-graph: fix GRAPH_MIN_SIZE
  commit-graph: parse commit from chosen graph
  commit: force commit to parse from object database
  commit-graph: load a root tree from specific graph
  commit-graph: add 'verify' subcommand
  commit-graph: verify catches corrupt signature
  commit-graph: verify required chunks are present
  commit-graph: verify corrupt OID fanout and lookup
  commit-graph: verify objects exist
  commit-graph: verify root tree OIDs
  commit-graph: verify parent list
  commit-graph: verify generation number
  commit-graph: verify commit date
  commit-graph: test for corrupted octopus edge
  commit-graph: verify contents match checksum
  fsck: verify commit-graph
  commit-graph: add '--reachable' option
  gc: automatically write commit-graph files
  commit-graph: update design document

 Documentation/config.txt |   6 +
 Documentation/git-commit-graph.txt   |  14 +-
 Documentation/git-fsck.txt   |   3 +
 Documentation/git-gc.txt |   4 +
 Documentation/technical/commit-graph.txt |  22 ---
 builtin/commit-graph.c   |  59 +++-
 builtin/fsck.c   |  21 +++
 builtin/gc.c |   6 +
 commit-graph.c   | 234 +--
 commit-graph.h   |   3 +
 commit.c |   9 +-
 commit.h |   1 +
 t/t5318-commit-graph.sh  | 196 ++
 13 files changed, 539 insertions(+), 39 deletions(-)


base-commit: 34fdd433396ee0e3ef4de02eb2189f8226eafe4e
-- 
2.16.2.329.gfb62395de6