Re: [PATCH 03/14] packed-graph: create git-graph builtin

[Derrick Stolee <stolee@gmail.com>]

On 1/25/2018 4:45 PM, Stefan Beller wrote:
> On Thu, Jan 25, 2018 at 6:02 AM, Derrick Stolee <stolee@gmail.com> wrote:
>> Teach Git the 'graph' builtin that will be used for writing and
>> reading packed graph files. The current implementation is mostly
>> empty, except for a check that the core.graph setting is enabled
>> and a '--pack-dir' option.
> I wonder if this builtin should not respect the boolean core graph,
> as this new builtin commands' whole existence
> is to deal with these new files?
>
> As you assume this builtin as a plumbing command, I would
> expect it to pay less attention to config rather than more.

My thought was to alert the caller "This graph isn't going to be good 
for anything!" and fail quickly before doing work. You do have a good 
point, and I think we can remove that condition here. When we integrate 
with other commands ('repack', 'fetch', 'clone') we will want a 
different setting that signals automatically writing the graph and we 
don't want those to fail because they are not aware of a second config 
setting.

>
>> @@ -408,6 +408,7 @@ static struct cmd_struct commands[] = {
>>          { "fsck-objects", cmd_fsck, RUN_SETUP },
>>          { "gc", cmd_gc, RUN_SETUP },
>>          { "get-tar-commit-id", cmd_get_tar_commit_id },
>> +       { "graph", cmd_graph, RUN_SETUP_GENTLY },
> Why gently, though?
>
>  From reading the docs (and assumptions on further patches)
> we'd want to abort if there is no .git dir to be found?
>
> Or is a future patch having manual logic? (e.g. if pack-dir is
> given, the command may be invoked from outside a git dir)

You are right. I inherited this from my MIDX patch which can operate on 
a list of IDX files without a .git folder. The commit graph operations 
need an ODB.

Thanks,
-Stolee