Re: [PATCH v4 08/13] commit-graph: implement --delete-expired

[Derrick Stolee <stolee@gmail.com>]

On 2/21/2018 4:34 PM, Stefan Beller wrote:
> On Mon, Feb 19, 2018 at 10:53 AM, Derrick Stolee <stolee@gmail.com> wrote:
>
>>          graph_name = write_commit_graph(opts.obj_dir);
>>
>>          if (graph_name) {
>>                  if (opts.set_latest)
>>                          set_latest_file(opts.obj_dir, graph_name);
>>
>> +               if (opts.delete_expired)
>> +                       do_delete_expired(opts.obj_dir,
>> +                                         old_graph_name,
>> +                                         graph_name);
>> +
> So this only allows to delete expired things and setting the latest
> when writing a new graph. Would we ever envision a user to produce
> a new graph (e.g. via obtaining a graph that they got from a server) and
> then manually rerouting the latest to that new graph file without writing
> that graph file in the same process? The same for expired.
>
> I guess these operations are just available via editing the
> latest or deleting files manually, which slightly contradicts
> e.g. "git update-ref", which in olden times was just a fancy way
> of rewriting the refs file manually. (though it claims to be less
> prone to errors as it takes lock files)

I imagine these alternatives for placing a new, latest commit graph file 
would want Git to handle rewriting the "graph-latest" file. Given such a 
use case, we could consider extending the 'commit-graph' interface, but 
I don't want to plan for it now.

>
>>   extern char *get_graph_latest_filename(const char *obj_dir);
>> +extern char *get_graph_latest_contents(const char *obj_dir);
> Did
> https://public-inbox.org/git/20180208213806.GA6381@sigill.intra.peff.net/
> ever make it into tree? (It is sort of new, but I feel we'd want to
> strive for consistency in the code base, eventually.)

Thank you for the reminder. I've removed the externs from 'commit-graph.h'.

Should I also remove the externs from other methods I introduce even 
though their surrounding definitions include 'extern'?

Thanks,
-Stolee