On 9/3/2019 3:05 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index 53ce99d2bb..d36a403859 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -23,6 +23,7 @@
>> #include "packfile.h"
>> #include "list-objects-filter-options.h"
>> #include "commit-reach.h"
>> +#include "commit-graph.h"
>>
>> #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
>>
>> @@ -1715,6 +1716,20 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>>
>> string_list_clear(&list, 0);
>>
>> + prepare_repo_settings(the_repository);
>> + if (the_repository->settings.fetch_write_commit_graph) {
>> + int commit_graph_flags = COMMIT_GRAPH_SPLIT;
>> + struct split_commit_graph_opts split_opts;
>> + memset(&split_opts, 0, sizeof(struct split_commit_graph_opts));
>> +
>> + if (progress)
>> + commit_graph_flags |= COMMIT_GRAPH_PROGRESS;
>> +
>> + write_commit_graph_reachable(get_object_directory(),
>> + commit_graph_flags,
>> + &split_opts);
>> + }
>
> As a low-impact change this is good.
>
> For longer term, it feels a bit unfortunate that this is still a
> separate phase of the program, though. We know what new refs we
> added, we know what new objects we received, and we even scanned
> each and every one of them while running the index-pack step to
> store the .pack and compute the .idx file, i.e. it feels that we
> have most of the information already in-core to extend the commit
> graph for new parts of the history we just received.
You're right that we could isolate the new write to the refs we
just received. We could use the more cumbersome write_commit_graph()
method with a list of commit oids as starting points. I'm happy to
make that change if we see a lot of value there.
However, the current patch also gives us a way to add local refs
to the commit graph and "catch up" to the work the user had done.
With this in mind, I do think the simpler code has another functional
advantage.
Thanks,
-Stolee
