{"thread":{"id":"52245","subject":"[PATCH 0/1] Move 'git commit-graph read' subcommand to test helper","startedAt":"2019-11-12T16:58:25Z","lastAt":"2019-11-13T05:31:54Z","messageCount":3,"participants":["Derrick Stolee via GitGitGadget","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"386023","messageId":"pull.460.git.1573577900.gitgitgadget@gmail.com","threadId":"52245","inReplyTo":null,"subject":"[PATCH 0/1] Move 'git commit-graph read' subcommand to test helper","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-12T16:58:19Z","receivedAt":"2019-11-12T16:58:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"As discussed [1], the 'git commit-graph read' subcommand has little value to\nusers and should instead live in a test helper. The implementation was able\nto move almost wholesale, with only some new code to find the default object\ndirectory. This was necessary because of some tests that use a bare\nrepository.\n\nThanks, -Stolee\n\n[1] \nhttps://public-inbox.org/git/CAGyf7-G3NDp--2nUbri_0EqvSLF21M0gsFCOKDCWMY+e68Htog@mail.gmail.com/\n\nDerrick Stolee (1):\n  test-tool: use 'read-graph' helper\n\n Documentation/git-commit-graph.txt | 12 ------\n Makefile                           |  1 +\n builtin/commit-graph.c             | 68 ------------------------------\n t/helper/test-read-graph.c         | 53 +++++++++++++++++++++++\n t/helper/test-tool.c               |  1 +\n t/helper/test-tool.h               |  1 +\n t/t5318-commit-graph.sh            |  2 +-\n t/t5324-split-commit-graph.sh      |  2 +-\n 8 files changed, 58 insertions(+), 82 deletions(-)\n create mode 100644 t/helper/test-read-graph.c\n\n\nbase-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-460%2Fderrickstolee%2Fcommit-graph-read-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-460/derrickstolee/commit-graph-read-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/460\n-- \ngitgitgadget\n"},{"id":"386024","messageId":"bdbc7b71884cba192c67c2bc26db5b9b3a040043.1573577900.git.gitgitgadget@gmail.com","threadId":"52245","inReplyTo":"pull.460.git.1573577900.gitgitgadget@gmail.com","subject":"[PATCH 1/1] test-tool: use 'read-graph' helper","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-12T16:58:20Z","receivedAt":"2019-11-12T16:58:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThe 'git commit-graph read' subcommand is used in test scripts to check\nthat the commit-graph contents match the expected data. Mostly, this\nhelps check the header information and the list of chunks. Users do not\nneed this information, so move the functionality to a test helper.\n\nReported-by: Bryan Turner <bturner@atlassian.com>\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n Documentation/git-commit-graph.txt | 12 ------\n Makefile                           |  1 +\n builtin/commit-graph.c             | 68 ------------------------------\n t/helper/test-read-graph.c         | 53 +++++++++++++++++++++++\n t/helper/test-tool.c               |  1 +\n t/helper/test-tool.h               |  1 +\n t/t5318-commit-graph.sh            |  2 +-\n t/t5324-split-commit-graph.sh      |  2 +-\n 8 files changed, 58 insertions(+), 82 deletions(-)\n create mode 100644 t/helper/test-read-graph.c\n\ndiff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt\nindex 8c708a7a16..bcd85c1976 100644\n--- a/Documentation/git-commit-graph.txt\n+++ b/Documentation/git-commit-graph.txt\n@@ -9,7 +9,6 @@ git-commit-graph - Write and verify Git commit-graph files\n SYNOPSIS\n --------\n [verse]\n-'git commit-graph read' [--object-dir <dir>]\n 'git commit-graph verify' [--object-dir <dir>] [--shallow] [--[no-]progress]\n 'git commit-graph write' <options> [--object-dir <dir>] [--[no-]progress]\n \n@@ -74,11 +73,6 @@ Finally, if `--expire-time=<datetime>` is not specified, let `datetime`\n be the current time. After writing the split commit-graph, delete all\n unused commit-graph whose modified times are older than `datetime`.\n \n-'read'::\n-\n-Read the commit-graph file and output basic details about it.\n-Used for debugging purposes.\n-\n 'verify'::\n \n Read the commit-graph file and verify its contents against the object\n@@ -118,12 +112,6 @@ $ git show-ref -s | git commit-graph write --stdin-commits\n $ git rev-parse HEAD | git commit-graph write --stdin-commits --append\n ------------------------------------------------\n \n-* Read basic information from the commit-graph file.\n-+\n-------------------------------------------------\n-$ git commit-graph read\n-------------------------------------------------\n-\n \n GIT\n ---\ndiff --git a/Makefile b/Makefile\nindex 58b92af54b..44997f6f57 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -727,6 +727,7 @@ TEST_BUILTINS_OBJS += test-prio-queue.o\n TEST_BUILTINS_OBJS += test-progress.o\n TEST_BUILTINS_OBJS += test-reach.o\n TEST_BUILTINS_OBJS += test-read-cache.o\n+TEST_BUILTINS_OBJS += test-read-graph.o\n TEST_BUILTINS_OBJS += test-read-midx.o\n TEST_BUILTINS_OBJS += test-ref-store.o\n TEST_BUILTINS_OBJS += test-regex.o\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex ec0fc93d39..e0c6fc4bbf 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -8,7 +8,6 @@\n #include \"object-store.h\"\n \n static char const * const builtin_commit_graph_usage[] = {\n-\tN_(\"git commit-graph read [--object-dir <objdir>]\"),\n \tN_(\"git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]\"),\n \tN_(\"git commit-graph write [--object-dir <objdir>] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] [--[no-]progress] <split options>\"),\n \tNULL\n@@ -19,11 +18,6 @@ static const char * const builtin_commit_graph_verify_usage[] = {\n \tNULL\n };\n \n-static const char * const builtin_commit_graph_read_usage[] = {\n-\tN_(\"git commit-graph read [--object-dir <objdir>]\"),\n-\tNULL\n-};\n-\n static const char * const builtin_commit_graph_write_usage[] = {\n \tN_(\"git commit-graph write [--object-dir <objdir>] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] [--[no-]progress] <split options>\"),\n \tNULL\n@@ -93,66 +87,6 @@ static int graph_verify(int argc, const char **argv)\n \treturn verify_commit_graph(the_repository, graph, flags);\n }\n \n-static int graph_read(int argc, const char **argv)\n-{\n-\tstruct commit_graph *graph = NULL;\n-\tchar *graph_name;\n-\tint open_ok;\n-\tint fd;\n-\tstruct stat st;\n-\n-\tstatic struct option builtin_commit_graph_read_options[] = {\n-\t\tOPT_STRING(0, \"object-dir\", &opts.obj_dir,\n-\t\t\tN_(\"dir\"),\n-\t\t\tN_(\"The object directory to store the graph\")),\n-\t\tOPT_END(),\n-\t};\n-\n-\ttrace2_cmd_mode(\"read\");\n-\n-\targc = parse_options(argc, argv, NULL,\n-\t\t\t     builtin_commit_graph_read_options,\n-\t\t\t     builtin_commit_graph_read_usage, 0);\n-\n-\tif (!opts.obj_dir)\n-\t\topts.obj_dir = get_object_directory();\n-\n-\tgraph_name = get_commit_graph_filename(opts.obj_dir);\n-\n-\topen_ok = open_commit_graph(graph_name, &fd, &st);\n-\tif (!open_ok)\n-\t\tdie_errno(_(\"Could not open commit-graph '%s'\"), graph_name);\n-\n-\tgraph = load_commit_graph_one_fd_st(fd, &st);\n-\tif (!graph)\n-\t\treturn 1;\n-\n-\tFREE_AND_NULL(graph_name);\n-\n-\tprintf(\"header: %08x %d %d %d %d\\n\",\n-\t\tntohl(*(uint32_t*)graph->data),\n-\t\t*(unsigned char*)(graph->data + 4),\n-\t\t*(unsigned char*)(graph->data + 5),\n-\t\t*(unsigned char*)(graph->data + 6),\n-\t\t*(unsigned char*)(graph->data + 7));\n-\tprintf(\"num_commits: %u\\n\", graph->num_commits);\n-\tprintf(\"chunks:\");\n-\n-\tif (graph->chunk_oid_fanout)\n-\t\tprintf(\" oid_fanout\");\n-\tif (graph->chunk_oid_lookup)\n-\t\tprintf(\" oid_lookup\");\n-\tif (graph->chunk_commit_data)\n-\t\tprintf(\" commit_metadata\");\n-\tif (graph->chunk_extra_edges)\n-\t\tprintf(\" extra_edges\");\n-\tprintf(\"\\n\");\n-\n-\tUNLEAK(graph);\n-\n-\treturn 0;\n-}\n-\n extern int read_replace_refs;\n static struct split_commit_graph_opts split_opts;\n \n@@ -268,8 +202,6 @@ int cmd_commit_graph(int argc, const char **argv, const char *prefix)\n \tsave_commit_buffer = 0;\n \n \tif (argc > 0) {\n-\t\tif (!strcmp(argv[0], \"read\"))\n-\t\t\treturn graph_read(argc, argv);\n \t\tif (!strcmp(argv[0], \"verify\"))\n \t\t\treturn graph_verify(argc, argv);\n \t\tif (!strcmp(argv[0], \"write\"))\ndiff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c\nnew file mode 100644\nindex 0000000000..d2884efe0a\n--- /dev/null\n+++ b/t/helper/test-read-graph.c\n@@ -0,0 +1,53 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"commit-graph.h\"\n+#include \"repository.h\"\n+#include \"object-store.h\"\n+\n+int cmd__read_graph(int argc, const char **argv)\n+{\n+\tstruct commit_graph *graph = NULL;\n+\tchar *graph_name;\n+\tint open_ok;\n+\tint fd;\n+\tstruct stat st;\n+\tconst char *object_dir;\n+\n+\tsetup_git_directory();\n+\tobject_dir = get_object_directory();\n+\n+\tgraph_name = get_commit_graph_filename(object_dir);\n+\n+\topen_ok = open_commit_graph(graph_name, &fd, &st);\n+\tif (!open_ok)\n+\t\tdie_errno(_(\"Could not open commit-graph '%s'\"), graph_name);\n+\n+\tgraph = load_commit_graph_one_fd_st(fd, &st);\n+\tif (!graph)\n+\t\treturn 1;\n+\n+\tFREE_AND_NULL(graph_name);\n+\n+\tprintf(\"header: %08x %d %d %d %d\\n\",\n+\t\tntohl(*(uint32_t*)graph->data),\n+\t\t*(unsigned char*)(graph->data + 4),\n+\t\t*(unsigned char*)(graph->data + 5),\n+\t\t*(unsigned char*)(graph->data + 6),\n+\t\t*(unsigned char*)(graph->data + 7));\n+\tprintf(\"num_commits: %u\\n\", graph->num_commits);\n+\tprintf(\"chunks:\");\n+\n+\tif (graph->chunk_oid_fanout)\n+\t\tprintf(\" oid_fanout\");\n+\tif (graph->chunk_oid_lookup)\n+\t\tprintf(\" oid_lookup\");\n+\tif (graph->chunk_commit_data)\n+\t\tprintf(\" commit_metadata\");\n+\tif (graph->chunk_extra_edges)\n+\t\tprintf(\" extra_edges\");\n+\tprintf(\"\\n\");\n+\n+\tUNLEAK(graph);\n+\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 19ee26d931..f20989d449 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -45,6 +45,7 @@ static struct test_cmd cmds[] = {\n \t{ \"progress\", cmd__progress },\n \t{ \"reach\", cmd__reach },\n \t{ \"read-cache\", cmd__read_cache },\n+\t{ \"read-graph\", cmd__read_graph },\n \t{ \"read-midx\", cmd__read_midx },\n \t{ \"ref-store\", cmd__ref_store },\n \t{ \"regex\", cmd__regex },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex c2aa56ef50..8ed2af71d1 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -35,6 +35,7 @@ int cmd__prio_queue(int argc, const char **argv);\n int cmd__progress(int argc, const char **argv);\n int cmd__reach(int argc, const char **argv);\n int cmd__read_cache(int argc, const char **argv);\n+int cmd__read_graph(int argc, const char **argv);\n int cmd__read_midx(int argc, const char **argv);\n int cmd__ref_store(int argc, const char **argv);\n int cmd__regex(int argc, const char **argv);\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex d42b3efe39..798968374f 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -85,7 +85,7 @@ graph_read_expect() {\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \tEOF\n-\tgit commit-graph read >output &&\n+\ttest-tool read-graph >output &&\n \ttest_cmp expect output\n }\n \ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 115aabd141..c24823431f 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -25,7 +25,7 @@ graph_read_expect() {\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata\n \tEOF\n-\tgit commit-graph read >output &&\n+\ttest-tool read-graph >output &&\n \ttest_cmp expect output\n }\n \n-- \ngitgitgadget\n"},{"id":"386095","messageId":"20191113053151.GC3547@sigill.intra.peff.net","threadId":"52245","inReplyTo":"bdbc7b71884cba192c67c2bc26db5b9b3a040043.1573577900.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] test-tool: use 'read-graph' helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-13T05:31:51Z","receivedAt":"2019-11-13T05:31:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 12, 2019 at 04:58:20PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee <dstolee@microsoft.com>\n> \n> The 'git commit-graph read' subcommand is used in test scripts to check\n> that the commit-graph contents match the expected data. Mostly, this\n> helps check the header information and the list of chunks. Users do not\n> need this information, so move the functionality to a test helper.\n\nThis looks good to me.\n\n>  t/helper/test-read-graph.c         | 53 +++++++++++++++++++++++\n\nI wondered if we might want a commit-graph test-tool with sub-commands.\nBut it probably doesn't matter. This would be the only thing in it. And\nthere's less need these days to keep the number of tools down, since\nthey're all linked into the same binary. And we can always change it\nlater, since it's not a public interface.\n\nSo this is the simplest thing, which is probably best for now.\n\n-Peff\n"}]}