{"thread":{"id":"49244","subject":"[PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","startedAt":"2018-08-28T20:33:26Z","lastAt":"2018-10-09T05:53:36Z","messageCount":18,"participants":["Derrick Stolee via GitGitGadget","Stefan Beller","Derrick Stolee","Eric Sunshine","Duy Nguyen","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"356734","messageId":"pull.26.git.gitgitgadget@gmail.com","threadId":"49244","inReplyTo":null,"subject":"[PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-28T20:33:22Z","receivedAt":"2018-08-28T20:33:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The commit-graph (and multi-pack-index) features are optional data\nstructures that can make Git operations faster. Since they are optional, we\ndo not enable them in most Git tests. The commit-graph is tested in\nt5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\none script cannot cover the data shapes present in the rest of the test\nsuite.\n\nThis patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n. Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\nwith every git commit command. Thanks, Duy, for pointing out this direction\n[1].\n\nA few tests needed to be modified. These are the same tests that were\nmentioned in my previous example patch [2].\n\nWhen this merges down, I'll create a CI build in VSTS that runs the test\nsuite with this option enabled.\n\nThanks, -Stolee\n\n[1] \nhttps://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/\nRe: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment\nvariables)\n\n[2] \nhttps://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/\n[PATCH] DO-NOT-MERGE: write and read commit-graph always\n\nBased-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.com\n\nDerrick Stolee (1):\n  commit-graph: define GIT_TEST_COMMIT_GRAPH\n\n builtin/commit.c                    |  4 ++++\n commit-graph.c                      |  5 +++--\n commit-graph.h                      |  2 ++\n t/README                            |  4 ++++\n t/t0410-partial-clone.sh            |  2 +-\n t/t5307-pack-missing-commit.sh      | 10 ++++++++--\n t/t6011-rev-list-with-bad-commit.sh | 10 ++++++----\n t/t6024-recursive-merge.sh          |  9 ++++++---\n 8 files changed, 34 insertions(+), 12 deletions(-)\n\n\nbase-commit: 829a321569d8e8f2c582aef9f0c990df976ab842\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/26\n-- \ngitgitgadget\n"},{"id":"356735","messageId":"85d02ac8d8c9a8950ce1a9760a541ff506945de0.1535488400.git.gitgitgadget@gmail.com","threadId":"49244","inReplyTo":"pull.26.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-28T20:33:23Z","receivedAt":"2018-08-28T20:33:32Z","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 commit-graph feature is tested in isolation by\nt5318-commit-graph.sh and t6600-test-reach.sh, but there are many\nmore interesting scenarios involving commit walks. Many of these\nscenarios are covered by the existing test suite, but we need to\nmaintain coverage when the optional commit-graph structure is not\npresent.\n\nTo allow running the full test suite with the commit-graph present,\nadd a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\nto GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\nto load the commit-graph when parsing commits, and writes the\ncommit-graph file after every 'git commit' command.\n\nThere are a few tests that rely on commits not existing in\npack-files to trigger important events, so manually set\nGIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n\nThere is one test in t6024-recursive-merge.sh that relies on the\nmerge-base algorithm picking one of two ambiguous merge-bases, and\nthe commit-graph feature changes which merge-base is picked.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n builtin/commit.c                    |  4 ++++\n commit-graph.c                      |  5 +++--\n commit-graph.h                      |  2 ++\n t/README                            |  4 ++++\n t/t0410-partial-clone.sh            |  2 +-\n t/t5307-pack-missing-commit.sh      | 10 ++++++++--\n t/t6011-rev-list-with-bad-commit.sh | 10 ++++++----\n t/t6024-recursive-merge.sh          |  9 ++++++---\n 8 files changed, 34 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 158e3f843a..a25e652102 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -33,6 +33,7 @@\n #include \"sequencer.h\"\n #include \"mailmap.h\"\n #include \"help.h\"\n+#include \"commit-graph.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [<options>] [--] <pathspec>...\"),\n@@ -1651,6 +1652,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"new_index file. Check that disk is not full and quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n+\tif (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))\n+\t\twrite_commit_graph_reachable(get_object_directory(), 0);\n+\n \trerere(0);\n \trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n \trun_commit_hook(use_editor, get_index_file(), \"post-commit\", NULL);\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 4bd1a4abbf..5cae83fe03 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -237,8 +237,9 @@ static int prepare_commit_graph(struct repository *r)\n \t\treturn !!r->objects->commit_graph;\n \tr->objects->commit_graph_attempted = 1;\n \n-\tif (repo_config_get_bool(r, \"core.commitgraph\", &config_value) ||\n-\t    !config_value)\n+\tif (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&\n+\t    (repo_config_get_bool(r, \"core.commitgraph\", &config_value) ||\n+\t    !config_value))\n \t\t/*\n \t\t * This repository is not configured to use commit graphs, so\n \t\t * do not load one. (But report commit_graph_attempted anyway\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 13d736cdde..cf9141f356 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -5,6 +5,8 @@\n #include \"repository.h\"\n #include \"string-list.h\"\n \n+#define GIT_TEST_COMMIT_GRAPH \"GIT_TEST_COMMIT_GRAPH\"\n+\n struct commit;\n \n char *get_commit_graph_filename(const char *obj_dir);\ndiff --git a/t/README b/t/README\nindex 8373a27fea..2b90c433f5 100644\n--- a/t/README\n+++ b/t/README\n@@ -315,6 +315,10 @@ packs on demand. This normally only happens when the object size is\n over 2GB. This variable forces the code path on any object larger than\n <n> bytes.\n \n+GIT_TEST_COMMIT_GRAPH=<boolean>, when true, forces the commit-graph to\n+be written after every 'git commit' command, and overrides the\n+'core.commitGraph' setting to true.\n+\n Naming Tests\n ------------\n \ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 4984ca583d..73d5284a91 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n \tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n-\tgit -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n+\tGIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n \tgrep $(git -C repo rev-parse bar) out &&\n \t! grep $FOO out\n '\ndiff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh\nindex ae52a1882d..15df19a040 100755\n--- a/t/t5307-pack-missing-commit.sh\n+++ b/t/t5307-pack-missing-commit.sh\n@@ -24,11 +24,17 @@ test_expect_success 'check corruption' '\n '\n \n test_expect_success 'rev-list notices corruption (1)' '\n-\ttest_must_fail git rev-list HEAD\n+\t(\n+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n+\t\ttest_must_fail git rev-list HEAD\n+\t)\n '\n \n test_expect_success 'rev-list notices corruption (2)' '\n-\ttest_must_fail git rev-list --objects HEAD\n+\t(\n+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n+\t\ttest_must_fail git rev-list --objects HEAD\n+\t)\n '\n \n test_expect_success 'pack-objects notices corruption' '\ndiff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh\nindex e51eb41f4b..d774c69871 100755\n--- a/t/t6011-rev-list-with-bad-commit.sh\n+++ b/t/t6011-rev-list-with-bad-commit.sh\n@@ -41,10 +41,12 @@ test_expect_success 'corrupt second commit object' \\\n    test_must_fail git fsck --full\n    '\n \n-test_expect_success 'rev-list should fail' \\\n-   '\n-   test_must_fail git rev-list --all > /dev/null\n-   '\n+test_expect_success 'rev-list should fail' '\n+\t(\n+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n+\t\ttest_must_fail git rev-list --all > /dev/null\n+\t)\n+'\n \n test_expect_success 'git repack _MUST_ fail' \\\n    '\ndiff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh\nindex 3f59e58dfb..04b30c5b76 100755\n--- a/t/t6024-recursive-merge.sh\n+++ b/t/t6024-recursive-merge.sh\n@@ -60,9 +60,12 @@ git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:08\" git commit -m F\n '\n \n-test_expect_success \"combined merge conflicts\" \"\n-\ttest_must_fail git merge -m final G\n-\"\n+test_expect_success 'combined merge conflicts' '\n+\t(\n+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n+\t\ttest_must_fail git merge -m final G\n+\t)\n+'\n \n cat > expect << EOF\n <<<<<<< HEAD\n-- \ngitgitgadget\n"},{"id":"356736","messageId":"CAGZ79kZ=jsCmxKdcPB+Q1__uO8Z2tXx6OkdyTSM8P3QdNGpKQA@mail.gmail.com","threadId":"49244","inReplyTo":"pull.26.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-28T20:37:59Z","receivedAt":"2018-08-28T20:38:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> The commit-graph (and multi-pack-index) features are optional data\n> structures that can make Git operations faster. Since they are optional, we\n> do not enable them in most Git tests. The commit-graph is tested in\n> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\n> one script cannot cover the data shapes present in the rest of the test\n> suite.\n>\n> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\n> with every git commit command.\n\n> Thanks, Duy, for pointing out this direction\n\nDid you mean to cc Duy (instead of me)?\n(I'll happily review the patch, too... just asking)\n\n>\n> A few tests needed to be modified. These are the same tests that were\n> mentioned in my previous example patch [2].\n>\n> When this merges down, I'll create a CI build in VSTS that runs the test\n> suite with this option enabled.\n>\n> Thanks, -Stolee\n>\n> [1]\n> https://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/\n> Re: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment\n> variables)\n>\n> [2]\n> https://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/\n> [PATCH] DO-NOT-MERGE: write and read commit-graph always\n>\n> Based-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.com\n>\n> Derrick Stolee (1):\n>   commit-graph: define GIT_TEST_COMMIT_GRAPH\n>\n>  builtin/commit.c                    |  4 ++++\n>  commit-graph.c                      |  5 +++--\n>  commit-graph.h                      |  2 ++\n>  t/README                            |  4 ++++\n>  t/t0410-partial-clone.sh            |  2 +-\n>  t/t5307-pack-missing-commit.sh      | 10 ++++++++--\n>  t/t6011-rev-list-with-bad-commit.sh | 10 ++++++----\n>  t/t6024-recursive-merge.sh          |  9 ++++++---\n>  8 files changed, 34 insertions(+), 12 deletions(-)\n>\n>\n> base-commit: 829a321569d8e8f2c582aef9f0c990df976ab842\n> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/26\n> --\n> gitgitgadget\n"},{"id":"356737","messageId":"CAGZ79kbsJXF=X7tzpdogAY7LdLd87YkPY0euiYaa3uQKQPoZuQ@mail.gmail.com","threadId":"49244","inReplyTo":"85d02ac8d8c9a8950ce1a9760a541ff506945de0.1535488400.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-28T20:41:50Z","receivedAt":"2018-08-28T20:42:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Derrick Stolee <dstolee@microsoft.com>\n>\n> The commit-graph feature is tested in isolation by\n> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n> more interesting scenarios involving commit walks. Many of these\n> scenarios are covered by the existing test suite, but we need to\n> maintain coverage when the optional commit-graph structure is not\n> present.\n>\n> To allow running the full test suite with the commit-graph present,\n> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n> to load the commit-graph when parsing commits, and writes the\n> commit-graph file after every 'git commit' command.\n>\n> There are a few tests that rely on commits not existing in\n> pack-files to trigger important events, so manually set\n> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n\nSo the plan is to turn on the commit graph for the whole test suite\nexcluding these selected tests?\n\n\n> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\n> index 4984ca583d..73d5284a91 100755\n> --- a/t/t0410-partial-clone.sh\n> +++ b/t/t0410-partial-clone.sh\n> @@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '\n>\n>         git -C repo config core.repositoryformatversion 1 &&\n>         git -C repo config extensions.partialclone \"arbitrary string\" &&\n> -       git -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n> +       GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n\n\n> +               GIT_TEST_COMMIT_GRAPH=0 &&\n> +               test_must_fail git merge -m final G\n\nThis could go on the same line without the && in between, setting the\nvariable as a prefix.\n"},{"id":"356766","messageId":"3bb1d527-87ae-5fdb-be9a-b83f6a68d3a0@gmail.com","threadId":"49244","inReplyTo":"CAGZ79kbsJXF=X7tzpdogAY7LdLd87YkPY0euiYaa3uQKQPoZuQ@mail.gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-08-28T21:31:40Z","receivedAt":"2018-08-28T21:31:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/28/2018 4:41 PM, Stefan Beller wrote:\n> On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> From: Derrick Stolee <dstolee@microsoft.com>\n>>\n>> The commit-graph feature is tested in isolation by\n>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n>> more interesting scenarios involving commit walks. Many of these\n>> scenarios are covered by the existing test suite, but we need to\n>> maintain coverage when the optional commit-graph structure is not\n>> present.\n>>\n>> To allow running the full test suite with the commit-graph present,\n>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n>> to load the commit-graph when parsing commits, and writes the\n>> commit-graph file after every 'git commit' command.\n>>\n>> There are a few tests that rely on commits not existing in\n>> pack-files to trigger important events, so manually set\n>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n> So the plan is to turn on the commit graph for the whole test suite\n> excluding these selected tests?\nExcluding these specific _steps_, but yes.\n>\n>> +               GIT_TEST_COMMIT_GRAPH=0 &&\n>> +               test_must_fail git merge -m final G\n> This could go on the same line without the && in between, setting the\n> variable as a prefix.\n\nIt cannot! The Linux build I ran complained that you can't put \nenvironment variables through test_must_fail.\n\n-Stolee\n\n"},{"id":"356767","messageId":"42231d43-c1f7-592f-cec8-0b07eb9f70bc@gmail.com","threadId":"49244","inReplyTo":"CAGZ79kZ=jsCmxKdcPB+Q1__uO8Z2tXx6OkdyTSM8P3QdNGpKQA@mail.gmail.com","subject":"Re: [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-08-28T21:32:42Z","receivedAt":"2018-08-28T21:32:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/28/2018 4:37 PM, Stefan Beller wrote:\n> On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> The commit-graph (and multi-pack-index) features are optional data\n>> structures that can make Git operations faster. Since they are optional, we\n>> do not enable them in most Git tests. The commit-graph is tested in\n>> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\n>> one script cannot cover the data shapes present in the rest of the test\n>> suite.\n>>\n>> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n>> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\n>> with every git commit command.\n>> Thanks, Duy, for pointing out this direction\n> Did you mean to cc Duy (instead of me)?\n> (I'll happily review the patch, too... just asking)\n\nI just added you because you've been on a lot of commit-graph things \nlately. But yes, I forgot to add Duy to the CC list. Added to this message.\n\nThanks,\n\n-Stolee\n\n"},{"id":"356775","messageId":"CAPig+cSjanDi=jV75PdzYpAjwVgd4Suh3UyvY+Vy7yeHAuY8RA@mail.gmail.com","threadId":"49244","inReplyTo":"3bb1d527-87ae-5fdb-be9a-b83f6a68d3a0@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:59:29Z","receivedAt":"2018-08-28T21:59:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee <stolee@gmail.com> wrote:\n> On 8/28/2018 4:41 PM, Stefan Beller wrote:\n> > On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n> > <gitgitgadget@gmail.com> wrote:\n> >> +               GIT_TEST_COMMIT_GRAPH=0 &&\n> >> +               test_must_fail git merge -m final G\n> > This could go on the same line without the && in between, setting the\n> > variable as a prefix.\n>\n> It cannot! The Linux build I ran complained that you can't put\n> environment variables through test_must_fail.\n\nIs GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on\ngit-merge anyhow.\n\nAs for the special case of one-shot environment variable and\ntest_must_fail(), you'll find \"env\" used as a workaround in a number\nof tests:\n\n    test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... &&\n"},{"id":"356825","messageId":"42329f9c-c4b5-f5fd-54cc-6b94b74574dc@gmail.com","threadId":"49244","inReplyTo":"CAPig+cSjanDi=jV75PdzYpAjwVgd4Suh3UyvY+Vy7yeHAuY8RA@mail.gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-08-29T12:14:24Z","receivedAt":"2018-08-29T12:14:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/28/2018 5:59 PM, Eric Sunshine wrote:\n> On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee <stolee@gmail.com> wrote:\n>> On 8/28/2018 4:41 PM, Stefan Beller wrote:\n>>> On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget\n>>> <gitgitgadget@gmail.com> wrote:\n>>>> +               GIT_TEST_COMMIT_GRAPH=0 &&\n>>>> +               test_must_fail git merge -m final G\n>>> This could go on the same line without the && in between, setting the\n>>> variable as a prefix.\n>> It cannot! The Linux build I ran complained that you can't put\n>> environment variables through test_must_fail.\n> Is GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on\n> git-merge anyhow.\n\nIn my testing this changed the behavior from fail to pass when passing \nGIT_TEST_COMMIT_GRAPH=1 from the command.\n\n\n> As for the special case of one-shot environment variable and\n> test_must_fail(), you'll find \"env\" used as a workaround in a number\n> of tests:\n>\n>      test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... &&\n\nThanks for this! This is clearly the better solution.\n\n-Stolee\n\n"},{"id":"356828","messageId":"pull.26.v2.git.gitgitgadget@gmail.com","threadId":"49244","inReplyTo":"pull.26.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-29T12:49:02Z","receivedAt":"2018-08-29T12:49:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The commit-graph (and multi-pack-index) features are optional data\nstructures that can make Git operations faster. Since they are optional, we\ndo not enable them in most Git tests. The commit-graph is tested in\nt5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\none script cannot cover the data shapes present in the rest of the test\nsuite.\n\nThis patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n. Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\nwith every git commit command. Thanks, Duy, for pointing out this direction\n[1].\n\nA few tests needed to be modified. These are the same tests that were\nmentioned in my previous example patch [2]. Thanks, Eric, for providing the\ncorrect way to override the settings [3].\n\nWhen this merges down, I'll create a CI build in VSTS that runs the test\nsuite with this option enabled.\n\nThanks, -Stolee\n\n[1] \nhttps://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/\nRe: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment\nvariables)\n\n[2] \nhttps://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/\n[PATCH] DO-NOT-MERGE: write and read commit-graph always\n\n[3] \nhttps://public-inbox.org/git/CAPig+cSjanDi=jV75PdzYpAjwVgd4Suh3UyvY+Vy7yeHAuY8RA@mail.gmail.com/\n\nBased-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.comCc: \nsbeller@google.comCc: sunshine@sunshineco.com\n\nDerrick Stolee (1):\n  commit-graph: define GIT_TEST_COMMIT_GRAPH\n\n builtin/commit.c                    | 4 ++++\n commit-graph.c                      | 5 +++--\n commit-graph.h                      | 2 ++\n t/README                            | 4 ++++\n t/t0410-partial-clone.sh            | 2 +-\n t/t5307-pack-missing-commit.sh      | 4 ++--\n t/t6011-rev-list-with-bad-commit.sh | 7 +++----\n t/t6024-recursive-merge.sh          | 6 +++---\n 8 files changed, 22 insertions(+), 12 deletions(-)\n\n\nbase-commit: 829a321569d8e8f2c582aef9f0c990df976ab842\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/26\n\nRange-diff vs v1:\n\n 1:  85d02ac8d8 ! 1:  4ff6695c7e commit-graph: define GIT_TEST_COMMIT_GRAPH\n     @@ -23,6 +23,7 @@\n          merge-base algorithm picking one of two ambiguous merge-bases, and\n          the commit-graph feature changes which merge-base is picked.\n      \n     +    Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n      \n      diff --git a/builtin/commit.c b/builtin/commit.c\n     @@ -112,18 +113,12 @@\n       \n       test_expect_success 'rev-list notices corruption (1)' '\n      -\ttest_must_fail git rev-list HEAD\n     -+\t(\n     -+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n     -+\t\ttest_must_fail git rev-list HEAD\n     -+\t)\n     ++\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list HEAD\n       '\n       \n       test_expect_success 'rev-list notices corruption (2)' '\n      -\ttest_must_fail git rev-list --objects HEAD\n     -+\t(\n     -+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n     -+\t\ttest_must_fail git rev-list --objects HEAD\n     -+\t)\n     ++\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --objects HEAD\n       '\n       \n       test_expect_success 'pack-objects notices corruption' '\n     @@ -140,10 +135,7 @@\n      -   test_must_fail git rev-list --all > /dev/null\n      -   '\n      +test_expect_success 'rev-list should fail' '\n     -+\t(\n     -+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n     -+\t\ttest_must_fail git rev-list --all > /dev/null\n     -+\t)\n     ++\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --all > /dev/null\n      +'\n       \n       test_expect_success 'git repack _MUST_ fail' \\\n     @@ -160,10 +152,7 @@\n      -\ttest_must_fail git merge -m final G\n      -\"\n      +test_expect_success 'combined merge conflicts' '\n     -+\t(\n     -+\t\tGIT_TEST_COMMIT_GRAPH=0 &&\n     -+\t\ttest_must_fail git merge -m final G\n     -+\t)\n     ++\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge -m final G\n      +'\n       \n       cat > expect << EOF\n\n-- \ngitgitgadget\n"},{"id":"356829","messageId":"4ff6695c7e9bea7f6033f5e124d940f189e290ac.1535546941.git.gitgitgadget@gmail.com","threadId":"49244","inReplyTo":"pull.26.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-29T12:49:04Z","receivedAt":"2018-08-29T12:49:07Z","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 commit-graph feature is tested in isolation by\nt5318-commit-graph.sh and t6600-test-reach.sh, but there are many\nmore interesting scenarios involving commit walks. Many of these\nscenarios are covered by the existing test suite, but we need to\nmaintain coverage when the optional commit-graph structure is not\npresent.\n\nTo allow running the full test suite with the commit-graph present,\nadd a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\nto GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\nto load the commit-graph when parsing commits, and writes the\ncommit-graph file after every 'git commit' command.\n\nThere are a few tests that rely on commits not existing in\npack-files to trigger important events, so manually set\nGIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n\nThere is one test in t6024-recursive-merge.sh that relies on the\nmerge-base algorithm picking one of two ambiguous merge-bases, and\nthe commit-graph feature changes which merge-base is picked.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n builtin/commit.c                    | 4 ++++\n commit-graph.c                      | 5 +++--\n commit-graph.h                      | 2 ++\n t/README                            | 4 ++++\n t/t0410-partial-clone.sh            | 2 +-\n t/t5307-pack-missing-commit.sh      | 4 ++--\n t/t6011-rev-list-with-bad-commit.sh | 7 +++----\n t/t6024-recursive-merge.sh          | 6 +++---\n 8 files changed, 22 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 158e3f843a..a25e652102 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -33,6 +33,7 @@\n #include \"sequencer.h\"\n #include \"mailmap.h\"\n #include \"help.h\"\n+#include \"commit-graph.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [<options>] [--] <pathspec>...\"),\n@@ -1651,6 +1652,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"new_index file. Check that disk is not full and quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n+\tif (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))\n+\t\twrite_commit_graph_reachable(get_object_directory(), 0);\n+\n \trerere(0);\n \trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n \trun_commit_hook(use_editor, get_index_file(), \"post-commit\", NULL);\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 4bd1a4abbf..5cae83fe03 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -237,8 +237,9 @@ static int prepare_commit_graph(struct repository *r)\n \t\treturn !!r->objects->commit_graph;\n \tr->objects->commit_graph_attempted = 1;\n \n-\tif (repo_config_get_bool(r, \"core.commitgraph\", &config_value) ||\n-\t    !config_value)\n+\tif (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&\n+\t    (repo_config_get_bool(r, \"core.commitgraph\", &config_value) ||\n+\t    !config_value))\n \t\t/*\n \t\t * This repository is not configured to use commit graphs, so\n \t\t * do not load one. (But report commit_graph_attempted anyway\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 13d736cdde..cf9141f356 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -5,6 +5,8 @@\n #include \"repository.h\"\n #include \"string-list.h\"\n \n+#define GIT_TEST_COMMIT_GRAPH \"GIT_TEST_COMMIT_GRAPH\"\n+\n struct commit;\n \n char *get_commit_graph_filename(const char *obj_dir);\ndiff --git a/t/README b/t/README\nindex 8373a27fea..2b90c433f5 100644\n--- a/t/README\n+++ b/t/README\n@@ -315,6 +315,10 @@ packs on demand. This normally only happens when the object size is\n over 2GB. This variable forces the code path on any object larger than\n <n> bytes.\n \n+GIT_TEST_COMMIT_GRAPH=<boolean>, when true, forces the commit-graph to\n+be written after every 'git commit' command, and overrides the\n+'core.commitGraph' setting to true.\n+\n Naming Tests\n ------------\n \ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 4984ca583d..73d5284a91 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n \tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n-\tgit -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n+\tGIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n \tgrep $(git -C repo rev-parse bar) out &&\n \t! grep $FOO out\n '\ndiff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh\nindex ae52a1882d..dacb440b27 100755\n--- a/t/t5307-pack-missing-commit.sh\n+++ b/t/t5307-pack-missing-commit.sh\n@@ -24,11 +24,11 @@ test_expect_success 'check corruption' '\n '\n \n test_expect_success 'rev-list notices corruption (1)' '\n-\ttest_must_fail git rev-list HEAD\n+\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list HEAD\n '\n \n test_expect_success 'rev-list notices corruption (2)' '\n-\ttest_must_fail git rev-list --objects HEAD\n+\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --objects HEAD\n '\n \n test_expect_success 'pack-objects notices corruption' '\ndiff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh\nindex e51eb41f4b..545b461e51 100755\n--- a/t/t6011-rev-list-with-bad-commit.sh\n+++ b/t/t6011-rev-list-with-bad-commit.sh\n@@ -41,10 +41,9 @@ test_expect_success 'corrupt second commit object' \\\n    test_must_fail git fsck --full\n    '\n \n-test_expect_success 'rev-list should fail' \\\n-   '\n-   test_must_fail git rev-list --all > /dev/null\n-   '\n+test_expect_success 'rev-list should fail' '\n+\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --all > /dev/null\n+'\n \n test_expect_success 'git repack _MUST_ fail' \\\n    '\ndiff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh\nindex 3f59e58dfb..27c7de90ce 100755\n--- a/t/t6024-recursive-merge.sh\n+++ b/t/t6024-recursive-merge.sh\n@@ -60,9 +60,9 @@ git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:08\" git commit -m F\n '\n \n-test_expect_success \"combined merge conflicts\" \"\n-\ttest_must_fail git merge -m final G\n-\"\n+test_expect_success 'combined merge conflicts' '\n+\ttest_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge -m final G\n+'\n \n cat > expect << EOF\n <<<<<<< HEAD\n-- \ngitgitgadget\n"},{"id":"357306","messageId":"CACsJy8DRzhnxJHXr+zRpBX+rfGPakgbVHYWRDb-XiatFsstfpg@mail.gmail.com","threadId":"49244","inReplyTo":"pull.26.v2.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-09-04T16:49:15Z","receivedAt":"2018-09-04T16:49:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> The commit-graph (and multi-pack-index) features are optional data\n> structures that can make Git operations faster. Since they are optional, we\n> do not enable them in most Git tests. The commit-graph is tested in\n> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\n> one script cannot cover the data shapes present in the rest of the test\n> suite.\n>\n> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\n> with every git commit command. Thanks, Duy, for pointing out this direction\n> [1].\n\nAny reason to not add this new flag in ci/run-build-and-tests.sh\n(which is used by Travis)? I see your note about VSTS but I don't see\nwhy it has to be exclusive to VSTS.\n\n> A few tests needed to be modified. These are the same tests that were\n> mentioned in my previous example patch [2]. Thanks, Eric, for providing the\n> correct way to override the settings [3].\n>\n> When this merges down, I'll create a CI build in VSTS that runs the test\n> suite with this option enabled.\n-- \nDuy\n"},{"id":"357307","messageId":"faebdf82-7e9b-b662-2884-1398d90ab26c@gmail.com","threadId":"49244","inReplyTo":"CACsJy8DRzhnxJHXr+zRpBX+rfGPakgbVHYWRDb-XiatFsstfpg@mail.gmail.com","subject":"Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-04T17:12:55Z","receivedAt":"2018-09-04T17:12:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/4/2018 12:49 PM, Duy Nguyen wrote:\n> On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> The commit-graph (and multi-pack-index) features are optional data\n>> structures that can make Git operations faster. Since they are optional, we\n>> do not enable them in most Git tests. The commit-graph is tested in\n>> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\n>> one script cannot cover the data shapes present in the rest of the test\n>> suite.\n>>\n>> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n>> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\n>> with every git commit command. Thanks, Duy, for pointing out this direction\n>> [1].\n> Any reason to not add this new flag in ci/run-build-and-tests.sh\n> (which is used by Travis)? I see your note about VSTS but I don't see\n> why it has to be exclusive to VSTS.\n\nI only wanted to volunteer resources that I know are available. I am \nlooking into an additional test run in Travis CI builds, but didn't want \nto presume the extra resources would be available.\n\n"},{"id":"357308","messageId":"20180904171801.GA31791@duynguyen.home","threadId":"49244","inReplyTo":"faebdf82-7e9b-b662-2884-1398d90ab26c@gmail.com","subject":"Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-09-04T17:18:01Z","receivedAt":"2018-09-04T17:18:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Sep 04, 2018 at 01:12:55PM -0400, Derrick Stolee wrote:\n> On 9/4/2018 12:49 PM, Duy Nguyen wrote:\n> > On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget\n> > <gitgitgadget@gmail.com> wrote:\n> >> The commit-graph (and multi-pack-index) features are optional data\n> >> structures that can make Git operations faster. Since they are optional, we\n> >> do not enable them in most Git tests. The commit-graph is tested in\n> >> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that\n> >> one script cannot cover the data shapes present in the rest of the test\n> >> suite.\n> >>\n> >> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH\n> >> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it\n> >> with every git commit command. Thanks, Duy, for pointing out this direction\n> >> [1].\n> > Any reason to not add this new flag in ci/run-build-and-tests.sh\n> > (which is used by Travis)? I see your note about VSTS but I don't see\n> > why it has to be exclusive to VSTS.\n> \n> I only wanted to volunteer resources that I know are available. I am \n> looking into an additional test run in Travis CI builds, but didn't want \n> to presume the extra resources would be available.\n> \n\nOK I think there's a misunderstanding. You would not need a separate\ntest run for this, instead the second (existing) run just activates\nboth split index and commit-graph together. The run may be a bit\nslower (doubtful since commit-graph is all about speed) but it would\nnot cost much more resources. Such a change would look like this\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 2a5bff4a1c..185305e3e2 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -15,6 +15,7 @@ then\n \texport GIT_TEST_FULL_IN_PACK_ARRAY=true\n \texport GIT_TEST_OE_SIZE=10\n \texport GIT_TEST_OE_DELTA_SIZE=5\n+\texport GIT_TEST_COMMIT_GRAPH=yes\n \tmake --quiet test\n fi\n \n\n--\nDuy\n"},{"id":"359822","messageId":"87bm84a70p.fsf@evledraar.gmail.com","threadId":"49244","inReplyTo":"85d02ac8d8c9a8950ce1a9760a541ff506945de0.1535488400.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-08T13:43:18Z","receivedAt":"2018-10-08T13:43:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee <dstolee@microsoft.com>\n>\n> The commit-graph feature is tested in isolation by\n> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n> more interesting scenarios involving commit walks. Many of these\n> scenarios are covered by the existing test suite, but we need to\n> maintain coverage when the optional commit-graph structure is not\n> present.\n>\n> To allow running the full test suite with the commit-graph present,\n> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n> to load the commit-graph when parsing commits, and writes the\n> commit-graph file after every 'git commit' command.\n>\n> There are a few tests that rely on commits not existing in\n> pack-files to trigger important events, so manually set\n> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n>\n> There is one test in t6024-recursive-merge.sh that relies on the\n> merge-base algorithm picking one of two ambiguous merge-bases, and\n> the commit-graph feature changes which merge-base is picked.\n>\n\nThe test feature itself seems fine, but this consistently fails ever\nsince it got introduced (a reset --hard on the commit merged to msater\nin git.git):\n\n    GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh\n    Test Summary Report\n    -------------------\n    t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6)\n      Failed tests:  3, 5, 7, 9, 11, 13\n      Non-zero exit status: 1\n    t6050-replace.sh       (Wstat: 256 Tests: 35 Failed: 9)\n      Failed tests:  12-16, 24-25, 30, 35\n      Non-zero exit status: 1\n    t5500-fetch-pack.sh    (Wstat: 256 Tests: 357 Failed: 1)\n      Failed test:  351\n      Non-zero exit status: 1\n\nThis is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I\ncan provide more info (-x output etc..).\n"},{"id":"359825","messageId":"a2299be9-0840-da69-ebeb-f64ebe66db5a@gmail.com","threadId":"49244","inReplyTo":"87bm84a70p.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-08T14:45:21Z","receivedAt":"2018-10-08T14:45:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote:\n>\n>> From: Derrick Stolee <dstolee@microsoft.com>\n>>\n>> The commit-graph feature is tested in isolation by\n>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n>> more interesting scenarios involving commit walks. Many of these\n>> scenarios are covered by the existing test suite, but we need to\n>> maintain coverage when the optional commit-graph structure is not\n>> present.\n>>\n>> To allow running the full test suite with the commit-graph present,\n>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n>> to load the commit-graph when parsing commits, and writes the\n>> commit-graph file after every 'git commit' command.\n>>\n>> There are a few tests that rely on commits not existing in\n>> pack-files to trigger important events, so manually set\n>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n>>\n>> There is one test in t6024-recursive-merge.sh that relies on the\n>> merge-base algorithm picking one of two ambiguous merge-bases, and\n>> the commit-graph feature changes which merge-base is picked.\n>>\n> The test feature itself seems fine, but this consistently fails ever\n> since it got introduced (a reset --hard on the commit merged to msater\n> in git.git):\n>\n>      GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh\n>      Test Summary Report\n>      -------------------\n>      t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6)\n>        Failed tests:  3, 5, 7, 9, 11, 13\n>        Non-zero exit status: 1\n>      t6050-replace.sh       (Wstat: 256 Tests: 35 Failed: 9)\n>        Failed tests:  12-16, 24-25, 30, 35\n>        Non-zero exit status: 1\n>      t5500-fetch-pack.sh    (Wstat: 256 Tests: 357 Failed: 1)\n>        Failed test:  351\n>        Non-zero exit status: 1\n>\n> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I\n> can provide more info (-x output etc..).\nI see these failures, too, but I believe they are due to \nds/commit-graph-with-grafts not being merged to 'next' yet. The purpose \nof that branch is to fix these test breaks. The environment variable got \nmerged a lot faster.\n\nI just built & tested the 'jch' branch at 515d82d9 with \nGIT_TEST_COMMIT_GRAPH=1 and they all passed.\n\nThanks,\n-Stolee\n"},{"id":"359829","messageId":"877eisa3j4.fsf@evledraar.gmail.com","threadId":"49244","inReplyTo":"a2299be9-0840-da69-ebeb-f64ebe66db5a@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-08T14:58:39Z","receivedAt":"2018-10-08T14:58:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 08 2018, Derrick Stolee wrote:\n\n> On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote:\n>>\n>>> From: Derrick Stolee <dstolee@microsoft.com>\n>>>\n>>> The commit-graph feature is tested in isolation by\n>>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n>>> more interesting scenarios involving commit walks. Many of these\n>>> scenarios are covered by the existing test suite, but we need to\n>>> maintain coverage when the optional commit-graph structure is not\n>>> present.\n>>>\n>>> To allow running the full test suite with the commit-graph present,\n>>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n>>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n>>> to load the commit-graph when parsing commits, and writes the\n>>> commit-graph file after every 'git commit' command.\n>>>\n>>> There are a few tests that rely on commits not existing in\n>>> pack-files to trigger important events, so manually set\n>>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n>>>\n>>> There is one test in t6024-recursive-merge.sh that relies on the\n>>> merge-base algorithm picking one of two ambiguous merge-bases, and\n>>> the commit-graph feature changes which merge-base is picked.\n>>>\n>> The test feature itself seems fine, but this consistently fails ever\n>> since it got introduced (a reset --hard on the commit merged to msater\n>> in git.git):\n>>\n>>      GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh\n>>      Test Summary Report\n>>      -------------------\n>>      t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6)\n>>        Failed tests:  3, 5, 7, 9, 11, 13\n>>        Non-zero exit status: 1\n>>      t6050-replace.sh       (Wstat: 256 Tests: 35 Failed: 9)\n>>        Failed tests:  12-16, 24-25, 30, 35\n>>        Non-zero exit status: 1\n>>      t5500-fetch-pack.sh    (Wstat: 256 Tests: 357 Failed: 1)\n>>        Failed test:  351\n>>        Non-zero exit status: 1\n>>\n>> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I\n>> can provide more info (-x output etc..).\n> I see these failures, too, but I believe they are due to\n> ds/commit-graph-with-grafts not being merged to 'next' yet. The\n> purpose of that branch is to fix these test breaks. The environment\n> variable got merged a lot faster.\n>\n> I just built & tested the 'jch' branch at 515d82d9 with\n> GIT_TEST_COMMIT_GRAPH=1 and they all passed.\n\nI should have tested \"pu\" first. These failures are indeed fixed\nthere. Thanks, and sorry about the noise.\n"},{"id":"359830","messageId":"ac73c76f-a292-c52d-d56a-631b929d6ce5@gmail.com","threadId":"49244","inReplyTo":"877eisa3j4.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-08T15:01:15Z","receivedAt":"2018-10-08T15:01:18Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/8/2018 10:58 AM, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Oct 08 2018, Derrick Stolee wrote:\n>\n>> On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote:\n>>> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote:\n>>>\n>>>> From: Derrick Stolee <dstolee@microsoft.com>\n>>>>\n>>>> The commit-graph feature is tested in isolation by\n>>>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many\n>>>> more interesting scenarios involving commit walks. Many of these\n>>>> scenarios are covered by the existing test suite, but we need to\n>>>> maintain coverage when the optional commit-graph structure is not\n>>>> present.\n>>>>\n>>>> To allow running the full test suite with the commit-graph present,\n>>>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar\n>>>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try\n>>>> to load the commit-graph when parsing commits, and writes the\n>>>> commit-graph file after every 'git commit' command.\n>>>>\n>>>> There are a few tests that rely on commits not existing in\n>>>> pack-files to trigger important events, so manually set\n>>>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands.\n>>>>\n>>>> There is one test in t6024-recursive-merge.sh that relies on the\n>>>> merge-base algorithm picking one of two ambiguous merge-bases, and\n>>>> the commit-graph feature changes which merge-base is picked.\n>>>>\n>>> The test feature itself seems fine, but this consistently fails ever\n>>> since it got introduced (a reset --hard on the commit merged to msater\n>>> in git.git):\n>>>\n>>>       GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh\n>>>       Test Summary Report\n>>>       -------------------\n>>>       t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6)\n>>>         Failed tests:  3, 5, 7, 9, 11, 13\n>>>         Non-zero exit status: 1\n>>>       t6050-replace.sh       (Wstat: 256 Tests: 35 Failed: 9)\n>>>         Failed tests:  12-16, 24-25, 30, 35\n>>>         Non-zero exit status: 1\n>>>       t5500-fetch-pack.sh    (Wstat: 256 Tests: 357 Failed: 1)\n>>>         Failed test:  351\n>>>         Non-zero exit status: 1\n>>>\n>>> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I\n>>> can provide more info (-x output etc..).\n>> I see these failures, too, but I believe they are due to\n>> ds/commit-graph-with-grafts not being merged to 'next' yet. The\n>> purpose of that branch is to fix these test breaks. The environment\n>> variable got merged a lot faster.\n>>\n>> I just built & tested the 'jch' branch at 515d82d9 with\n>> GIT_TEST_COMMIT_GRAPH=1 and they all passed.\n> I should have tested \"pu\" first. These failures are indeed fixed\n> there. Thanks, and sorry about the noise.\nThanks for testing with the optional features! It's good to keep them \nexercised.\n"},{"id":"359902","messageId":"xmqqd0sjr7hg.fsf@gitster-ct.c.googlers.com","threadId":"49244","inReplyTo":"a2299be9-0840-da69-ebeb-f64ebe66db5a@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-09T05:53:31Z","receivedAt":"2018-10-09T05:53:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> I see these failures, too, but I believe they are due to\n> ds/commit-graph-with-grafts not being merged to 'next' yet. The\n> purpose of that branch is to fix these test breaks. The environment\n> variable got merged a lot faster.\n\nA separate \"ping\" would have helped me.  Will merge it down to\n'next'.\n"}]}