* [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage @ 2018-08-28 20:33 Derrick Stolee via GitGitGadget 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-08-28 20:33 UTC (permalink / raw) To: git; +Cc: sbeller, Junio C Hamano The commit-graph (and multi-pack-index) features are optional data structures that can make Git operations faster. Since they are optional, we do not enable them in most Git tests. The commit-graph is tested in t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that one script cannot cover the data shapes present in the rest of the test suite. This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it with every git commit command. Thanks, Duy, for pointing out this direction [1]. A few tests needed to be modified. These are the same tests that were mentioned in my previous example patch [2]. When this merges down, I'll create a CI build in VSTS that runs the test suite with this option enabled. Thanks, -Stolee [1] https://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/ Re: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment variables) [2] https://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/ [PATCH] DO-NOT-MERGE: write and read commit-graph always Based-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.com Derrick Stolee (1): commit-graph: define GIT_TEST_COMMIT_GRAPH builtin/commit.c | 4 ++++ commit-graph.c | 5 +++-- commit-graph.h | 2 ++ t/README | 4 ++++ t/t0410-partial-clone.sh | 2 +- t/t5307-pack-missing-commit.sh | 10 ++++++++-- t/t6011-rev-list-with-bad-commit.sh | 10 ++++++---- t/t6024-recursive-merge.sh | 9 ++++++--- 8 files changed, 34 insertions(+), 12 deletions(-) base-commit: 829a321569d8e8f2c582aef9f0c990df976ab842 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/26 -- gitgitgadget ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 20:33 [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Derrick Stolee via GitGitGadget @ 2018-08-28 20:33 ` Derrick Stolee via GitGitGadget 2018-08-28 20:41 ` Stefan Beller 2018-10-08 13:43 ` Ævar Arnfjörð Bjarmason 2018-08-28 20:37 ` [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Stefan Beller 2018-08-29 12:49 ` [PATCH v2 " Derrick Stolee via GitGitGadget 2 siblings, 2 replies; 18+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-08-28 20:33 UTC (permalink / raw) To: git; +Cc: sbeller, Junio C Hamano, Derrick Stolee From: Derrick Stolee <dstolee@microsoft.com> The commit-graph feature is tested in isolation by t5318-commit-graph.sh and t6600-test-reach.sh, but there are many more interesting scenarios involving commit walks. Many of these scenarios are covered by the existing test suite, but we need to maintain coverage when the optional commit-graph structure is not present. To allow running the full test suite with the commit-graph present, add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try to load the commit-graph when parsing commits, and writes the commit-graph file after every 'git commit' command. There are a few tests that rely on commits not existing in pack-files to trigger important events, so manually set GIT_TEST_COMMIT_GRAPH to false for the necessary commands. There is one test in t6024-recursive-merge.sh that relies on the merge-base algorithm picking one of two ambiguous merge-bases, and the commit-graph feature changes which merge-base is picked. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- builtin/commit.c | 4 ++++ commit-graph.c | 5 +++-- commit-graph.h | 2 ++ t/README | 4 ++++ t/t0410-partial-clone.sh | 2 +- t/t5307-pack-missing-commit.sh | 10 ++++++++-- t/t6011-rev-list-with-bad-commit.sh | 10 ++++++---- t/t6024-recursive-merge.sh | 9 ++++++--- 8 files changed, 34 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 158e3f843a..a25e652102 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -33,6 +33,7 @@ #include "sequencer.h" #include "mailmap.h" #include "help.h" +#include "commit-graph.h" static const char * const builtin_commit_usage[] = { N_("git commit [<options>] [--] <pathspec>..."), @@ -1651,6 +1652,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "new_index file. Check that disk is not full and quota is\n" "not exceeded, and then \"git reset HEAD\" to recover.")); + if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0)) + write_commit_graph_reachable(get_object_directory(), 0); + rerere(0); run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); diff --git a/commit-graph.c b/commit-graph.c index 4bd1a4abbf..5cae83fe03 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -237,8 +237,9 @@ static int prepare_commit_graph(struct repository *r) return !!r->objects->commit_graph; r->objects->commit_graph_attempted = 1; - if (repo_config_get_bool(r, "core.commitgraph", &config_value) || - !config_value) + if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && + (repo_config_get_bool(r, "core.commitgraph", &config_value) || + !config_value)) /* * This repository is not configured to use commit graphs, so * do not load one. (But report commit_graph_attempted anyway diff --git a/commit-graph.h b/commit-graph.h index 13d736cdde..cf9141f356 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -5,6 +5,8 @@ #include "repository.h" #include "string-list.h" +#define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH" + struct commit; char *get_commit_graph_filename(const char *obj_dir); diff --git a/t/README b/t/README index 8373a27fea..2b90c433f5 100644 --- a/t/README +++ b/t/README @@ -315,6 +315,10 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than <n> bytes. +GIT_TEST_COMMIT_GRAPH=<boolean>, when true, forces the commit-graph to +be written after every 'git commit' command, and overrides the +'core.commitGraph' setting to true. + Naming Tests ------------ diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 4984ca583d..73d5284a91 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' ' git -C repo config core.repositoryformatversion 1 && git -C repo config extensions.partialclone "arbitrary string" && - git -C repo rev-list --exclude-promisor-objects --objects bar >out && + GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out && grep $(git -C repo rev-parse bar) out && ! grep $FOO out ' diff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh index ae52a1882d..15df19a040 100755 --- a/t/t5307-pack-missing-commit.sh +++ b/t/t5307-pack-missing-commit.sh @@ -24,11 +24,17 @@ test_expect_success 'check corruption' ' ' test_expect_success 'rev-list notices corruption (1)' ' - test_must_fail git rev-list HEAD + ( + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git rev-list HEAD + ) ' test_expect_success 'rev-list notices corruption (2)' ' - test_must_fail git rev-list --objects HEAD + ( + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git rev-list --objects HEAD + ) ' test_expect_success 'pack-objects notices corruption' ' diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh index e51eb41f4b..d774c69871 100755 --- a/t/t6011-rev-list-with-bad-commit.sh +++ b/t/t6011-rev-list-with-bad-commit.sh @@ -41,10 +41,12 @@ test_expect_success 'corrupt second commit object' \ test_must_fail git fsck --full ' -test_expect_success 'rev-list should fail' \ - ' - test_must_fail git rev-list --all > /dev/null - ' +test_expect_success 'rev-list should fail' ' + ( + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git rev-list --all > /dev/null + ) +' test_expect_success 'git repack _MUST_ fail' \ ' diff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh index 3f59e58dfb..04b30c5b76 100755 --- a/t/t6024-recursive-merge.sh +++ b/t/t6024-recursive-merge.sh @@ -60,9 +60,12 @@ git update-index a1 && GIT_AUTHOR_DATE="2006-12-12 23:00:08" git commit -m F ' -test_expect_success "combined merge conflicts" " - test_must_fail git merge -m final G -" +test_expect_success 'combined merge conflicts' ' + ( + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git merge -m final G + ) +' cat > expect << EOF <<<<<<< HEAD -- gitgitgadget ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget @ 2018-08-28 20:41 ` Stefan Beller 2018-08-28 21:31 ` Derrick Stolee 2018-10-08 13:43 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 18+ messages in thread From: Stefan Beller @ 2018-08-28 20:41 UTC (permalink / raw) To: gitgitgadget; +Cc: git, Junio C Hamano, Derrick Stolee On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > The commit-graph feature is tested in isolation by > t5318-commit-graph.sh and t6600-test-reach.sh, but there are many > more interesting scenarios involving commit walks. Many of these > scenarios are covered by the existing test suite, but we need to > maintain coverage when the optional commit-graph structure is not > present. > > To allow running the full test suite with the commit-graph present, > add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar > to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try > to load the commit-graph when parsing commits, and writes the > commit-graph file after every 'git commit' command. > > There are a few tests that rely on commits not existing in > pack-files to trigger important events, so manually set > GIT_TEST_COMMIT_GRAPH to false for the necessary commands. So the plan is to turn on the commit graph for the whole test suite excluding these selected tests? > diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh > index 4984ca583d..73d5284a91 100755 > --- a/t/t0410-partial-clone.sh > +++ b/t/t0410-partial-clone.sh > @@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' ' > > git -C repo config core.repositoryformatversion 1 && > git -C repo config extensions.partialclone "arbitrary string" && > - git -C repo rev-list --exclude-promisor-objects --objects bar >out && > + GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out && > + GIT_TEST_COMMIT_GRAPH=0 && > + test_must_fail git merge -m final G This could go on the same line without the && in between, setting the variable as a prefix. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 20:41 ` Stefan Beller @ 2018-08-28 21:31 ` Derrick Stolee 2018-08-28 21:59 ` Eric Sunshine 0 siblings, 1 reply; 18+ messages in thread From: Derrick Stolee @ 2018-08-28 21:31 UTC (permalink / raw) To: Stefan Beller, gitgitgadget; +Cc: git, Junio C Hamano, Derrick Stolee On 8/28/2018 4:41 PM, Stefan Beller wrote: > On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> From: Derrick Stolee <dstolee@microsoft.com> >> >> The commit-graph feature is tested in isolation by >> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many >> more interesting scenarios involving commit walks. Many of these >> scenarios are covered by the existing test suite, but we need to >> maintain coverage when the optional commit-graph structure is not >> present. >> >> To allow running the full test suite with the commit-graph present, >> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar >> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try >> to load the commit-graph when parsing commits, and writes the >> commit-graph file after every 'git commit' command. >> >> There are a few tests that rely on commits not existing in >> pack-files to trigger important events, so manually set >> GIT_TEST_COMMIT_GRAPH to false for the necessary commands. > So the plan is to turn on the commit graph for the whole test suite > excluding these selected tests? Excluding these specific _steps_, but yes. > >> + GIT_TEST_COMMIT_GRAPH=0 && >> + test_must_fail git merge -m final G > This could go on the same line without the && in between, setting the > variable as a prefix. It cannot! The Linux build I ran complained that you can't put environment variables through test_must_fail. -Stolee ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 21:31 ` Derrick Stolee @ 2018-08-28 21:59 ` Eric Sunshine 2018-08-29 12:14 ` Derrick Stolee 0 siblings, 1 reply; 18+ messages in thread From: Eric Sunshine @ 2018-08-28 21:59 UTC (permalink / raw) To: Derrick Stolee Cc: Stefan Beller, gitgitgadget, Git List, Junio C Hamano, Derrick Stolee On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee <stolee@gmail.com> wrote: > On 8/28/2018 4:41 PM, Stefan Beller wrote: > > On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > >> + GIT_TEST_COMMIT_GRAPH=0 && > >> + test_must_fail git merge -m final G > > This could go on the same line without the && in between, setting the > > variable as a prefix. > > It cannot! The Linux build I ran complained that you can't put > environment variables through test_must_fail. Is GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on git-merge anyhow. As for the special case of one-shot environment variable and test_must_fail(), you'll find "env" used as a workaround in a number of tests: test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... && ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 21:59 ` Eric Sunshine @ 2018-08-29 12:14 ` Derrick Stolee 0 siblings, 0 replies; 18+ messages in thread From: Derrick Stolee @ 2018-08-29 12:14 UTC (permalink / raw) To: Eric Sunshine Cc: Stefan Beller, gitgitgadget, Git List, Junio C Hamano, Derrick Stolee On 8/28/2018 5:59 PM, Eric Sunshine wrote: > On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee <stolee@gmail.com> wrote: >> On 8/28/2018 4:41 PM, Stefan Beller wrote: >>> On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget >>> <gitgitgadget@gmail.com> wrote: >>>> + GIT_TEST_COMMIT_GRAPH=0 && >>>> + test_must_fail git merge -m final G >>> This could go on the same line without the && in between, setting the >>> variable as a prefix. >> It cannot! The Linux build I ran complained that you can't put >> environment variables through test_must_fail. > Is GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on > git-merge anyhow. In my testing this changed the behavior from fail to pass when passing GIT_TEST_COMMIT_GRAPH=1 from the command. > As for the special case of one-shot environment variable and > test_must_fail(), you'll find "env" used as a workaround in a number > of tests: > > test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... && Thanks for this! This is clearly the better solution. -Stolee ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget 2018-08-28 20:41 ` Stefan Beller @ 2018-10-08 13:43 ` Ævar Arnfjörð Bjarmason 2018-10-08 14:45 ` Derrick Stolee 1 sibling, 1 reply; 18+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-10-08 13:43 UTC (permalink / raw) To: Derrick Stolee via GitGitGadget Cc: git, sbeller, Junio C Hamano, Derrick Stolee On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > The commit-graph feature is tested in isolation by > t5318-commit-graph.sh and t6600-test-reach.sh, but there are many > more interesting scenarios involving commit walks. Many of these > scenarios are covered by the existing test suite, but we need to > maintain coverage when the optional commit-graph structure is not > present. > > To allow running the full test suite with the commit-graph present, > add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar > to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try > to load the commit-graph when parsing commits, and writes the > commit-graph file after every 'git commit' command. > > There are a few tests that rely on commits not existing in > pack-files to trigger important events, so manually set > GIT_TEST_COMMIT_GRAPH to false for the necessary commands. > > There is one test in t6024-recursive-merge.sh that relies on the > merge-base algorithm picking one of two ambiguous merge-bases, and > the commit-graph feature changes which merge-base is picked. > The test feature itself seems fine, but this consistently fails ever since it got introduced (a reset --hard on the commit merged to msater in git.git): GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh Test Summary Report ------------------- t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) Failed tests: 3, 5, 7, 9, 11, 13 Non-zero exit status: 1 t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) Failed tests: 12-16, 24-25, 30, 35 Non-zero exit status: 1 t5500-fetch-pack.sh (Wstat: 256 Tests: 357 Failed: 1) Failed test: 351 Non-zero exit status: 1 This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I can provide more info (-x output etc..). ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-10-08 13:43 ` Ævar Arnfjörð Bjarmason @ 2018-10-08 14:45 ` Derrick Stolee 2018-10-08 14:58 ` Ævar Arnfjörð Bjarmason 2018-10-09 5:53 ` Junio C Hamano 0 siblings, 2 replies; 18+ messages in thread From: Derrick Stolee @ 2018-10-08 14:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Derrick Stolee via GitGitGadget Cc: git, sbeller, Junio C Hamano, Derrick Stolee On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: > On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: > >> From: Derrick Stolee <dstolee@microsoft.com> >> >> The commit-graph feature is tested in isolation by >> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many >> more interesting scenarios involving commit walks. Many of these >> scenarios are covered by the existing test suite, but we need to >> maintain coverage when the optional commit-graph structure is not >> present. >> >> To allow running the full test suite with the commit-graph present, >> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar >> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try >> to load the commit-graph when parsing commits, and writes the >> commit-graph file after every 'git commit' command. >> >> There are a few tests that rely on commits not existing in >> pack-files to trigger important events, so manually set >> GIT_TEST_COMMIT_GRAPH to false for the necessary commands. >> >> There is one test in t6024-recursive-merge.sh that relies on the >> merge-base algorithm picking one of two ambiguous merge-bases, and >> the commit-graph feature changes which merge-base is picked. >> > The test feature itself seems fine, but this consistently fails ever > since it got introduced (a reset --hard on the commit merged to msater > in git.git): > > GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh > Test Summary Report > ------------------- > t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) > Failed tests: 3, 5, 7, 9, 11, 13 > Non-zero exit status: 1 > t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) > Failed tests: 12-16, 24-25, 30, 35 > Non-zero exit status: 1 > t5500-fetch-pack.sh (Wstat: 256 Tests: 357 Failed: 1) > Failed test: 351 > Non-zero exit status: 1 > > This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I > can provide more info (-x output etc..). I see these failures, too, but I believe they are due to ds/commit-graph-with-grafts not being merged to 'next' yet. The purpose of that branch is to fix these test breaks. The environment variable got merged a lot faster. I just built & tested the 'jch' branch at 515d82d9 with GIT_TEST_COMMIT_GRAPH=1 and they all passed. Thanks, -Stolee ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-10-08 14:45 ` Derrick Stolee @ 2018-10-08 14:58 ` Ævar Arnfjörð Bjarmason 2018-10-08 15:01 ` Derrick Stolee 2018-10-09 5:53 ` Junio C Hamano 1 sibling, 1 reply; 18+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-10-08 14:58 UTC (permalink / raw) To: Derrick Stolee Cc: Derrick Stolee via GitGitGadget, git, sbeller, Junio C Hamano, Derrick Stolee On Mon, Oct 08 2018, Derrick Stolee wrote: > On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: >> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: >> >>> From: Derrick Stolee <dstolee@microsoft.com> >>> >>> The commit-graph feature is tested in isolation by >>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many >>> more interesting scenarios involving commit walks. Many of these >>> scenarios are covered by the existing test suite, but we need to >>> maintain coverage when the optional commit-graph structure is not >>> present. >>> >>> To allow running the full test suite with the commit-graph present, >>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar >>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try >>> to load the commit-graph when parsing commits, and writes the >>> commit-graph file after every 'git commit' command. >>> >>> There are a few tests that rely on commits not existing in >>> pack-files to trigger important events, so manually set >>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands. >>> >>> There is one test in t6024-recursive-merge.sh that relies on the >>> merge-base algorithm picking one of two ambiguous merge-bases, and >>> the commit-graph feature changes which merge-base is picked. >>> >> The test feature itself seems fine, but this consistently fails ever >> since it got introduced (a reset --hard on the commit merged to msater >> in git.git): >> >> GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh >> Test Summary Report >> ------------------- >> t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) >> Failed tests: 3, 5, 7, 9, 11, 13 >> Non-zero exit status: 1 >> t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) >> Failed tests: 12-16, 24-25, 30, 35 >> Non-zero exit status: 1 >> t5500-fetch-pack.sh (Wstat: 256 Tests: 357 Failed: 1) >> Failed test: 351 >> Non-zero exit status: 1 >> >> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I >> can provide more info (-x output etc..). > I see these failures, too, but I believe they are due to > ds/commit-graph-with-grafts not being merged to 'next' yet. The > purpose of that branch is to fix these test breaks. The environment > variable got merged a lot faster. > > I just built & tested the 'jch' branch at 515d82d9 with > GIT_TEST_COMMIT_GRAPH=1 and they all passed. I should have tested "pu" first. These failures are indeed fixed there. Thanks, and sorry about the noise. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-10-08 14:58 ` Ævar Arnfjörð Bjarmason @ 2018-10-08 15:01 ` Derrick Stolee 0 siblings, 0 replies; 18+ messages in thread From: Derrick Stolee @ 2018-10-08 15:01 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Derrick Stolee via GitGitGadget, git, sbeller, Junio C Hamano, Derrick Stolee On 10/8/2018 10:58 AM, Ævar Arnfjörð Bjarmason wrote: > On Mon, Oct 08 2018, Derrick Stolee wrote: > >> On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: >>> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: >>> >>>> From: Derrick Stolee <dstolee@microsoft.com> >>>> >>>> The commit-graph feature is tested in isolation by >>>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many >>>> more interesting scenarios involving commit walks. Many of these >>>> scenarios are covered by the existing test suite, but we need to >>>> maintain coverage when the optional commit-graph structure is not >>>> present. >>>> >>>> To allow running the full test suite with the commit-graph present, >>>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar >>>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try >>>> to load the commit-graph when parsing commits, and writes the >>>> commit-graph file after every 'git commit' command. >>>> >>>> There are a few tests that rely on commits not existing in >>>> pack-files to trigger important events, so manually set >>>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands. >>>> >>>> There is one test in t6024-recursive-merge.sh that relies on the >>>> merge-base algorithm picking one of two ambiguous merge-bases, and >>>> the commit-graph feature changes which merge-base is picked. >>>> >>> The test feature itself seems fine, but this consistently fails ever >>> since it got introduced (a reset --hard on the commit merged to msater >>> in git.git): >>> >>> GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh >>> Test Summary Report >>> ------------------- >>> t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) >>> Failed tests: 3, 5, 7, 9, 11, 13 >>> Non-zero exit status: 1 >>> t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) >>> Failed tests: 12-16, 24-25, 30, 35 >>> Non-zero exit status: 1 >>> t5500-fetch-pack.sh (Wstat: 256 Tests: 357 Failed: 1) >>> Failed test: 351 >>> Non-zero exit status: 1 >>> >>> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I >>> can provide more info (-x output etc..). >> I see these failures, too, but I believe they are due to >> ds/commit-graph-with-grafts not being merged to 'next' yet. The >> purpose of that branch is to fix these test breaks. The environment >> variable got merged a lot faster. >> >> I just built & tested the 'jch' branch at 515d82d9 with >> GIT_TEST_COMMIT_GRAPH=1 and they all passed. > I should have tested "pu" first. These failures are indeed fixed > there. Thanks, and sorry about the noise. Thanks for testing with the optional features! It's good to keep them exercised. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-10-08 14:45 ` Derrick Stolee 2018-10-08 14:58 ` Ævar Arnfjörð Bjarmason @ 2018-10-09 5:53 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-10-09 5:53 UTC (permalink / raw) To: Derrick Stolee Cc: Ævar Arnfjörð Bjarmason, Derrick Stolee via GitGitGadget, git, sbeller, Derrick Stolee Derrick Stolee <stolee@gmail.com> writes: > I see these failures, too, but I believe they are due to > ds/commit-graph-with-grafts not being merged to 'next' yet. The > purpose of that branch is to fix these test breaks. The environment > variable got merged a lot faster. A separate "ping" would have helped me. Will merge it down to 'next'. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-08-28 20:33 [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Derrick Stolee via GitGitGadget 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget @ 2018-08-28 20:37 ` Stefan Beller 2018-08-28 21:32 ` Derrick Stolee 2018-08-29 12:49 ` [PATCH v2 " Derrick Stolee via GitGitGadget 2 siblings, 1 reply; 18+ messages in thread From: Stefan Beller @ 2018-08-28 20:37 UTC (permalink / raw) To: gitgitgadget; +Cc: git, Junio C Hamano On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > The commit-graph (and multi-pack-index) features are optional data > structures that can make Git operations faster. Since they are optional, we > do not enable them in most Git tests. The commit-graph is tested in > t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that > one script cannot cover the data shapes present in the rest of the test > suite. > > This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH > . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it > with every git commit command. > Thanks, Duy, for pointing out this direction Did you mean to cc Duy (instead of me)? (I'll happily review the patch, too... just asking) > > A few tests needed to be modified. These are the same tests that were > mentioned in my previous example patch [2]. > > When this merges down, I'll create a CI build in VSTS that runs the test > suite with this option enabled. > > Thanks, -Stolee > > [1] > https://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/ > Re: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment > variables) > > [2] > https://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/ > [PATCH] DO-NOT-MERGE: write and read commit-graph always > > Based-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.com > > Derrick Stolee (1): > commit-graph: define GIT_TEST_COMMIT_GRAPH > > builtin/commit.c | 4 ++++ > commit-graph.c | 5 +++-- > commit-graph.h | 2 ++ > t/README | 4 ++++ > t/t0410-partial-clone.sh | 2 +- > t/t5307-pack-missing-commit.sh | 10 ++++++++-- > t/t6011-rev-list-with-bad-commit.sh | 10 ++++++---- > t/t6024-recursive-merge.sh | 9 ++++++--- > 8 files changed, 34 insertions(+), 12 deletions(-) > > > base-commit: 829a321569d8e8f2c582aef9f0c990df976ab842 > Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/26 > -- > gitgitgadget ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-08-28 20:37 ` [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Stefan Beller @ 2018-08-28 21:32 ` Derrick Stolee 0 siblings, 0 replies; 18+ messages in thread From: Derrick Stolee @ 2018-08-28 21:32 UTC (permalink / raw) To: Stefan Beller, gitgitgadget; +Cc: git, Junio C Hamano, Duy Nguyen On 8/28/2018 4:37 PM, Stefan Beller wrote: > On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> The commit-graph (and multi-pack-index) features are optional data >> structures that can make Git operations faster. Since they are optional, we >> do not enable them in most Git tests. The commit-graph is tested in >> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that >> one script cannot cover the data shapes present in the rest of the test >> suite. >> >> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH >> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it >> with every git commit command. >> Thanks, Duy, for pointing out this direction > Did you mean to cc Duy (instead of me)? > (I'll happily review the patch, too... just asking) I just added you because you've been on a lot of commit-graph things lately. But yes, I forgot to add Duy to the CC list. Added to this message. Thanks, -Stolee ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-08-28 20:33 [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Derrick Stolee via GitGitGadget 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget 2018-08-28 20:37 ` [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Stefan Beller @ 2018-08-29 12:49 ` Derrick Stolee via GitGitGadget 2018-08-29 12:49 ` [PATCH v2 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget 2018-09-04 16:49 ` [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Duy Nguyen 2 siblings, 2 replies; 18+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-08-29 12:49 UTC (permalink / raw) To: git; +Cc: pclouds, Junio C Hamano The commit-graph (and multi-pack-index) features are optional data structures that can make Git operations faster. Since they are optional, we do not enable them in most Git tests. The commit-graph is tested in t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that one script cannot cover the data shapes present in the rest of the test suite. This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it with every git commit command. Thanks, Duy, for pointing out this direction [1]. A few tests needed to be modified. These are the same tests that were mentioned in my previous example patch [2]. Thanks, Eric, for providing the correct way to override the settings [3]. When this merges down, I'll create a CI build in VSTS that runs the test suite with this option enabled. Thanks, -Stolee [1] https://public-inbox.org/git/CACsJy8CKnXVJYkKM_W=N=Vq-TVXf+YCqZP_uP7B-dN_6xddB=g@mail.gmail.com/ Re: [PATCH 0/9] multi-pack-index cleanups (Discussing test environment variables) [2] https://public-inbox.org/git/20180718152244.45513-1-dstolee@microsoft.com/ [PATCH] DO-NOT-MERGE: write and read commit-graph always [3] https://public-inbox.org/git/CAPig+cSjanDi=jV75PdzYpAjwVgd4Suh3UyvY+Vy7yeHAuY8RA@mail.gmail.com/ Based-On: ds/commit-graph-with-grafts Cc: jnareb@gmail.comCc: sbeller@google.comCc: sunshine@sunshineco.com Derrick Stolee (1): commit-graph: define GIT_TEST_COMMIT_GRAPH builtin/commit.c | 4 ++++ commit-graph.c | 5 +++-- commit-graph.h | 2 ++ t/README | 4 ++++ t/t0410-partial-clone.sh | 2 +- t/t5307-pack-missing-commit.sh | 4 ++-- t/t6011-rev-list-with-bad-commit.sh | 7 +++---- t/t6024-recursive-merge.sh | 6 +++--- 8 files changed, 22 insertions(+), 12 deletions(-) base-commit: 829a321569d8e8f2c582aef9f0c990df976ab842 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-26%2Fderrickstolee%2Fshallow%2Ftest-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-26/derrickstolee/shallow/test-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/26 Range-diff vs v1: 1: 85d02ac8d8 ! 1: 4ff6695c7e commit-graph: define GIT_TEST_COMMIT_GRAPH @@ -23,6 +23,7 @@ merge-base algorithm picking one of two ambiguous merge-bases, and the commit-graph feature changes which merge-base is picked. + Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> diff --git a/builtin/commit.c b/builtin/commit.c @@ -112,18 +113,12 @@ test_expect_success 'rev-list notices corruption (1)' ' - test_must_fail git rev-list HEAD -+ ( -+ GIT_TEST_COMMIT_GRAPH=0 && -+ test_must_fail git rev-list HEAD -+ ) ++ test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list HEAD ' test_expect_success 'rev-list notices corruption (2)' ' - test_must_fail git rev-list --objects HEAD -+ ( -+ GIT_TEST_COMMIT_GRAPH=0 && -+ test_must_fail git rev-list --objects HEAD -+ ) ++ test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --objects HEAD ' test_expect_success 'pack-objects notices corruption' ' @@ -140,10 +135,7 @@ - test_must_fail git rev-list --all > /dev/null - ' +test_expect_success 'rev-list should fail' ' -+ ( -+ GIT_TEST_COMMIT_GRAPH=0 && -+ test_must_fail git rev-list --all > /dev/null -+ ) ++ test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --all > /dev/null +' test_expect_success 'git repack _MUST_ fail' \ @@ -160,10 +152,7 @@ - test_must_fail git merge -m final G -" +test_expect_success 'combined merge conflicts' ' -+ ( -+ GIT_TEST_COMMIT_GRAPH=0 && -+ test_must_fail git merge -m final G -+ ) ++ test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge -m final G +' cat > expect << EOF -- gitgitgadget ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH 2018-08-29 12:49 ` [PATCH v2 " Derrick Stolee via GitGitGadget @ 2018-08-29 12:49 ` Derrick Stolee via GitGitGadget 2018-09-04 16:49 ` [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Duy Nguyen 1 sibling, 0 replies; 18+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-08-29 12:49 UTC (permalink / raw) To: git; +Cc: pclouds, Junio C Hamano, Derrick Stolee From: Derrick Stolee <dstolee@microsoft.com> The commit-graph feature is tested in isolation by t5318-commit-graph.sh and t6600-test-reach.sh, but there are many more interesting scenarios involving commit walks. Many of these scenarios are covered by the existing test suite, but we need to maintain coverage when the optional commit-graph structure is not present. To allow running the full test suite with the commit-graph present, add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try to load the commit-graph when parsing commits, and writes the commit-graph file after every 'git commit' command. There are a few tests that rely on commits not existing in pack-files to trigger important events, so manually set GIT_TEST_COMMIT_GRAPH to false for the necessary commands. There is one test in t6024-recursive-merge.sh that relies on the merge-base algorithm picking one of two ambiguous merge-bases, and the commit-graph feature changes which merge-base is picked. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- builtin/commit.c | 4 ++++ commit-graph.c | 5 +++-- commit-graph.h | 2 ++ t/README | 4 ++++ t/t0410-partial-clone.sh | 2 +- t/t5307-pack-missing-commit.sh | 4 ++-- t/t6011-rev-list-with-bad-commit.sh | 7 +++---- t/t6024-recursive-merge.sh | 6 +++--- 8 files changed, 22 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 158e3f843a..a25e652102 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -33,6 +33,7 @@ #include "sequencer.h" #include "mailmap.h" #include "help.h" +#include "commit-graph.h" static const char * const builtin_commit_usage[] = { N_("git commit [<options>] [--] <pathspec>..."), @@ -1651,6 +1652,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "new_index file. Check that disk is not full and quota is\n" "not exceeded, and then \"git reset HEAD\" to recover.")); + if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0)) + write_commit_graph_reachable(get_object_directory(), 0); + rerere(0); run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); diff --git a/commit-graph.c b/commit-graph.c index 4bd1a4abbf..5cae83fe03 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -237,8 +237,9 @@ static int prepare_commit_graph(struct repository *r) return !!r->objects->commit_graph; r->objects->commit_graph_attempted = 1; - if (repo_config_get_bool(r, "core.commitgraph", &config_value) || - !config_value) + if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && + (repo_config_get_bool(r, "core.commitgraph", &config_value) || + !config_value)) /* * This repository is not configured to use commit graphs, so * do not load one. (But report commit_graph_attempted anyway diff --git a/commit-graph.h b/commit-graph.h index 13d736cdde..cf9141f356 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -5,6 +5,8 @@ #include "repository.h" #include "string-list.h" +#define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH" + struct commit; char *get_commit_graph_filename(const char *obj_dir); diff --git a/t/README b/t/README index 8373a27fea..2b90c433f5 100644 --- a/t/README +++ b/t/README @@ -315,6 +315,10 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than <n> bytes. +GIT_TEST_COMMIT_GRAPH=<boolean>, when true, forces the commit-graph to +be written after every 'git commit' command, and overrides the +'core.commitGraph' setting to true. + Naming Tests ------------ diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 4984ca583d..73d5284a91 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' ' git -C repo config core.repositoryformatversion 1 && git -C repo config extensions.partialclone "arbitrary string" && - git -C repo rev-list --exclude-promisor-objects --objects bar >out && + GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out && grep $(git -C repo rev-parse bar) out && ! grep $FOO out ' diff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh index ae52a1882d..dacb440b27 100755 --- a/t/t5307-pack-missing-commit.sh +++ b/t/t5307-pack-missing-commit.sh @@ -24,11 +24,11 @@ test_expect_success 'check corruption' ' ' test_expect_success 'rev-list notices corruption (1)' ' - test_must_fail git rev-list HEAD + test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list HEAD ' test_expect_success 'rev-list notices corruption (2)' ' - test_must_fail git rev-list --objects HEAD + test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --objects HEAD ' test_expect_success 'pack-objects notices corruption' ' diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh index e51eb41f4b..545b461e51 100755 --- a/t/t6011-rev-list-with-bad-commit.sh +++ b/t/t6011-rev-list-with-bad-commit.sh @@ -41,10 +41,9 @@ test_expect_success 'corrupt second commit object' \ test_must_fail git fsck --full ' -test_expect_success 'rev-list should fail' \ - ' - test_must_fail git rev-list --all > /dev/null - ' +test_expect_success 'rev-list should fail' ' + test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git rev-list --all > /dev/null +' test_expect_success 'git repack _MUST_ fail' \ ' diff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh index 3f59e58dfb..27c7de90ce 100755 --- a/t/t6024-recursive-merge.sh +++ b/t/t6024-recursive-merge.sh @@ -60,9 +60,9 @@ git update-index a1 && GIT_AUTHOR_DATE="2006-12-12 23:00:08" git commit -m F ' -test_expect_success "combined merge conflicts" " - test_must_fail git merge -m final G -" +test_expect_success 'combined merge conflicts' ' + test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge -m final G +' cat > expect << EOF <<<<<<< HEAD -- gitgitgadget ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-08-29 12:49 ` [PATCH v2 " Derrick Stolee via GitGitGadget 2018-08-29 12:49 ` [PATCH v2 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget @ 2018-09-04 16:49 ` Duy Nguyen 2018-09-04 17:12 ` Derrick Stolee 1 sibling, 1 reply; 18+ messages in thread From: Duy Nguyen @ 2018-09-04 16:49 UTC (permalink / raw) To: gitgitgadget; +Cc: Git Mailing List, Junio C Hamano On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > The commit-graph (and multi-pack-index) features are optional data > structures that can make Git operations faster. Since they are optional, we > do not enable them in most Git tests. The commit-graph is tested in > t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that > one script cannot cover the data shapes present in the rest of the test > suite. > > This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH > . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it > with every git commit command. Thanks, Duy, for pointing out this direction > [1]. Any reason to not add this new flag in ci/run-build-and-tests.sh (which is used by Travis)? I see your note about VSTS but I don't see why it has to be exclusive to VSTS. > A few tests needed to be modified. These are the same tests that were > mentioned in my previous example patch [2]. Thanks, Eric, for providing the > correct way to override the settings [3]. > > When this merges down, I'll create a CI build in VSTS that runs the test > suite with this option enabled. -- Duy ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-09-04 16:49 ` [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Duy Nguyen @ 2018-09-04 17:12 ` Derrick Stolee 2018-09-04 17:18 ` Duy Nguyen 0 siblings, 1 reply; 18+ messages in thread From: Derrick Stolee @ 2018-09-04 17:12 UTC (permalink / raw) To: Duy Nguyen, gitgitgadget; +Cc: Git Mailing List, Junio C Hamano On 9/4/2018 12:49 PM, Duy Nguyen wrote: > On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> The commit-graph (and multi-pack-index) features are optional data >> structures that can make Git operations faster. Since they are optional, we >> do not enable them in most Git tests. The commit-graph is tested in >> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that >> one script cannot cover the data shapes present in the rest of the test >> suite. >> >> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH >> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it >> with every git commit command. Thanks, Duy, for pointing out this direction >> [1]. > Any reason to not add this new flag in ci/run-build-and-tests.sh > (which is used by Travis)? I see your note about VSTS but I don't see > why it has to be exclusive to VSTS. I only wanted to volunteer resources that I know are available. I am looking into an additional test run in Travis CI builds, but didn't want to presume the extra resources would be available. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage 2018-09-04 17:12 ` Derrick Stolee @ 2018-09-04 17:18 ` Duy Nguyen 0 siblings, 0 replies; 18+ messages in thread From: Duy Nguyen @ 2018-09-04 17:18 UTC (permalink / raw) To: Derrick Stolee; +Cc: gitgitgadget, Git Mailing List, Junio C Hamano On Tue, Sep 04, 2018 at 01:12:55PM -0400, Derrick Stolee wrote: > On 9/4/2018 12:49 PM, Duy Nguyen wrote: > > On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > >> The commit-graph (and multi-pack-index) features are optional data > >> structures that can make Git operations faster. Since they are optional, we > >> do not enable them in most Git tests. The commit-graph is tested in > >> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that > >> one script cannot cover the data shapes present in the rest of the test > >> suite. > >> > >> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH > >> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it > >> with every git commit command. Thanks, Duy, for pointing out this direction > >> [1]. > > Any reason to not add this new flag in ci/run-build-and-tests.sh > > (which is used by Travis)? I see your note about VSTS but I don't see > > why it has to be exclusive to VSTS. > > I only wanted to volunteer resources that I know are available. I am > looking into an additional test run in Travis CI builds, but didn't want > to presume the extra resources would be available. > OK I think there's a misunderstanding. You would not need a separate test run for this, instead the second (existing) run just activates both split index and commit-graph together. The run may be a bit slower (doubtful since commit-graph is all about speed) but it would not cost much more resources. Such a change would look like this diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 2a5bff4a1c..185305e3e2 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -15,6 +15,7 @@ then export GIT_TEST_FULL_IN_PACK_ARRAY=true export GIT_TEST_OE_SIZE=10 export GIT_TEST_OE_DELTA_SIZE=5 + export GIT_TEST_COMMIT_GRAPH=yes make --quiet test fi -- Duy ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-09 5:53 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-28 20:33 [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Derrick Stolee via GitGitGadget 2018-08-28 20:33 ` [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget 2018-08-28 20:41 ` Stefan Beller 2018-08-28 21:31 ` Derrick Stolee 2018-08-28 21:59 ` Eric Sunshine 2018-08-29 12:14 ` Derrick Stolee 2018-10-08 13:43 ` Ævar Arnfjörð Bjarmason 2018-10-08 14:45 ` Derrick Stolee 2018-10-08 14:58 ` Ævar Arnfjörð Bjarmason 2018-10-08 15:01 ` Derrick Stolee 2018-10-09 5:53 ` Junio C Hamano 2018-08-28 20:37 ` [PATCH 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Stefan Beller 2018-08-28 21:32 ` Derrick Stolee 2018-08-29 12:49 ` [PATCH v2 " Derrick Stolee via GitGitGadget 2018-08-29 12:49 ` [PATCH v2 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH Derrick Stolee via GitGitGadget 2018-09-04 16:49 ` [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage Duy Nguyen 2018-09-04 17:12 ` Derrick Stolee 2018-09-04 17:18 ` Duy Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).