git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 07/14] commit-graph: implement git-commit-graph --update-head

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Feb 2, 2018, 01:35 UTC
Message-ID
<20180202013552.24931-1-szeder.dev@gmail.com>
In-Reply-To
<1517348383-112294-8-git-send-email-dstolee@microsoft.com>
> It is possible to have multiple commit graph files in a pack directory,
> but only one is important at a time. Use a 'graph_head' file to point
> to the important file.
This implies that all those other files are ignored, right?
Show 22 quoted lines
> Teach git-commit-graph to write 'graph_head' upon
> writing a new commit graph file.
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Documentation/git-commit-graph.txt | 34 ++++++++++++++++++++++++++++++++++
>  builtin/commit-graph.c             | 38 +++++++++++++++++++++++++++++++++++---
>  commit-graph.c                     | 25 +++++++++++++++++++++++++
>  commit-graph.h                     |  2 ++
>  t/t5318-commit-graph.sh            | 12 ++++++++++--
>  5 files changed, 106 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt
> index 09aeaf6c82..99ced16ddc 100644
> --- a/Documentation/git-commit-graph.txt
> +++ b/Documentation/git-commit-graph.txt
> @@ -12,15 +12,49 @@ SYNOPSIS
>  'git commit-graph' --write <options> [--pack-dir <pack_dir>]
>  'git commit-graph' --read <options> [--pack-dir <pack_dir>]
>  
> +OPTIONS
> +-------
Oh, look, the 'OPTIONS' section I missed in the previous patches! ;)

This should be split up and squashed into the previous patches where the individual --options are first mentioned.

Show 7 quoted lines
> +--pack-dir::
> +	Use given directory for the location of packfiles, graph-head,
> +	and graph files.
> +
> +--read::
> +	Read a graph file given by the graph-head file and output basic
> +	details about the graph file. (Cannot be combined with --write.)

From the output of 'git commit-graph --read' it seems that it's not a generally useful option to the user. Perhaps it should be mentioned that it's only intended as a debugging aid? Or maybe it doesn't really matter, because eventually this command will become irrelevant, as other commands (clone, fetch, gc) will invoke it automagically...

Show 6 quoted lines
> +--graph-id::
> +	When used with --read, consider the graph file graph-<oid>.graph.
> +
> +--write::
> +	Write a new graph file to the pack directory. (Cannot be combined
> +	with --read.)

I think this should also mention that it prints the generated graph file's checksum.

