Re: [PATCH v3 14/14] commit-graph: build graph from starting commits

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 9, 2018, 13:45 UTC
Message-ID: <054043b5-7f2e-0b69-d770-ab23b756f82b@gmail.com>
In-Reply-To: <CAM0VKjk-JvCNebTiHVm16vtwKpLF+N-gfGNL=7sYR=OVTb979w@mail.gmail.com>

On 2/9/2018 8:02 AM, SZEDER Gábor wrote:

Show 27 quoted lines

> On Thu, Feb 8, 2018 at 9:37 PM, Derrick Stolee <stolee@gmail.com> wrote:
>> Teach git-commit-graph to read commits from stdin when the
>> --stdin-commits flag is specified. Commits reachable from these
>> commits are added to the graph. This is a much faster way to construct
>> the graph than inspecting all packed objects, but is restricted to
>> known tips.
>>
>> For the Linux repository, 700,000+ commits were added to the graph
>> file starting from 'master' in 7-9 seconds, depending on the number
>> of packfiles in the repo (1, 24, or 120).
> It seems something went wrong with '--stdin-commits' in v3, look:
>
>    ~/src/git (commit-graph-v2 %)$ time { git rev-parse HEAD | ./git
> commit-graph --write --update-head --stdin-commits ; }
>    ee3223fe116bf7031a6c1ad6d41e0456beefa754
>
>    real  0m1.199s
>    user  0m1.123s
>    sys   0m0.024s
>
>    ~/src/git (commit-graph-v3 %)$ time { git rev-parse HEAD | ./git
> commit-graph write --update-head --stdin-commits ; }
>    ee3223fe116bf7031a6c1ad6d41e0456beefa754
>
>    real  0m30.766s
>    user  0m29.120s
>    sys   0m0.546s

Thanks, Szeder. You're right. This is the diff that I forgot to apply in the last commit:

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 28d043b..175b967 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -257,7 +257,7 @@ static int graph_write(int argc, const char **argv)

         has_existing = !!get_graph_head_hash(opts.pack_dir, 
&old_graph_hash);

-       if (opts.stdin_packs) {
+       if (opts.stdin_packs || opts.stdin_commits) {
                 struct strbuf buf = STRBUF_INIT;
                 nr_lines = 0;
                 alloc_lines = 128;


I'll work to create a test that ensures we are only adding commits 
reachable from specific commits to prevent this regression.

Thanks,
-Stolee