Re: [PATCH v3 08/14] commit-graph: implement 'git-commit-graph clear'

2018-02-13 Thread Jonathan Tan
On Thu,  8 Feb 2018 15:37:32 -0500
Derrick Stolee  wrote:

> Teach Git to delete the current 'graph_head' file and the commit graph
> it references. This is a good safety valve if somehow the file is
> corrupted and needs to be recalculated. Since the commit graph is a
> summary of contents already in the ODB, it can be regenerated.

Spelling of graph-head (hyphen, not underscore).

I'm not sure of the usefulness of this feature - if the graph is indeed
corrupt, the user can just be instructed to delete graph-head (not even
the commit graph it references, since when we create a new graph-head,
--delete-expired will take care of deleting the old one).

>  extern char *get_graph_head_filename(const char *pack_dir);
> +extern struct object_id *get_graph_head_hash(const char *pack_dir,
> +  struct object_id *hash);
>  extern char* get_commit_graph_filename_hash(const char *pack_dir,
>   struct object_id *hash);

This file is starting to need documentation - in particular, the
difference between get_graph_head_hash() and
get_commit_graph_filename_hash() is not clear.


[PATCH v3 08/14] commit-graph: implement 'git-commit-graph clear'

2018-02-08 Thread Derrick Stolee
Teach Git to delete the current 'graph_head' file and the commit graph
it references. This is a good safety valve if somehow the file is
corrupted and needs to be recalculated. Since the commit graph is a
summary of contents already in the ODB, it can be regenerated.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt | 11 +
 builtin/commit-graph.c | 50 ++
 commit-graph.c | 23 ++
 commit-graph.h |  2 ++
 t/t5318-commit-graph.sh|  5 
 5 files changed, 91 insertions(+)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 5e32c43b27..8c2cbbc923 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -9,6 +9,7 @@ git-commit-graph - Write and verify Git commit graphs (.graph 
files)
 SYNOPSIS
 
 [verse]
+'git commit-graph clear' [--pack-dir ]
 'git commit-graph read'  [--pack-dir ]
 'git commit-graph write'  [--pack-dir ]
 
@@ -45,6 +46,10 @@ details about the graph file.
 With `--graph-hash=` option, consider the graph file
 graph-.graph in the pack directory.
 
+'clear'::
+
+Delete the graph-head file and the graph file it references.
+
 
 EXAMPLES
 
@@ -68,6 +73,12 @@ $ git commit-graph write --update-head
 $ git commit-graph read --graph-hash=
 
 
+* Delete /graph-head and the file it references.
++
+
+$ git commit-graph clear --pack-dir=
+
+
 
 GIT
 ---
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 776ca087e8..529cb80de6 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -1,16 +1,23 @@
 #include "builtin.h"
 #include "config.h"
+#include "dir.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "commit-graph.h"
 
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--pack-dir ]"),
+   N_("git commit-graph clear [--pack-dir ]"),
N_("git commit-graph read [--graph-hash=]"),
N_("git commit-graph write [--pack-dir ] [--update-head]"),
NULL
 };
 
+static const char * const builtin_commit_graph_clear_usage[] = {
+   N_("git commit-graph clear [--pack-dir ]"),
+   NULL
+};
+
 static const char * const builtin_commit_graph_read_usage[] = {
N_("git commit-graph read [--pack-dir ]"),
NULL
@@ -27,6 +34,47 @@ static struct opts_commit_graph {
int update_head;
 } opts;
 
+static int graph_clear(int argc, const char **argv)
+{
+   char *old_path;
+   char *head_fname;
+   struct object_id old_graph_hash;
+
+   static struct option builtin_commit_graph_clear_options[] = {
+   { OPTION_STRING, 'p', "pack-dir", &opts.pack_dir,
+   N_("dir"),
+   N_("The pack directory to store the graph") },
+   OPT_END(),
+   };
+
+   argc = parse_options(argc, argv, NULL,
+builtin_commit_graph_clear_options,
+builtin_commit_graph_clear_usage, 0);
+
+   if (!opts.pack_dir) {
+   struct strbuf path = STRBUF_INIT;
+   strbuf_addstr(&path, get_object_directory());
+   strbuf_addstr(&path, "/pack");
+   opts.pack_dir = strbuf_detach(&path, NULL);
+   }
+
+   if (!get_graph_head_hash(opts.pack_dir, &old_graph_hash))
+   return 0;
+
+   head_fname = get_graph_head_filename(opts.pack_dir);
+   if (remove_path(head_fname))
+   die("failed to remove path %s", head_fname);
+   FREE_AND_NULL(head_fname);
+
+   old_path = get_commit_graph_filename_hash(opts.pack_dir,
+ &old_graph_hash);
+   if (remove_path(old_path))
+   die("failed to remove path %s", old_path);
+   free(old_path);
+
+   return 0;
+}
+
 static int graph_read(int argc, const char **argv)
 {
struct object_id graph_hash;
@@ -162,6 +210,8 @@ int cmd_commit_graph(int argc, const char **argv, const 
char *prefix)
 PARSE_OPT_STOP_AT_NON_OPTION);
 
if (argc > 0) {
+   if (!strcmp(argv[0], "clear"))
+   return graph_clear(argc, argv);
if (!strcmp(argv[0], "read"))
return graph_read(argc, argv);
if (!strcmp(argv[0], "write"))
diff --git a/commit-graph.c b/commit-graph.c
index 9789fe37f9..95b813c2c7 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -46,6 +46,29 @@ char *get_graph_head_filename(const char *pack_dir)
return strbuf_detach(&fname, 0);
 }
 
+struct object_id *get_graph_head_hash(const char *pack_dir, struct object_id 
*hash)
+{
+   char hex[GIT_MAX_HEXSZ + 1];
+   char *fname;
+   FILE *f;
+
+   f