> +
> +--update-head::
> +	When used with --write, update the graph-head file to point to
> +	the written graph file.
So it should be used with '--write', noted.
Show 72 quoted lines
>  EXAMPLES
>  --------
>  
> +* Output the hash of the graph file pointed to by <dir>/graph-head.
> ++
> +------------------------------------------------
> +$ git commit-graph --pack-dir=<dir>
> +------------------------------------------------
> +
>  * Write a commit graph file for the packed commits in your local .git folder.
>  +
>  ------------------------------------------------
>  $ git commit-graph --write
>  ------------------------------------------------
>  
> +* Write a graph file for the packed commits in your local .git folder,
> +* and update graph-head.
> ++
> +------------------------------------------------
> +$ git commit-graph --write --update-head
> +------------------------------------------------
> +
>  * Read basic information from a graph file.
>  +
>  ------------------------------------------------
> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> index 218740b1f8..d73cbc907d 100644
> --- a/builtin/commit-graph.c
> +++ b/builtin/commit-graph.c
> @@ -11,7 +11,7 @@
>  static char const * const builtin_commit_graph_usage[] = {
>  	N_("git commit-graph [--pack-dir <packdir>]"),
>  	N_("git commit-graph --read [--graph-hash=<hash>]"),
> -	N_("git commit-graph --write [--pack-dir <packdir>]"),
> +	N_("git commit-graph --write [--pack-dir <packdir>] [--update-head]"),
>  	NULL
>  };
>  
> @@ -20,6 +20,9 @@ static struct opts_commit_graph {
>  	int read;
>  	const char *graph_hash;
>  	int write;
> +	int update_head;
> +	int has_existing;
> +	struct object_id old_graph_hash;
>  } opts;
>  
>  static int graph_read(void)
> @@ -30,8 +33,8 @@ static int graph_read(void)
>  
>  	if (opts.graph_hash && strlen(opts.graph_hash) == GIT_MAX_HEXSZ)
>  		get_oid_hex(opts.graph_hash, &graph_hash);
> -	else
> -		die("no graph hash specified");
> +	else if (!get_graph_head_hash(opts.pack_dir, &graph_hash))
> +		die("no graph-head exists");
>  
>  	graph_file = get_commit_graph_filename_hash(opts.pack_dir, &graph_hash);
>  	graph = load_commit_graph_one(graph_file, opts.pack_dir);
> @@ -62,10 +65,33 @@ static int graph_read(void)
>  	return 0;
>  }
>  
> +static void update_head_file(const char *pack_dir, const struct object_id *graph_hash)
> +{
> +	struct strbuf head_path = STRBUF_INIT;
> +	int fd;
> +	struct lock_file lk = LOCK_INIT;
> +
> +	strbuf_addstr(&head_path, pack_dir);
> +	strbuf_addstr(&head_path, "/");
> +	strbuf_addstr(&head_path, "graph-head");
strbuf_addstr(&head_path, "/graph-head"); ?
Show 9 quoted lines
> +
> +	fd = hold_lock_file_for_update(&lk, head_path.buf, LOCK_DIE_ON_ERROR);
> +	strbuf_release(&head_path);
> +
> +	if (fd < 0)
> +		die_errno("unable to open graph-head");
> +
> +	write_in_full(fd, oid_to_hex(graph_hash), GIT_MAX_HEXSZ);
> +	commit_lock_file(&lk);

The new graph-head file will be writable. All other files in .git/objects/pack are created read-only, including graph files. Just pointing it out, but I don't think it's a bit deal; other than consistency with the permissions of other files I don't have any argument for making it read-only.

Show 5 quoted lines
> +}
> +
>  static int graph_write(void)
>  {
>  	struct object_id *graph_hash = construct_commit_graph(opts.pack_dir);
First the new graph file is written ...
>  
> +	if (opts.update_head)
> +		update_head_file(opts.pack_dir, graph_hash);

... and then the new graph head, good. There could be a race if it were the other way around.

Show 26 quoted lines
> +
>  	if (graph_hash)
>  		printf("%s\n", oid_to_hex(graph_hash));
>  
> @@ -83,6 +109,8 @@ int cmd_commit_graph(int argc, const char **argv, const char *prefix)
>  			N_("read graph file")),
>  		OPT_BOOL('w', "write", &opts.write,
>  			N_("write commit graph file")),
> +		OPT_BOOL('u', "update-head", &opts.update_head,
> +			N_("update graph-head to written graph file")),
>  		{ OPTION_STRING, 'H', "graph-hash", &opts.graph_hash,
>  			N_("hash"),
>  			N_("A hash for a specific graph file in the pack-dir."),
> @@ -109,10 +137,14 @@ int cmd_commit_graph(int argc, const char **argv, const char *prefix)
>  		opts.pack_dir = strbuf_detach(&path, NULL);
>  	}
>  
> +	opts.has_existing = !!get_graph_head_hash(opts.pack_dir, &opts.old_graph_hash);
> +
>  	if (opts.read)
>  		return graph_read();
>  	if (opts.write)
>  		return graph_write();
>  
> +	if (opts.has_existing)
> +		printf("%s\n", oid_to_hex(&opts.old_graph_hash));

It seems that a command like 'git commit-graph --read --update-head' succeeds and '--update-head' has no effect. I think it should error out. 'git commit-graph --update-head' doesn't complain, either.

Would it be more appropriate to have 'read' and 'write' subcommands instead of '--read' and '--write' options? Then parse-options alone would take care of a command line like 'git commit-graph read --update-index' and error out because of unrecognized option.

Show 83 quoted lines
>  	return 0;
>  }
> diff --git a/commit-graph.c b/commit-graph.c
> index 622a650259..764e016ddb 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -35,6 +35,31 @@
>  #define GRAPH_MIN_SIZE (GRAPH_CHUNKLOOKUP_SIZE + GRAPH_FANOUT_SIZE + \
>  			GRAPH_OID_LEN + sizeof(struct commit_graph_header))
>  
> +struct object_id *get_graph_head_hash(const char *pack_dir, struct object_id *hash)
> +{
> +	struct strbuf head_filename = STRBUF_INIT;
> +	char hex[GIT_MAX_HEXSZ + 1];
> +	FILE *f;
> +
> +	strbuf_addstr(&head_filename, pack_dir);
> +	strbuf_addstr(&head_filename, "/graph-head");
> +
> +	f = fopen(head_filename.buf, "r");
> +	strbuf_release(&head_filename);
> +
> +	if (!f)
> +		return 0;
> +
> +	if (!fgets(hex, sizeof(hex), f))
> +		die("failed to read graph-head");
> +
> +	fclose(f);
> +
> +	if (get_oid_hex(hex, hash))
> +		return 0;
> +	return hash;
> +}
> +
>  char* get_commit_graph_filename_hash(const char *pack_dir,
>  				     struct object_id *hash)
>  {
> diff --git a/commit-graph.h b/commit-graph.h
> index e046ae575c..43eb0aec84 100644
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -4,6 +4,8 @@
>  #include "git-compat-util.h"
>  #include "commit.h"
>  
> +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);
>  
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index da565624e3..d1a23bcdaf 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -13,7 +13,8 @@ test_expect_success 'setup full repo' \
>       packdir=".git/objects/pack"'
>  
>  test_expect_success 'write graph with no packs' \
> -    'git commit-graph --write --pack-dir .'
> +    'git commit-graph --write --pack-dir . &&
> +     test_path_is_missing graph-head'
>  
>  test_expect_success 'create commits and repack' \
>      'for i in $(test_seq 5)
> @@ -37,6 +38,7 @@ EOF
>  test_expect_success 'write graph' \
>      'graph1=$(git commit-graph --write) &&
>       test_path_is_file ${packdir}/graph-${graph1}.graph &&
> +     test_path_is_missing ${packdir}/graph-head &&
>       git commit-graph --read --graph-hash=${graph1} >output &&
>       _graph_read_expect "5" "${packdir}" &&
>       cmp expect output'
> @@ -90,8 +92,11 @@ test_expect_success 'Add more commits' \
>  # 1
>  
>  test_expect_success 'write graph with merges' \
> -    'graph2=$(git commit-graph --write) &&
> +    'graph2=$(git commit-graph --write --update-head) &&
>       test_path_is_file ${packdir}/graph-${graph2}.graph &&
> +     test_path_is_file ${packdir}/graph-head &&
> +     echo ${graph2} >expect &&
> +     cmp -n 40 expect ${packdir}/graph-head &&

This check is fishy, and that '-n 40' will need adjustment once we migrate to a longer hash function. I presume you used it, because 'graph-head' contains only 40 hexdigits without a trailing newline, but 'expect' created with 'echo' does contain a newline as well, right? Then this would be better instead:

  printf $graph2 >expect &&
  test_cmp expect $packdir/graph-head &&
Show 10 quoted lines
>       git commit-graph --read --graph-hash=${graph2} >output &&
>       _graph_read_expect "18" "${packdir}" &&
>       cmp expect output'
> @@ -107,6 +112,9 @@ test_expect_success 'setup bare repo' \
>  test_expect_success 'write graph in bare repo' \
>      'graphbare=$(git commit-graph --write) &&
>       test_path_is_file ${baredir}/graph-${graphbare}.graph &&
> +     test_path_is_file ${baredir}/graph-head &&
> +     echo ${graphbare} >expect &&
> +     cmp -n 40 expect ${baredir}/graph-head &&
Likewise.
Show 5 quoted lines
>       git commit-graph --read --graph-hash=${graphbare} >output &&
>       _graph_read_expect "18" "${baredir}" &&
>       cmp expect output'
> -- 
> 2.16.0.15.g9c3cf44.dirty
Previous: Derrick StoleeNext: Derrick Stolee
Message 38 of 146 in “Serialized Git Commit Graph”
  1. 00/14 Serialized Git Commit GraphDerrick Stolee, Jan 30, 2018
  2. 01/14 commit-graph: add format documentDerrick Stolee, Jan 30, 2018
  3. Jonathan TanFeb 1, 2018
  4. 03/14 commit-graph: create git-commit-graph builtinDerrick Stolee, Jan 30, 2018
  5. SZEDER GáborFeb 2, 2018
  6. 06/14 commit-graph: implement git-commit-graph --readDerrick Stolee, Jan 30, 2018
  7. Stefan BellerJan 31, 2018
  8. SZEDER GáborFeb 2, 2018
  9. Jonathan TanFeb 2, 2018
  10. Derrick StoleeFeb 5, 2018
  11. 08/14 commit-graph: implement git-commit-graph --clearDerrick Stolee, Jan 30, 2018
  12. SZEDER GáborFeb 2, 2018
  13. 11/14 commit: integrate commit graph with commit parsingDerrick Stolee, Jan 30, 2018
  14. Jonathan TanFeb 2, 2018
  15. Derrick StoleeFeb 6, 2018
  16. 13/14 commit-graph: close under reachabilityDerrick Stolee, Jan 30, 2018
  17. 09/14 commit-graph: teach git-commit-graph --delete-expiredDerrick Stolee, Jan 30, 2018
  18. SZEDER GáborFeb 2, 2018
  19. 12/14 commit-graph: read only from specific pack-indexesDerrick Stolee, Jan 30, 2018
  20. 14/14 commit-graph: build graph from starting commitsDerrick Stolee, Jan 30, 2018
  21. 10/14 commit-graph: add core.commitgraph settingDerrick Stolee, Jan 30, 2018
  22. Igor DjordjevicJan 31, 2018
  23. SZEDER GáborFeb 2, 2018
  24. 02/14 graph: add commit graph design documentDerrick Stolee, Jan 30, 2018
  25. Stefan BellerJan 31, 2018
  26. 05/14 commit-graph: implement git-commit-graph --writeDerrick Stolee, Jan 30, 2018
  27. Jonathan TanFeb 1, 2018
  28. Stefan BellerFeb 2, 2018
  29. Junio C HamanoFeb 2, 2018
  30. Derrick StoleeFeb 3, 2018
  31. Jeff KingFeb 3, 2018
  32. Junio C HamanoFeb 5, 2018
  33. Derrick StoleeFeb 6, 2018
  34. SZEDER GáborFeb 1, 2018
  35. Derrick StoleeFeb 5, 2018
  36. SZEDER GáborFeb 2, 2018
  37. 07/14 commit-graph: implement git-commit-graph --update-headDerrick Stolee, Jan 30, 2018
  38. SZEDER GáborFeb 2, 2018
  39. Derrick StoleeFeb 5, 2018
  40. SZEDER GáborFeb 2, 2018
  41. 04/14 commit-graph: implement construct_commit_graph()Derrick Stolee, Jan 30, 2018
  42. Jonathan TanFeb 1, 2018
  43. SZEDER GáborFeb 1, 2018
  44. SZEDER GáborFeb 2, 2018
  45. Derrick StoleeFeb 5, 2018
  46. SZEDER GáborFeb 7, 2018
  47. Derrick StoleeFeb 7, 2018
  48. Stefan BellerJan 30, 2018
  49. Stefan BellerFeb 1, 2018
  50. 00/14 Serialized Git Commit GraphDerrick Stolee, Feb 8, 2018
  51. 01/14 commit-graph: add format documentDerrick Stolee, Feb 8, 2018
  52. Junio C HamanoFeb 8, 2018
  53. Derrick StoleeFeb 8, 2018
  54. Junio C HamanoFeb 8, 2018
  55. 03/14 commit-graph: create git-commit-graph builtinDerrick Stolee, Feb 8, 2018
  56. Junio C HamanoFeb 8, 2018
  57. Derrick StoleeFeb 8, 2018
  58. Junio C HamanoFeb 8, 2018
  59. 07/14 commit-graph: update graph-head during writeDerrick Stolee, Feb 8, 2018
  60. Junio C HamanoFeb 12, 2018
  61. Junio C HamanoFeb 12, 2018
  62. Derrick StoleeFeb 12, 2018
  63. Jonathan TanFeb 13, 2018
  64. 10/14 commit-graph: add core.commitGraph settingDerrick Stolee, Feb 8, 2018
  65. 12/14 commit-graph: close under reachabilityDerrick Stolee, Feb 8, 2018
  66. 11/14 commit: integrate commit graph with commit parsingDerrick Stolee, Feb 8, 2018
  67. Jonathan TanFeb 14, 2018
  68. Derrick StoleeFeb 14, 2018
  69. Junio C HamanoFeb 15, 2018
  70. 05/14 commit-graph: implement 'git-commit-graph write'Derrick Stolee, Feb 8, 2018
  71. Jonathan TanFeb 13, 2018
  72. 08/14 commit-graph: implement 'git-commit-graph clear'Derrick Stolee, Feb 8, 2018
  73. Jonathan TanFeb 13, 2018
  74. 09/14 commit-graph: implement --delete-expiredDerrick Stolee, Feb 8, 2018
  75. 13/14 commit-graph: read only from specific pack-indexesDerrick Stolee, Feb 8, 2018
  76. 14/14 commit-graph: build graph from starting commitsDerrick Stolee, Feb 8, 2018
  77. SZEDER GáborFeb 9, 2018
  78. Derrick StoleeFeb 9, 2018
  79. 06/14 commit-graph: implement 'git-commit-graph read'Derrick Stolee, Feb 8, 2018
  80. Junio C HamanoFeb 8, 2018
  81. 02/14 graph: add commit graph design documentDerrick Stolee, Feb 8, 2018
  82. 04/14 commit-graph: implement write_commit_graph()Derrick Stolee, Feb 8, 2018
  83. Junio C HamanoFeb 8, 2018
  84. Junio C HamanoFeb 15, 2018
  85. Derrick StoleeFeb 15, 2018
  86. Derrick StoleeFeb 14, 2018
  87. Stefan BellerFeb 14, 2018
  88. Derrick StoleeFeb 14, 2018
  89. 00/13 Serialized Git Commit GraphDerrick Stolee, Feb 19, 2018
  90. 01/13 commit-graph: add format documentDerrick Stolee, Feb 19, 2018
  91. Junio C HamanoFeb 20, 2018
  92. Stefan BellerFeb 21, 2018
  93. Derrick StoleeFeb 21, 2018
  94. Stefan BellerFeb 21, 2018
  95. Jakub NarebskiMar 30, 2018
  96. Derrick StoleeApr 2, 2018
  97. Jakub NarebskiApr 2, 2018
  98. 03/13 commit-graph: create git-commit-graph builtinDerrick Stolee, Feb 19, 2018
  99. Junio C HamanoFeb 20, 2018
  100. Junio C HamanoFeb 21, 2018
  101. Derrick StoleeFeb 23, 2018
  102. SZEDER GáborFeb 26, 2018
  103. Derrick StoleeFeb 26, 2018
  104. 06/13 commit-graph: implement git commit-graph readDerrick Stolee, Feb 19, 2018
  105. Junio C HamanoFeb 21, 2018
  106. Junio C HamanoFeb 22, 2018
  107. 08/13 commit-graph: implement --delete-expiredDerrick Stolee, Feb 19, 2018
  108. Stefan BellerFeb 21, 2018
  109. Derrick StoleeFeb 23, 2018
  110. Junio C HamanoFeb 22, 2018
  111. Derrick StoleeFeb 23, 2018
  112. Junio C HamanoFeb 23, 2018
  113. Derrick StoleeFeb 23, 2018
  114. Junio C HamanoFeb 23, 2018
  115. 09/13 commit-graph: add core.commitGraph settingDerrick Stolee, Feb 19, 2018
  116. 10/13 commit-graph: close under reachabilityDerrick Stolee, Feb 19, 2018
  117. 11/13 commit: integrate commit graph with commit parsingDerrick Stolee, Feb 19, 2018
  118. 12/13 commit-graph: read only from specific pack-indexesDerrick Stolee, Feb 19, 2018
  119. Stefan BellerFeb 21, 2018
  120. Derrick StoleeFeb 23, 2018
  121. 13/13 commit-graph: build graph from starting commitsDerrick Stolee, Feb 19, 2018
  122. 07/13 commit-graph: implement --set-latestDerrick Stolee, Feb 19, 2018
  123. Junio C HamanoFeb 22, 2018
  124. Derrick StoleeFeb 23, 2018
  125. 02/13 graph: add commit graph design documentDerrick Stolee, Feb 19, 2018
  126. Junio C HamanoFeb 20, 2018
  127. Derrick StoleeFeb 23, 2018
  128. Stefan BellerFeb 21, 2018
  129. 04/13 commit-graph: implement write_commit_graph()Derrick Stolee, Feb 19, 2018
  130. Junio C HamanoFeb 20, 2018
  131. Derrick StoleeFeb 23, 2018
  132. Junio C HamanoFeb 23, 2018
  133. Junio C HamanoFeb 23, 2018
  134. Derrick StoleeFeb 23, 2018
  135. SZEDER GáborFeb 26, 2018
  136. Junio C HamanoFeb 28, 2018
  137. 05/13 commit-graph: implement 'git-commit-graph write'Derrick Stolee, Feb 19, 2018
  138. Junio C HamanoFeb 21, 2018
  139. Jakub NarebskiMar 30, 2018
  140. Derrick StoleeApr 2, 2018
  141. Jakub NarebskiApr 2, 2018
  142. Derrick StoleeApr 2, 2018
  143. Stefan BellerApr 2, 2018
  144. Derrick StoleeApr 2, 2018
  145. Stefan BellerApr 2, 2018
  146. Jakub NarebskiApr 7, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.