{"thread":{"id":"60020","subject":"[PATCH 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","startedAt":"2023-07-21T17:31:09Z","lastAt":"2023-07-24T21:48:44Z","messageCount":22,"participants":["Taylor Blau","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"479752","messageId":"084822126301c0e723155fd76942f2e259b77158.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:12Z","receivedAt":"2023-07-21T17:31:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `graph_read_expect()` function is used to ensure that the output of\nthe \"read-graph\" test helper matches certain parameters (e.g., how many\ncommits are in the graph, which chunks were written, etc.).\n\nIt expects the Git repository being tested to be at the current working\ndirectory. However, a handful of t5318 tests use different repositories\nstored in sub-directories. To work around this, several tests in t5318\nchange into the relevant repository outside of a sub-shell, altering the\ncontext for the rest of the suite.\n\nPrepare to remove these globally-scoped directory changes by teaching\n`graph_read_expect()` to take an optional \"-C dir\" to specify where the\nrepository containing the commit-graph being tested is.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex 5d79e1a4e96..c50553df0ed 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -32,6 +32,13 @@ graph_git_behavior() {\n graph_read_expect() {\n \tOPTIONAL=\"\"\n \tNUM_CHUNKS=3\n+\tDIR=\".\"\n+\tif test \"$1\" = -C\n+\tthen\n+\t\tshift\n+\t\tDIR=\"$1\"\n+\t\tshift\n+\tfi\n \tif test -n \"$2\"\n \tthen\n \t\tOPTIONAL=\" $2\"\n@@ -47,12 +54,15 @@ graph_read_expect() {\n \tthen\n \t\tOPTIONS=\" read_generation_data\"\n \tfi\n-\tcat >expect <<- EOF\n+\tcat >$DIR/expect <<- EOF\n \theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \toptions:$OPTIONS\n \tEOF\n-\ttest-tool read-graph >output &&\n-\ttest_cmp expect output\n+\t(\n+\t\tcd \"$DIR\" &&\n+\t\ttest-tool read-graph >output &&\n+\t\ttest_cmp expect output\n+\t)\n }\n-- \n2.41.0.381.gd8424d64777\n\n"},{"id":"479753","messageId":"ba550987055ea8ba3333f18c1a814602f124e419.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH 4/5] t5328: avoid top-level directory changes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:20Z","receivedAt":"2023-07-21T17:31:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In a similar spirit as the last commit, avoid top-level directory\nchanges in the last remaining commit-graph related test, t5328.\n\nSigned-off-by: Taylor Blau \n---\n t/t5328-commit-graph-64bit-time.sh | 54 +++++++++++++++---------------\n 1 file changed, 27 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh\nindex 57e4d9c6998..e9c521c061c 100755\n--- a/t/t5328-commit-graph-64bit-time.sh\n+++ b/t/t5328-commit-graph-64bit-time.sh\n@@ -37,39 +37,39 @@ test_expect_success 'lower layers have overflow chunk' '\n graph_git_behavior 'overflow' '' HEAD~2 HEAD\n \n test_expect_success 'set up and verify repo with generation data overflow chunk' '\n-\tmkdir repo &&\n-\tcd repo &&\n-\tgit init &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n-\ttest_commit 2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 3 generation_data &&\n-\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n-\ttest_commit 5 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n-\tgit branch left &&\n-\tgit reset --hard 3 &&\n-\ttest_commit 7 &&\n-\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n-\ttest_commit 9 &&\n-\tgit branch right &&\n-\tgit reset --hard 3 &&\n-\ttest_merge M left right &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n-\tgit commit-graph verify\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n+\t\ttest_commit 2 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 3 generation_data &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n+\t\ttest_commit 5 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n+\t\tgit branch left &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_commit 7 &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n+\t\ttest_commit 9 &&\n+\t\tgit branch right &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_merge M left right &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n+\t\tgit commit-graph verify\n+\t)\n '\n \n graph_git_behavior 'overflow 2' repo left right\n \n test_expect_success 'single commit with generation data exceeding UINT32_MAX' '\n \tgit init repo-uint32-max &&\n-\tcd repo-uint32-max &&\n-\ttest_commit --date \"@4294967297 +0000\" 1 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 1 \"generation_data\" &&\n-\tgit commit-graph verify\n+\ttest_commit -C repo-uint32-max --date \"@4294967297 +0000\" 1 &&\n+\tgit -C repo-uint32-max commit-graph write --reachable &&\n+\tgraph_read_expect -C repo-uint32-max 1 \"generation_data\" &&\n+\tgit -C repo-uint32-max commit-graph verify\n '\n \n test_done\n-- \n2.41.0.381.gd8424d64777\n\n"},{"id":"479756","messageId":"451ec003be8714bc38cc8cdc159add3e8899ce57.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH 3/5] t5318: avoid top-level directory changes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:18Z","receivedAt":"2023-07-21T17:31:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Avoid changing the current working directory from outside of a sub-shell\nduring the tests in t5318.\n\nEach test has mostly straightforward changes, either:\n\n - Removing the top-level `cd \"$TRASH_DIRECTORY/full\"`, which is\n unnecessary after ensuring that other tests don't change their\n working directory outside of a sub-shell.\n\n - Changing any Git invocations which want to be in a sub-directory by\n either (a) adding a \"-C $DIR\" argument, or (b) moving the whole test\n into a sub-shell.\n\nWhile we're here, remove any explicit \"git config core.commitGraph true\"\ninvocations which were designed to enable use of the commit-graph. These\nare unnecessary following 31b1de6a09b (commit-graph: turn on\ncommit-graph by default, 2019-08-13).\n\nSigned-off-by: Taylor Blau \n---\n t/t5318-commit-graph.sh | 378 ++++++++++++++++++----------------------\n 1 file changed, 172 insertions(+), 206 deletions(-)\n\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex bf8a92317b3..4df76173a8d 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -24,12 +24,10 @@ test_expect_success 'usage shown with an error on unknown sub-command' '\n \ttest_cmp expect actual\n '\n \n+objdir=\".git/objects\"\n+\n test_expect_success 'setup full repo' '\n-\tmkdir full &&\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit init &&\n-\tgit config core.commitGraph true &&\n-\tobjdir=\".git/objects\"\n+\tgit init full\n '\n \n test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n@@ -37,31 +35,28 @@ test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n '\n \n test_expect_success 'verify graph with no graph file' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify\n+\tgit -C full commit-graph verify\n '\n \n test_expect_success 'write graph with no packs' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --object-dir $objdir &&\n-\ttest_path_is_missing $objdir/info/commit-graph\n+\tgit -C full commit-graph write --object-dir $objdir &&\n+\ttest_path_is_missing full/$objdir/info/commit-graph\n '\n \n test_expect_success 'exit with correct error on bad input to --stdin-packs' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \techo doesnotexist >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-packs stderr &&\n+\ttest_expect_code 1 git -C full commit-graph write --stdin-packs \\\n+\t\tstderr &&\n \ttest_i18ngrep \"error adding pack\" stderr\n '\n \n test_expect_success 'create commits and repack' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \tfor i in $(test_seq 3)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit repack\n+\tgit -C full repack\n '\n \n . \"$TEST_DIRECTORY\"/lib-commit-graph.sh\n@@ -69,117 +64,106 @@ test_expect_success 'create commits and repack' '\n graph_git_behavior 'no graph' full commits/3 commits/1\n \n test_expect_success 'exit with correct error on bad input to --stdin-commits' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \t# invalid, non-hex OID\n-\techo HEAD >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-commits stderr &&\n+\techo HEAD | test_expect_code 1 git -C full commit-graph write \\\n+\t\t--stdin-commits 2>stderr &&\n \ttest_i18ngrep \"unexpected non-hex object ID: HEAD\" stderr &&\n \t# non-existent OID\n-\techo $ZERO_OID >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-commits stderr &&\n+\techo $ZERO_OID | test_expect_code 1 git -C full commit-graph write \\\n+\t\t--stdin-commits 2>stderr &&\n \ttest_i18ngrep \"invalid object\" stderr &&\n \t# valid commit and tree OID\n-\tgit rev-parse HEAD HEAD^{tree} >in &&\n-\tgit commit-graph write --stdin-commits in &&\n+\tgit -C full commit-graph write --stdin-commits expect &&\n-\ttest_modebits $objdir/info/commit-graph >actual &&\n+\ttest_modebits full/$objdir/info/commit-graph >actual &&\n \ttest_cmp expect actual\n '\n \n graph_git_behavior 'graph exists' full commits/3 commits/1\n \n test_expect_success 'Add more commits' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit reset --hard commits/1 &&\n+\tgit -C full reset --hard commits/1 &&\n \tfor i in $(test_seq 4 5)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit reset --hard commits/2 &&\n+\tgit -C full reset --hard commits/2 &&\n \tfor i in $(test_seq 6 7)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit reset --hard commits/2 &&\n-\tgit merge commits/4 &&\n-\tgit branch merge/1 &&\n-\tgit reset --hard commits/4 &&\n-\tgit merge commits/6 &&\n-\tgit branch merge/2 &&\n-\tgit reset --hard commits/3 &&\n-\tgit merge commits/5 commits/7 &&\n-\tgit branch merge/3 &&\n-\tgit repack\n+\tgit -C full reset --hard commits/2 &&\n+\tgit -C full merge commits/4 &&\n+\tgit -C full branch merge/1 &&\n+\tgit -C full reset --hard commits/4 &&\n+\tgit -C full merge commits/6 &&\n+\tgit -C full branch merge/2 &&\n+\tgit -C full reset --hard commits/3 &&\n+\tgit -C full merge commits/5 commits/7 &&\n+\tgit -C full branch merge/3 &&\n+\tgit -C full repack\n '\n \n test_expect_success 'commit-graph write progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write 2>err &&\n+\tgit -C full commit-graph write 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph write --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n test_expect_success 'commit-graph write with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --no-progress 2>err &&\n+\tgit -C full commit-graph write --no-progress 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write --stdin-commits progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tgit commit-graph write --stdin-commits err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tgit -C full commit-graph write --stdin-commits err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write --stdin-commits force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph write --stdin-commits --progress err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph write --stdin-commits \\\n+\t\t--progress err &&\n \ttest_i18ngrep \"Collecting commits from input\" err\n '\n \n test_expect_success 'commit-graph write --stdin-commits with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tgit commit-graph write --stdin-commits --no-progress err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tgit -C full commit-graph write --stdin-commits --no-progress err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph verify progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify 2>err &&\n+\tgit -C full commit-graph verify 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph verify force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph verify --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n test_expect_success 'commit-graph verify with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify --no-progress 2>err &&\n+\tgit -C full commit-graph verify --no-progress 2>err &&\n \ttest_must_be_empty err\n '\n \n@@ -194,10 +178,9 @@ test_expect_success 'commit-graph verify with the --no-progress option' '\n # 1\n \n test_expect_success 'write graph with merges' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"10\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 10 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'merge 1 vs 2' full merge/1 merge/2\n@@ -205,12 +188,11 @@ graph_git_behavior 'merge 1 vs 3' full merge/1 merge/3\n graph_git_behavior 'merge 2 vs 3' full merge/2 merge/3\n \n test_expect_success 'Add one more commit' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_commit 8 &&\n-\tgit branch commits/8 &&\n-\tls $objdir/pack | grep idx >existing-idx &&\n-\tgit repack &&\n-\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n+\ttest_commit -C full 8 &&\n+\tgit -C full branch commits/8 &&\n+\tls full/$objdir/pack | grep idx >existing-idx &&\n+\tgit -C full repack &&\n+\tls full/$objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n '\n \n # Current graph structure:\n@@ -229,114 +211,101 @@ graph_git_behavior 'mixed mode, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'mixed mode, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'write graph with new commit' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"11\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 11 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'full graph, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'full graph, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'write graph with nothing new' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"11\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 11 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'cleared graph, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'cleared graph, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'build graph from latest pack with closure' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tcat new-idx | git commit-graph write --stdin-packs &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"9\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write --stdin-packs commits-in &&\n-\tgit rev-parse merge/1 >>commits-in &&\n-\tcat commits-in | git commit-graph write --stdin-commits &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"6\" \"generation_data\"\n+\tgit -C full tag -a -m \"merge\" tag/merge merge/2 &&\n+\tgit -C full rev-parse tag/merge >commits-in &&\n+\tgit -C full rev-parse merge/1 >>commits-in &&\n+\tgit -C full commit-graph write --stdin-commits in &&\n+\tgit -C full commit-graph write --stdin-commits --append output &&\n-\tgit show-ref -s commits/8 >expect &&\n+\tgit -C full checkout -b merge-5-to-8 commits/5 &&\n+\tgit -C full merge commits/8 &&\n+\tgit -C full show-ref -s merge-5-to-8 >output &&\n+\tgit -C full show-ref -s commits/8 >expect &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'check that gc computes commit-graph' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit --allow-empty -m \"blank\" &&\n-\tgit commit-graph write --reachable &&\n-\tcp $objdir/info/commit-graph commit-graph-before-gc &&\n-\tgit reset --hard HEAD~1 &&\n-\tgit config gc.writeCommitGraph true &&\n-\tgit gc &&\n-\tcp $objdir/info/commit-graph commit-graph-after-gc &&\n+\ttest_commit -C full --no-tag blank &&\n+\tgit -C full commit-graph write --reachable &&\n+\tcp full/$objdir/info/commit-graph commit-graph-before-gc &&\n+\tgit -C full reset --hard HEAD~1 &&\n+\ttest_config -C full gc.writeCommitGraph true &&\n+\tgit -C full gc &&\n+\tcp full/$objdir/info/commit-graph commit-graph-after-gc &&\n \t! test_cmp_bin commit-graph-before-gc commit-graph-after-gc &&\n-\tgit commit-graph write --reachable &&\n-\ttest_cmp_bin commit-graph-after-gc $objdir/info/commit-graph\n+\tgit -C full commit-graph write --reachable &&\n+\ttest_cmp_bin commit-graph-after-gc full/$objdir/info/commit-graph\n '\n \n test_expect_success 'replace-objects invalidates commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf replace &&\n \tgit clone full replace &&\n \t(\n@@ -359,7 +328,6 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n '\n \n test_expect_success 'commit grafts invalidate commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf graft &&\n \tgit clone --template= full graft &&\n \t(\n@@ -384,7 +352,6 @@ test_expect_success 'commit grafts invalidate commit-graph' '\n '\n \n test_expect_success 'replace-objects invalidates commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf shallow &&\n \tgit clone --depth 2 \"file://$TRASH_DIRECTORY/full\" shallow &&\n \t(\n@@ -427,24 +394,25 @@ test_expect_success 'warn on improper hash version' '\n '\n \n test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow chunk' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \tUNIX_EPOCH_ZERO=\"@0 +0000\" &&\n \tFUTURE_DATE=\"@4147483646 +0000\" &&\n-\trm -f .git/objects/info/commit-graph &&\n-\ttest_commit --date \"$FUTURE_DATE\" future-1 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" old-1 &&\n-\tgit commit-graph write --reachable &&\n-\ttest_commit --date \"$FUTURE_DATE\" future-2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" old-2 &&\n-\tgit commit-graph write --reachable --split=no-merge &&\n-\ttest_commit extra &&\n-\tgit commit-graph write --reachable --split=no-merge &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 16 \"generation_data generation_data_overflow extra_edges\" &&\n-\tmv .git/objects/info/commit-graph commit-graph-upgraded &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 16 \"generation_data generation_data_overflow extra_edges\" &&\n-\ttest_cmp .git/objects/info/commit-graph commit-graph-upgraded\n+\trm -f full/.git/objects/info/commit-graph &&\n+\ttest_commit -C full --date \"$FUTURE_DATE\" future-1 &&\n+\ttest_commit -C full --date \"$UNIX_EPOCH_ZERO\" old-1 &&\n+\tgit -C full commit-graph write --reachable &&\n+\ttest_commit -C full --date \"$FUTURE_DATE\" future-2 &&\n+\ttest_commit -C full --date \"$UNIX_EPOCH_ZERO\" old-2 &&\n+\tgit -C full commit-graph write --reachable --split=no-merge &&\n+\ttest_commit -C full extra &&\n+\tgit -C full commit-graph write --reachable --split=no-merge &&\n+\tgit -C full commit-graph write --reachable &&\n+\tgraph_read_expect -C full 16 \\\n+\t\t\"generation_data generation_data_overflow extra_edges\" &&\n+\tmv full/.git/objects/info/commit-graph commit-graph-upgraded &&\n+\tgit -C full commit-graph write --reachable &&\n+\tgraph_read_expect -C full 16 \\\n+\t\t\"generation_data generation_data_overflow extra_edges\" &&\n+\ttest_cmp full/.git/objects/info/commit-graph commit-graph-upgraded\n '\n \n # the verify tests below expect the commit-graph to contain\n@@ -454,10 +422,11 @@ test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow ch\n # and the tests will likely break.\n \n test_expect_success 'git commit-graph verify' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/8 | git -c commitGraph.generationVersion=1 commit-graph write --stdin-commits &&\n-\tgit commit-graph verify >output &&\n-\tgraph_read_expect 9 extra_edges 1\n+\tgit -C full rev-parse commits/8 >in &&\n+\tgit -C full -c commitGraph.generationVersion=1 commit-graph write \\\n+\t\t--stdin-commits output &&\n+\tgraph_read_expect -C full 9 extra_edges 1\n '\n \n NUM_COMMITS=9\n@@ -495,25 +464,24 @@ GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))\n GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))\n \n corrupt_graph_setup() {\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n-\tcp $objdir/info/commit-graph commit-graph-backup &&\n-\tchmod u+w $objdir/info/commit-graph\n+\ttest_when_finished mv commit-graph-backup full/$objdir/info/commit-graph &&\n+\tcp full/$objdir/info/commit-graph commit-graph-backup &&\n+\tchmod u+w full/$objdir/info/commit-graph\n }\n \n corrupt_graph_verify() {\n \tgrepstr=$1\n-\ttest_must_fail git commit-graph verify 2>test_err &&\n+\ttest_must_fail git -C full commit-graph verify 2>test_err &&\n \tgrep -v \"^+\" test_err >err &&\n \ttest_i18ngrep \"$grepstr\" err &&\n \tif test \"$2\" != \"no-copy\"\n \tthen\n-\t\tcp $objdir/info/commit-graph commit-graph-pre-write-test\n+\t\tcp full/$objdir/info/commit-graph commit-graph-pre-write-test\n \tfi &&\n-\tgit status --short &&\n-\tGIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git commit-graph write &&\n-\tchmod u+w $objdir/info/commit-graph &&\n-\tgit commit-graph verify\n+\tgit -C full status --short &&\n+\tGIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git -C full commit-graph write &&\n+\tchmod u+w full/$objdir/info/commit-graph &&\n+\tgit -C full commit-graph verify\n }\n \n # usage: corrupt_graph_and_verify []\n@@ -527,24 +495,24 @@ corrupt_graph_and_verify() {\n \tdata=\"${2:-\\0}\"\n \tgrepstr=$3\n \tcorrupt_graph_setup &&\n-\torig_size=$(wc -c < $objdir/info/commit-graph) &&\n+\torig_size=$(wc -c >\"$objdir/info/commit-graph\" &&\n+\tprintf \"$data\" | dd of=\"full/$objdir/info/commit-graph\" bs=1 seek=\"$pos\" conv=notrunc &&\n+\tdd of=\"full/$objdir/info/commit-graph\" bs=1 seek=\"$zero_pos\" if=/dev/null &&\n+\ttest-tool genzeros $(($orig_size - $zero_pos)) >>\"full/$objdir/info/commit-graph\" &&\n \tcorrupt_graph_verify \"$grepstr\"\n \n }\n \n test_expect_success POSIXPERM,SANITY 'detect permission problem' '\n \tcorrupt_graph_setup &&\n-\tchmod 000 $objdir/info/commit-graph &&\n+\tchmod 000 full/$objdir/info/commit-graph &&\n \tcorrupt_graph_verify \"Could not open\" \"no-copy\"\n '\n \n test_expect_success 'detect too small' '\n \tcorrupt_graph_setup &&\n-\techo \"a small graph\" >$objdir/info/commit-graph &&\n+\techo \"a small graph\" >full/$objdir/info/commit-graph &&\n \tcorrupt_graph_verify \"too small\"\n '\n \n@@ -655,33 +623,30 @@ test_expect_success 'detect incorrect chunk count' '\n '\n \n test_expect_success 'git fsck (checks commit-graph when config set to true)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\ttest_must_fail git -c core.commitGraph=true fsck\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\ttest_must_fail git -C full -c core.commitGraph=true fsck\n '\n \n test_expect_success 'git fsck (ignores commit-graph when config set to false)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\tgit -c core.commitGraph=false fsck\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\tgit -C full -c core.commitGraph=false fsck\n '\n \n test_expect_success 'git fsck (checks commit-graph when config unset)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_when_finished \"git config core.commitGraph true\" &&\n+\ttest_when_finished \"git -C full config core.commitGraph true\" &&\n \n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\ttest_unconfig core.commitGraph &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\ttest_must_fail git fsck\n+\ttest_unconfig -C full core.commitGraph &&\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\ttest_must_fail git -C full fsck\n '\n \n test_expect_success 'git fsck shows commit-graph output with --progress' '\n@@ -792,32 +757,33 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '\n #\n \n test_expect_success 'set up and verify repo with generation data overflow chunk' '\n-\tobjdir=\".git/objects\" &&\n \tUNIX_EPOCH_ZERO=\"@0 +0000\" &&\n \tFUTURE_DATE=\"@2147483646 +0000\" &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\tmkdir repo &&\n-\tcd repo &&\n-\tgit init &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n-\ttest_commit 2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 3 generation_data &&\n-\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n-\ttest_commit 5 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n-\tgit branch left &&\n-\tgit reset --hard 3 &&\n-\ttest_commit 7 &&\n-\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n-\ttest_commit 9 &&\n-\tgit branch right &&\n-\tgit reset --hard 3 &&\n-\ttest_merge M left right &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n-\tgit commit-graph verify\n+\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n+\t\ttest_commit 2 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 3 generation_data &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n+\t\ttest_commit 5 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n+\t\tgit branch left &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_commit 7 &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n+\t\ttest_commit 9 &&\n+\t\tgit branch right &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_merge M left right &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n+\t\tgit commit-graph verify\n+\t)\n '\n \n graph_git_behavior 'generation data overflow chunk repo' repo left right\n-- \n2.41.0.381.gd8424d64777\n\n"},{"id":"479754","messageId":"715a160903be6dc8873a7215c55f90894e62f589.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:15Z","receivedAt":"2023-07-21T17:31:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `graph_git_behavior()` helper asserts that a number of common Git\noperations (such as `git log --oneline`, `git log --topo-order`, etc.)\nproduce identical output regardless of whether or not a commit-graph is\nin use.\n\nThis helper takes as its second argument the location (relative to the\n`$TRASH_DIRECTORY`) of the Git repostiory under test. In order to run\neach of its commands within that repository, it first changes into that\ndirectory, without the use of a sub-shell.\n\nThis pollutes future tests which expect to be run in the top-level\n`$TRASH_DIRECTORY` as usual. We could wrap `graph_git_behavior()` in a\nsub-shell, like:\n\n graph_git_behavior() {\n # ...\n (\n cd \"$TRASH_DIRECTORY/$DIR\" &&\n graph_git_two_modesl\n )\n }\n\n, but since we're invoking git directly, we can pass along a \"-C $DIR\"\nwhen \"$DIR\" is non-empty.\n\nNote, however, that until the remaining callers are cleaned up to avoid\nchanging working directories outside of a sub-shell, that we need to\nensure that we are operating in the top-level $TRASH_DIRECTORY. The\ninner-subshell will go away in a future commit once it is no longer\nnecessary.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex c50553df0ed..c93969ae74d 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -20,12 +20,14 @@ graph_git_behavior() {\n \tBRANCH=$3\n \tCOMPARE=$4\n \ttest_expect_success \"check normal git operations: $MSG\" '\n-\t\tcd \"$TRASH_DIRECTORY/$DIR\" &&\n-\t\tgraph_git_two_modes \"log --oneline $BRANCH\" &&\n-\t\tgraph_git_two_modes \"log --topo-order $BRANCH\" &&\n-\t\tgraph_git_two_modes \"log --graph $COMPARE..$BRANCH\" &&\n-\t\tgraph_git_two_modes \"branch -vv\" &&\n-\t\tgraph_git_two_modes \"merge-base -a $BRANCH $COMPARE\"\n+\t\t(\n+\t\t\tcd \"$TRASH_DIRECTORY\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n+\t\t)\n \t'\n }\n \n-- \n2.41.0.381.gd8424d64777\n\n"},{"id":"479755","messageId":"cover.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":null,"subject":"[PATCH 0/5] commit-graph: test cleanup and modernization","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:08Z","receivedAt":"2023-07-21T17:31:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"This short series addresses a few style nitpicks that I noticed while\nlooking through the commit-graph tests while writing [1].\n\nMost importantly, it removes many instances in t5318 that change\ndirectories outside of a sub-shell, altering the current working\ndirectory of subsequent tests. This makes it difficult to run a subset\nof tests, or otherwise include `cd \"$TRASH_DIRECTORY\"` at the top of\neach test.\n\nThe first two patches are predatory, the next two are the substantive\ntest clean-ups, and the final patch cleans up some intermediate state\nnecessary to perform the clean-up over multiple commits.\n\nThanks in advance for your review.\n\n[1]: https://lore.kernel.org/git/cover.1688776280.git.me@ttaylorr.com/\n\nTaylor Blau (5):\n t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories\n t/lib-commit-graph.sh: avoid directory change in\n `graph_git_behavior()`\n t5318: avoid top-level directory changes\n t5328: avoid top-level directory changes\n t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()`\n\n t/lib-commit-graph.sh | 27 ++-\n t/t5318-commit-graph.sh | 378 +++++++++++++----------------\n t/t5328-commit-graph-64bit-time.sh | 54 ++---\n 3 files changed, 217 insertions(+), 242 deletions(-)\n\n-- \n2.41.0.381.gd8424d64777\n"},{"id":"479757","messageId":"c3432f27b94bbd72d3e721bfede94d158acd20ae.1689960606.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH 5/5] t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T17:30:23Z","receivedAt":"2023-07-21T17:31:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In a previous commit, we introduced a sub-shell in the implementation of\n`graph_git_behavior()`, in order to allow us to pass `-C \"$DIR\"`\ndirectly to the git processes spawned by `graph_git_two_modes()`.\n\nNow that its callers are always operating from the \"$TRASH_DIRECTORY\"\ninstead of one of its sub-directories, we can drop the inner sub-shell,\nas it is no longer required.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex c93969ae74d..b7b52b4291b 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -20,14 +20,11 @@ graph_git_behavior() {\n \tBRANCH=$3\n \tCOMPARE=$4\n \ttest_expect_success \"check normal git operations: $MSG\" '\n-\t\t(\n-\t\t\tcd \"$TRASH_DIRECTORY\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n-\t\t)\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n \t'\n }\n \n-- \n2.41.0.381.gd8424d64777\n"},{"id":"479759","messageId":"CAPig+cQ2h+67Oxnt7FvZL-LrMF9RazfjLm5pkw4FUh3UU0T+4Q@mail.gmail.com","threadId":"60020","inReplyTo":"084822126301c0e723155fd76942f2e259b77158.1689960606.git.me@ttaylorr.com","subject":"Re: [PATCH 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-07-21T17:41:06Z","receivedAt":"2023-07-21T17:41:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 21, 2023 at 1:31 PM Taylor Blau wrote:\n> The `graph_read_expect()` function is used to ensure that the output of\n> the \"read-graph\" test helper matches certain parameters (e.g., how many\n> commits are in the graph, which chunks were written, etc.).\n>\n> It expects the Git repository being tested to be at the current working\n> directory. However, a handful of t5318 tests use different repositories\n> stored in sub-directories. To work around this, several tests in t5318\n> change into the relevant repository outside of a sub-shell, altering the\n> context for the rest of the suite.\n>\n> Prepare to remove these globally-scoped directory changes by teaching\n> `graph_read_expect()` to take an optional \"-C dir\" to specify where the\n> repository containing the commit-graph being tested is.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\n> @@ -32,6 +32,13 @@ graph_git_behavior() {\n> graph_read_expect() {\n> + DIR=\".\"\n> + if test \"$1\" = -C\n> + then\n> + shift\n> + DIR=\"$1\"\n> + shift\n> + fi\n> @@ -47,12 +54,15 @@ graph_read_expect() {\n> - cat >expect <<- EOF\n> + cat >$DIR/expect <<- EOF\n\nIt may not matter for any of the current callers, but we'd normally\nwant to quote the expansion of $DIR. Also, as I recall, some versions\nof bash complain if the target of '>' is not quoted. So:\n\n cat >\"$DIR/expect\" <<-EOF\n\n> header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n> num_commits: $1\n> chunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n> options:$OPTIONS\n> EOF\n> - test-tool read-graph >output &&\n> - test_cmp expect output\n> + (\n> + cd \"$DIR\" &&\n> + test-tool read-graph >output &&\n> + test_cmp expect output\n> + )\n> }\n"},{"id":"479760","messageId":"CAPig+cTse-bgpApQR3jVGir0Tur22NEgAoTKpmLek+pTRovxwg@mail.gmail.com","threadId":"60020","inReplyTo":"715a160903be6dc8873a7215c55f90894e62f589.1689960606.git.me@ttaylorr.com","subject":"Re: [PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-07-21T18:01:58Z","receivedAt":"2023-07-21T18:02:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 21, 2023 at 1:32 PM Taylor Blau wrote:\n> The `graph_git_behavior()` helper asserts that a number of common Git\n> operations (such as `git log --oneline`, `git log --topo-order`, etc.)\n> produce identical output regardless of whether or not a commit-graph is\n> in use.\n>\n> This helper takes as its second argument the location (relative to the\n> `$TRASH_DIRECTORY`) of the Git repostiory under test. In order to run\n> each of its commands within that repository, it first changes into that\n> directory, without the use of a sub-shell.\n>\n> This pollutes future tests which expect to be run in the top-level\n> `$TRASH_DIRECTORY` as usual. We could wrap `graph_git_behavior()` in a\n> sub-shell, like:\n>\n> graph_git_behavior() {\n> # ...\n> (\n> cd \"$TRASH_DIRECTORY/$DIR\" &&\n> graph_git_two_modesl\n> )\n> }\n>\n> , but since we're invoking git directly, we can pass along a \"-C $DIR\"\n> when \"$DIR\" is non-empty.\n>\n> Note, however, that until the remaining callers are cleaned up to avoid\n> changing working directories outside of a sub-shell, that we need to\n> ensure that we are operating in the top-level $TRASH_DIRECTORY. The\n> inner-subshell will go away in a future commit once it is no longer\n> necessary.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\n> @@ -20,12 +20,14 @@ graph_git_behavior() {\n> test_expect_success \"check normal git operations: $MSG\" '\n> - cd \"$TRASH_DIRECTORY/$DIR\" &&\n> - graph_git_two_modes \"log --oneline $BRANCH\" &&\n> - graph_git_two_modes \"log --topo-order $BRANCH\" &&\n> - graph_git_two_modes \"log --graph $COMPARE..$BRANCH\" &&\n> - graph_git_two_modes \"branch -vv\" &&\n> - graph_git_two_modes \"merge-base -a $BRANCH $COMPARE\"\n> + (\n> + cd \"$TRASH_DIRECTORY\" &&\n> + graph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n> + graph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n> + graph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n> + graph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n> + graph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n> + )\n> '\n> }\n\nAs mentioned in my review of patch [1/5], for safety, you'd probably\nwant to quote the expansion of DIR in case it ever contains whitespace\n(or other weird characters). The obvious POSIX-correct way to do this\nwould be:\n\n graph_git_two_modes \"${DIR:+-C \\\"$DIR\\\"} log ...\" &&\n\nUnfortunately, however, some older broken shells incorrectly expand\nthis to a single argument (\"-C \") rather than the expected two\narguments (-C and \"\")[1,2,3,4]. The workaround is unsightly but\ndoable:\n\n graph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log ...\" &&\n\n[1]: https://lore.kernel.org/git/20160517215214.GA16905@sigill.intra.peff.net/\n[2]: https://lore.kernel.org/git/e3bfc53363b14826d828e1adffbbeea@74d39fa044aa309eaea14b9f57fe79c/\n[3]: https://lore.kernel.org/git/20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@webmail.informatik.kit.edu/\n[4]: https://lore.kernel.org/git/1240044459-57227-1-git-send-email-ben@ben.com/\n"},{"id":"479762","messageId":"CAPig+cSkoHnUY0obKkz93k7mHXrFj+kwJ21yPHy=KYroAQtiUA@mail.gmail.com","threadId":"60020","inReplyTo":"451ec003be8714bc38cc8cdc159add3e8899ce57.1689960606.git.me@ttaylorr.com","subject":"Re: [PATCH 3/5] t5318: avoid top-level directory changes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-07-21T18:28:17Z","receivedAt":"2023-07-21T18:28:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 21, 2023 at 1:33 PM Taylor Blau wrote:\n> Avoid changing the current working directory from outside of a sub-shell\n> during the tests in t5318.\n>\n> Each test has mostly straightforward changes, either:\n>\n> - Removing the top-level `cd \"$TRASH_DIRECTORY/full\"`, which is\n> unnecessary after ensuring that other tests don't change their\n> working directory outside of a sub-shell.\n>\n> - Changing any Git invocations which want to be in a sub-directory by\n> either (a) adding a \"-C $DIR\" argument, or (b) moving the whole test\n> into a sub-shell.\n>\n> While we're here, remove any explicit \"git config core.commitGraph true\"\n> invocations which were designed to enable use of the commit-graph. These\n> are unnecessary following 31b1de6a09b (commit-graph: turn on\n> commit-graph by default, 2019-08-13).\n>\n> Signed-off-by: Taylor Blau \n> ---\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> @@ -69,117 +64,106 @@ test_expect_success 'create commits and repack' '\n> test_expect_success 'Add more commits' '\n> ...\n> - git reset --hard commits/2 &&\n> - git merge commits/4 &&\n> - git branch merge/1 &&\n> - git reset --hard commits/4 &&\n> - git merge commits/6 &&\n> - git branch merge/2 &&\n> - git reset --hard commits/3 &&\n> - git merge commits/5 commits/7 &&\n> - git branch merge/3 &&\n> - git repack\n> + git -C full reset --hard commits/2 &&\n> + git -C full merge commits/4 &&\n> + git -C full branch merge/1 &&\n> + git -C full reset --hard commits/4 &&\n> + git -C full merge commits/6 &&\n> + git -C full branch merge/2 &&\n> + git -C full reset --hard commits/3 &&\n> + git -C full merge commits/5 commits/7 &&\n> + git -C full branch merge/3 &&\n> + git -C full repack\n> '\n\nThis is one of those cases in which \"cd full\" while in subshell,\nrather than using \"-C full\" repeatedly, would perhaps make the code a\nbit less noisy, but it is, of course, subjective and what you have\nhere is good enough.\n\n> @@ -229,114 +211,101 @@ graph_git_behavior 'mixed mode, commit 8 vs merge 1' full commits/8 merge/1\n> test_expect_success 'build graph from latest pack with closure' '\n> - cd \"$TRASH_DIRECTORY/full\" &&\n> - cat new-idx | git commit-graph write --stdin-packs &&\n> - test_path_is_file $objdir/info/commit-graph &&\n> - graph_read_expect \"9\" \"generation_data extra_edges\"\n> + git -C full commit-graph write --stdin-packs + test_path_is_file full/$objdir/info/commit-graph &&\n> + graph_read_expect -C full 9 \"generation_data extra_edges\"\n> '\n\nThis works because an earlier test created \"new-idx\", and it created\nit directly in the trash-directory rather than in the \"full\"\nsubdirectory where it used to reside before this patch. Okay.\n\n> @@ -495,25 +464,24 @@ GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))\n> corrupt_graph_setup() {\n> - cd \"$TRASH_DIRECTORY/full\" &&\n> - test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n> - cp $objdir/info/commit-graph commit-graph-backup &&\n> - chmod u+w $objdir/info/commit-graph\n> + test_when_finished mv commit-graph-backup full/$objdir/info/commit-graph &&\n> + cp full/$objdir/info/commit-graph commit-graph-backup &&\n> + chmod u+w full/$objdir/info/commit-graph\n> }\n\nPrior to this patch, \"commit-graph-backup\" was placed in the \"full\"\nsubdirectory and test_when_finished() removed it from that location.\nAs of this patch, \"commit-graph-backup\" is placed in the top-level\ntrash-directory and cleaned up from there. Okay.\n"},{"id":"479763","messageId":"ZLrPin9L9LtaDHaC@nand.local","threadId":"60020","inReplyTo":"CAPig+cQ2h+67Oxnt7FvZL-LrMF9RazfjLm5pkw4FUh3UU0T+4Q@mail.gmail.com","subject":"Re: [PATCH 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T18:33:46Z","receivedAt":"2023-07-21T18:33:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 21, 2023 at 01:41:06PM -0400, Eric Sunshine wrote:\n> > diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\n> > @@ -32,6 +32,13 @@ graph_git_behavior() {\n> > graph_read_expect() {\n> > + DIR=\".\"\n> > + if test \"$1\" = -C\n> > + then\n> > + shift\n> > + DIR=\"$1\"\n> > + shift\n> > + fi\n> > @@ -47,12 +54,15 @@ graph_read_expect() {\n> > - cat >expect <<- EOF\n> > + cat >$DIR/expect <<- EOF\n>\n> It may not matter for any of the current callers, but we'd normally\n> want to quote the expansion of $DIR. Also, as I recall, some versions\n> of bash complain if the target of '>' is not quoted. So:\n>\n> cat >\"$DIR/expect\" <<-EOF\n\nHmm. I'm certainly happy to make this change, but there are many other\nspots within our tests that would need similar updates. Looking through\nthe output of:\n\n $ git grep -E '>\\$[[:alnum:]_]+/.*' -- t/**/*.sh\n\nI see 25 such instances (including this one) that would need similar\ntreatment.\n\nThanks,\nTaylor\n"},{"id":"479764","messageId":"CAPig+cR8S_fEvZfMb=_hHOMy+_Q_MbvRaYcZbp678x7G7E=TQA@mail.gmail.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"Re: [PATCH 0/5] commit-graph: test cleanup and modernization","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-07-21T18:34:18Z","receivedAt":"2023-07-21T18:34:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 21, 2023 at 1:31 PM Taylor Blau wrote:\n> This short series addresses a few style nitpicks that I noticed while\n> looking through the commit-graph tests while writing [1].\n>\n> Most importantly, it removes many instances in t5318 that change\n> directories outside of a sub-shell, altering the current working\n> directory of subsequent tests. This makes it difficult to run a subset\n> of tests, or otherwise include `cd \"$TRASH_DIRECTORY\"` at the top of\n> each test.\n>\n> The first two patches are predatory, the next two are the substantive\n> test clean-ups, and the final patch cleans up some intermediate state\n> necessary to perform the clean-up over multiple commits.\n\nNice to see this being cleaned up. I had tackled this script, as well,\nback in November 2022 as part of an unsubmitted patch series which\nremoves _all_ unprotected `cd` commands from all test scripts. t5318\nwas one of several particularly egregious cases.\n"},{"id":"479765","messageId":"ZLrQyZAoRIaga1mT@nand.local","threadId":"60020","inReplyTo":"CAPig+cTse-bgpApQR3jVGir0Tur22NEgAoTKpmLek+pTRovxwg@mail.gmail.com","subject":"Re: [PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-21T18:39:05Z","receivedAt":"2023-07-21T18:39:13Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 21, 2023 at 02:01:58PM -0400, Eric Sunshine wrote:\n> > diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\n> > @@ -20,12 +20,14 @@ graph_git_behavior() {\n> > test_expect_success \"check normal git operations: $MSG\" '\n> > - cd \"$TRASH_DIRECTORY/$DIR\" &&\n> > - graph_git_two_modes \"log --oneline $BRANCH\" &&\n> > - graph_git_two_modes \"log --topo-order $BRANCH\" &&\n> > - graph_git_two_modes \"log --graph $COMPARE..$BRANCH\" &&\n> > - graph_git_two_modes \"branch -vv\" &&\n> > - graph_git_two_modes \"merge-base -a $BRANCH $COMPARE\"\n> > + (\n> > + cd \"$TRASH_DIRECTORY\" &&\n> > + graph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n> > + graph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n> > + graph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n> > + graph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n> > + graph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n> > + )\n> > '\n> > }\n>\n> As mentioned in my review of patch [1/5], for safety, you'd probably\n> want to quote the expansion of DIR in case it ever contains whitespace\n> (or other weird characters). The obvious POSIX-correct way to do this\n> would be:\n>\n> graph_git_two_modes \"${DIR:+-C \\\"$DIR\\\"} log ...\" &&\n>\n> Unfortunately, however, some older broken shells incorrectly expand\n> this to a single argument (\"-C \") rather than the expected two\n> arguments (-C and \"\")[1,2,3,4]. The workaround is unsightly but\n> doable:\n>\n> graph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log ...\" &&\n\nHmm. I get what you're saying, but I think in this case we're OK, since\nthis all goes to `graph_git_two_modes`, whose implementation looks like:\n\n graph_git_two_modes() {\n git -c core.commitGraph=true $1 >output\n }\n\nSo I think we really do want everything smashed together into a single\nargument.\n\n(Just to sure, here's the diff of what I applied on top of [2/5] before\nreplying to your message):\n\n--- 8< ---\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex c93969ae74..d60e64cb0b 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -22,11 +22,11 @@ graph_git_behavior() {\n \ttest_expect_success \"check normal git operations: $MSG\" '\n \t\t(\n \t\t\tcd \"$TRASH_DIRECTORY\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n+\t\t\tgraph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log --oneline $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log --topo-order $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log --graph $COMPARE..$BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} branch -vv\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} merge-base -a $BRANCH $COMPARE\"\n \t\t)\n \t'\n }\n--- >8 ---\n\n> [1]: https://lore.kernel.org/git/20160517215214.GA16905@sigill.intra.peff.net/\n> [2]: https://lore.kernel.org/git/e3bfc53363b14826d828e1adffbbeea@74d39fa044aa309eaea14b9f57fe79c/\n> [3]: https://lore.kernel.org/git/20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@webmail.informatik.kit.edu/\n> [4]: https://lore.kernel.org/git/1240044459-57227-1-git-send-email-ben@ben.com/\n\nThanks,\nTaylor\n"},{"id":"479766","messageId":"xmqqo7k5umcr.fsf@gitster.g","threadId":"60020","inReplyTo":"ZLrPin9L9LtaDHaC@nand.local","subject":"Re: [PATCH 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T18:54:12Z","receivedAt":"2023-07-21T18:54:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Fri, Jul 21, 2023 at 01:41:06PM -0400, Eric Sunshine wrote:\n>> > diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\n>> > @@ -32,6 +32,13 @@ graph_git_behavior() {\n>> > graph_read_expect() {\n>> > + DIR=\".\"\n>> > + if test \"$1\" = -C\n>> > + then\n>> > + shift\n>> > + DIR=\"$1\"\n>> > + shift\n>> > + fi\n>> > @@ -47,12 +54,15 @@ graph_read_expect() {\n>> > - cat >expect <<- EOF\n>> > + cat >$DIR/expect <<- EOF\n>>\n>> It may not matter for any of the current callers, but we'd normally\n>> want to quote the expansion of $DIR. Also, as I recall, some versions\n>> of bash complain if the target of '>' is not quoted. So:\n>>\n>> cat >\"$DIR/expect\" <<-EOF\n\nCorrect. \n\nDocumentation/CodingGuidelines spells out this shell redirection\nrule and it applies not just to tests but our scripted porcelains\n(if any remains, that is).\n\n> Hmm. I'm certainly happy to make this change, but there are many other\n> spots within our tests that would need similar updates. Looking through\n> the output of:\n>\n> $ git grep -E '>\\$[[:alnum:]_]+/.*' -- t/**/*.sh\n>\n> I see 25 such instances (including this one) that would need similar\n> treatment.\n\nLet's not make things worse.\n\nThanks.\n"},{"id":"479767","messageId":"xmqqfs5hulyq.fsf@gitster.g","threadId":"60020","inReplyTo":"ZLrQyZAoRIaga1mT@nand.local","subject":"Re: [PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T19:02:37Z","receivedAt":"2023-07-21T19:02:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n>> graph_git_two_modes \"${DIR:+-C \\\"$DIR\\\"} log ...\" &&\n>>\n>> Unfortunately, however, some older broken shells incorrectly expand\n>> this to a single argument (\"-C \") rather than the expected two\n>> arguments (-C and \"\")[1,2,3,4]. The workaround is unsightly but\n>> doable:\n>>\n>> graph_git_two_modes \"${DIR:+-C} ${DIR:+\\\"$DIR\\\"} log ...\" &&\n>\n> Hmm. I get what you're saying, but I think in this case we're OK, since\n> this all goes to `graph_git_two_modes`, whose implementation looks like:\n>\n> graph_git_two_modes() {\n> git -c core.commitGraph=true $1 >output\n> }\n>\n> So I think we really do want everything smashed together into a single\n> argument.\n\nThis is not a scripted Porcelain that must cope with any funnies the\nend users may throw at us, after all.\n\nHow about adding a new prominent note at the beginning of\ngraph_git_behavior helper to declare that it is a bug if DIR\ncontains any characters (e.g. $IFS) that may make the shell\nmisbehave, and then stop worrying about quoting of this one?\n\nThat would keep the end result slightly more readable ;-)\n"},{"id":"479772","messageId":"xmqqmszouc4b.fsf@gitster.g","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"Re: [PATCH 0/5] commit-graph: test cleanup and modernization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T22:35:16Z","receivedAt":"2023-07-21T22:35:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Most importantly, it removes many instances in t5318 that change\n> directories outside of a sub-shell, altering the current working\n> directory of subsequent tests. This makes it difficult to run a subset\n> of tests, or otherwise include `cd \"$TRASH_DIRECTORY\"` at the top of\n> each test.\n\nThanks for tackling this aspect of this test script. It does look\nproblematic and unnecessarily makes it harder to debug the test\nitself.\n\n"},{"id":"479812","messageId":"cover.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1689960606.git.me@ttaylorr.com","subject":"[PATCH v2 0/5] commit-graph: test cleanup and modernization","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:19Z","receivedAt":"2023-07-24T16:39:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Here is a reroll of my series to address a few style nitpicks in the\ncommit-graph tests.\n\nMuch is the same from the previous round, except for:\n\n - quotes >\"$DIR/expect\" to ensure that we work on all shells>\n - adds a NOTE above graph_git_behavior to indicate that its second\n argument cannot contain any character in $IFS.\n - rebased on top of the current tip of 'master'.\n\nAs usual, a range-diff is available below. Thanks in advance for your\nreview!\n\nTaylor Blau (5):\n t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories\n t/lib-commit-graph.sh: avoid directory change in\n `graph_git_behavior()`\n t5318: avoid top-level directory changes\n t5328: avoid top-level directory changes\n t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()`\n\n t/lib-commit-graph.sh | 34 ++-\n t/t5318-commit-graph.sh | 378 +++++++++++++----------------\n t/t5328-commit-graph-64bit-time.sh | 54 ++---\n 3 files changed, 224 insertions(+), 242 deletions(-)\n\nRange-diff against v1:\n1: 08482212630 ! 1: c81a059c181 t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories\n @@ t/lib-commit-graph.sh: graph_read_expect() {\n \t\tOPTIONS=\" read_generation_data\"\n \tfi\n -\tcat >expect <<- EOF\n -+\tcat >$DIR/expect <<- EOF\n ++\tcat >\"$DIR/expect\" <<-EOF\n \theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n2: 715a160903b ! 2: 115df6fe226 t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`\n @@ Commit message\n Signed-off-by: Taylor Blau \n \n ## t/lib-commit-graph.sh ##\n -@@ t/lib-commit-graph.sh: graph_git_behavior() {\n +@@ t/lib-commit-graph.sh: graph_git_two_modes() {\n + \ttest_cmp expect output\n + }\n + \n ++# graph_git_behavior \n ++#\n ++# Ensures that a handful of traversal operations produce the same\n ++# results with and without the commit-graph in use.\n ++#\n ++# NOTE: it is a bug to call this function with containing\n ++# any characters in $IFS.\n + graph_git_behavior() {\n + \tMSG=$1\n + \tDIR=$2\n \tBRANCH=$3\n \tCOMPARE=$4\n \ttest_expect_success \"check normal git operations: $MSG\" '\n3: 451ec003be8 = 3: 12ce967bafe t5318: avoid top-level directory changes\n4: ba550987055 = 4: 79b3444660f t5328: avoid top-level directory changes\n5: c3432f27b94 = 5: 887006eab46 t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()`\n-- \n2.41.0.399.g887006eab46\n"},{"id":"479813","messageId":"c81a059c1815a00fc0f5c7576814a830479483af.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1690216758.git.me@ttaylorr.com","subject":"[PATCH v2 1/5] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:22Z","receivedAt":"2023-07-24T16:39:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `graph_read_expect()` function is used to ensure that the output of\nthe \"read-graph\" test helper matches certain parameters (e.g., how many\ncommits are in the graph, which chunks were written, etc.).\n\nIt expects the Git repository being tested to be at the current working\ndirectory. However, a handful of t5318 tests use different repositories\nstored in sub-directories. To work around this, several tests in t5318\nchange into the relevant repository outside of a sub-shell, altering the\ncontext for the rest of the suite.\n\nPrepare to remove these globally-scoped directory changes by teaching\n`graph_read_expect()` to take an optional \"-C dir\" to specify where the\nrepository containing the commit-graph being tested is.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex 5d79e1a4e96..4d3e7f0623e 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -32,6 +32,13 @@ graph_git_behavior() {\n graph_read_expect() {\n \tOPTIONAL=\"\"\n \tNUM_CHUNKS=3\n+\tDIR=\".\"\n+\tif test \"$1\" = -C\n+\tthen\n+\t\tshift\n+\t\tDIR=\"$1\"\n+\t\tshift\n+\tfi\n \tif test -n \"$2\"\n \tthen\n \t\tOPTIONAL=\" $2\"\n@@ -47,12 +54,15 @@ graph_read_expect() {\n \tthen\n \t\tOPTIONS=\" read_generation_data\"\n \tfi\n-\tcat >expect <<- EOF\n+\tcat >\"$DIR/expect\" <<-EOF\n \theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \toptions:$OPTIONS\n \tEOF\n-\ttest-tool read-graph >output &&\n-\ttest_cmp expect output\n+\t(\n+\t\tcd \"$DIR\" &&\n+\t\ttest-tool read-graph >output &&\n+\t\ttest_cmp expect output\n+\t)\n }\n-- \n2.41.0.399.g887006eab46\n\n"},{"id":"479814","messageId":"115df6fe22688bc6305dd5bdbbec83e37cccc1df.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1690216758.git.me@ttaylorr.com","subject":"[PATCH v2 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:25Z","receivedAt":"2023-07-24T16:39:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `graph_git_behavior()` helper asserts that a number of common Git\noperations (such as `git log --oneline`, `git log --topo-order`, etc.)\nproduce identical output regardless of whether or not a commit-graph is\nin use.\n\nThis helper takes as its second argument the location (relative to the\n`$TRASH_DIRECTORY`) of the Git repostiory under test. In order to run\neach of its commands within that repository, it first changes into that\ndirectory, without the use of a sub-shell.\n\nThis pollutes future tests which expect to be run in the top-level\n`$TRASH_DIRECTORY` as usual. We could wrap `graph_git_behavior()` in a\nsub-shell, like:\n\n graph_git_behavior() {\n # ...\n (\n cd \"$TRASH_DIRECTORY/$DIR\" &&\n graph_git_two_modesl\n )\n }\n\n, but since we're invoking git directly, we can pass along a \"-C $DIR\"\nwhen \"$DIR\" is non-empty.\n\nNote, however, that until the remaining callers are cleaned up to avoid\nchanging working directories outside of a sub-shell, that we need to\nensure that we are operating in the top-level $TRASH_DIRECTORY. The\ninner-subshell will go away in a future commit once it is no longer\nnecessary.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 21 +++++++++++++++------\n 1 file changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex 4d3e7f0623e..c8bd76a7777 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -14,18 +14,27 @@ graph_git_two_modes() {\n \ttest_cmp expect output\n }\n \n+# graph_git_behavior \n+#\n+# Ensures that a handful of traversal operations produce the same\n+# results with and without the commit-graph in use.\n+#\n+# NOTE: it is a bug to call this function with containing\n+# any characters in $IFS.\n graph_git_behavior() {\n \tMSG=$1\n \tDIR=$2\n \tBRANCH=$3\n \tCOMPARE=$4\n \ttest_expect_success \"check normal git operations: $MSG\" '\n-\t\tcd \"$TRASH_DIRECTORY/$DIR\" &&\n-\t\tgraph_git_two_modes \"log --oneline $BRANCH\" &&\n-\t\tgraph_git_two_modes \"log --topo-order $BRANCH\" &&\n-\t\tgraph_git_two_modes \"log --graph $COMPARE..$BRANCH\" &&\n-\t\tgraph_git_two_modes \"branch -vv\" &&\n-\t\tgraph_git_two_modes \"merge-base -a $BRANCH $COMPARE\"\n+\t\t(\n+\t\t\tcd \"$TRASH_DIRECTORY\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n+\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n+\t\t)\n \t'\n }\n \n-- \n2.41.0.399.g887006eab46\n\n"},{"id":"479815","messageId":"12ce967bafecf993bb4eb1ed3c17878ae98c9fc4.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1690216758.git.me@ttaylorr.com","subject":"[PATCH v2 3/5] t5318: avoid top-level directory changes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:28Z","receivedAt":"2023-07-24T16:39:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Avoid changing the current working directory from outside of a sub-shell\nduring the tests in t5318.\n\nEach test has mostly straightforward changes, either:\n\n - Removing the top-level `cd \"$TRASH_DIRECTORY/full\"`, which is\n unnecessary after ensuring that other tests don't change their\n working directory outside of a sub-shell.\n\n - Changing any Git invocations which want to be in a sub-directory by\n either (a) adding a \"-C $DIR\" argument, or (b) moving the whole test\n into a sub-shell.\n\nWhile we're here, remove any explicit \"git config core.commitGraph true\"\ninvocations which were designed to enable use of the commit-graph. These\nare unnecessary following 31b1de6a09b (commit-graph: turn on\ncommit-graph by default, 2019-08-13).\n\nSigned-off-by: Taylor Blau \n---\n t/t5318-commit-graph.sh | 378 ++++++++++++++++++----------------------\n 1 file changed, 172 insertions(+), 206 deletions(-)\n\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex bf8a92317b3..4df76173a8d 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -24,12 +24,10 @@ test_expect_success 'usage shown with an error on unknown sub-command' '\n \ttest_cmp expect actual\n '\n \n+objdir=\".git/objects\"\n+\n test_expect_success 'setup full repo' '\n-\tmkdir full &&\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit init &&\n-\tgit config core.commitGraph true &&\n-\tobjdir=\".git/objects\"\n+\tgit init full\n '\n \n test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n@@ -37,31 +35,28 @@ test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n '\n \n test_expect_success 'verify graph with no graph file' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify\n+\tgit -C full commit-graph verify\n '\n \n test_expect_success 'write graph with no packs' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --object-dir $objdir &&\n-\ttest_path_is_missing $objdir/info/commit-graph\n+\tgit -C full commit-graph write --object-dir $objdir &&\n+\ttest_path_is_missing full/$objdir/info/commit-graph\n '\n \n test_expect_success 'exit with correct error on bad input to --stdin-packs' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \techo doesnotexist >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-packs stderr &&\n+\ttest_expect_code 1 git -C full commit-graph write --stdin-packs \\\n+\t\tstderr &&\n \ttest_i18ngrep \"error adding pack\" stderr\n '\n \n test_expect_success 'create commits and repack' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \tfor i in $(test_seq 3)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit repack\n+\tgit -C full repack\n '\n \n . \"$TEST_DIRECTORY\"/lib-commit-graph.sh\n@@ -69,117 +64,106 @@ test_expect_success 'create commits and repack' '\n graph_git_behavior 'no graph' full commits/3 commits/1\n \n test_expect_success 'exit with correct error on bad input to --stdin-commits' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \t# invalid, non-hex OID\n-\techo HEAD >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-commits stderr &&\n+\techo HEAD | test_expect_code 1 git -C full commit-graph write \\\n+\t\t--stdin-commits 2>stderr &&\n \ttest_i18ngrep \"unexpected non-hex object ID: HEAD\" stderr &&\n \t# non-existent OID\n-\techo $ZERO_OID >in &&\n-\ttest_expect_code 1 git commit-graph write --stdin-commits stderr &&\n+\techo $ZERO_OID | test_expect_code 1 git -C full commit-graph write \\\n+\t\t--stdin-commits 2>stderr &&\n \ttest_i18ngrep \"invalid object\" stderr &&\n \t# valid commit and tree OID\n-\tgit rev-parse HEAD HEAD^{tree} >in &&\n-\tgit commit-graph write --stdin-commits in &&\n+\tgit -C full commit-graph write --stdin-commits expect &&\n-\ttest_modebits $objdir/info/commit-graph >actual &&\n+\ttest_modebits full/$objdir/info/commit-graph >actual &&\n \ttest_cmp expect actual\n '\n \n graph_git_behavior 'graph exists' full commits/3 commits/1\n \n test_expect_success 'Add more commits' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit reset --hard commits/1 &&\n+\tgit -C full reset --hard commits/1 &&\n \tfor i in $(test_seq 4 5)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit reset --hard commits/2 &&\n+\tgit -C full reset --hard commits/2 &&\n \tfor i in $(test_seq 6 7)\n \tdo\n-\t\ttest_commit $i &&\n-\t\tgit branch commits/$i || return 1\n+\t\ttest_commit -C full $i &&\n+\t\tgit -C full branch commits/$i || return 1\n \tdone &&\n-\tgit reset --hard commits/2 &&\n-\tgit merge commits/4 &&\n-\tgit branch merge/1 &&\n-\tgit reset --hard commits/4 &&\n-\tgit merge commits/6 &&\n-\tgit branch merge/2 &&\n-\tgit reset --hard commits/3 &&\n-\tgit merge commits/5 commits/7 &&\n-\tgit branch merge/3 &&\n-\tgit repack\n+\tgit -C full reset --hard commits/2 &&\n+\tgit -C full merge commits/4 &&\n+\tgit -C full branch merge/1 &&\n+\tgit -C full reset --hard commits/4 &&\n+\tgit -C full merge commits/6 &&\n+\tgit -C full branch merge/2 &&\n+\tgit -C full reset --hard commits/3 &&\n+\tgit -C full merge commits/5 commits/7 &&\n+\tgit -C full branch merge/3 &&\n+\tgit -C full repack\n '\n \n test_expect_success 'commit-graph write progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write 2>err &&\n+\tgit -C full commit-graph write 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph write --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n test_expect_success 'commit-graph write with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --no-progress 2>err &&\n+\tgit -C full commit-graph write --no-progress 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write --stdin-commits progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tgit commit-graph write --stdin-commits err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tgit -C full commit-graph write --stdin-commits err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph write --stdin-commits force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph write --stdin-commits --progress err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph write --stdin-commits \\\n+\t\t--progress err &&\n \ttest_i18ngrep \"Collecting commits from input\" err\n '\n \n test_expect_success 'commit-graph write --stdin-commits with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/5 >in &&\n-\tgit commit-graph write --stdin-commits --no-progress err &&\n+\tgit -C full rev-parse commits/5 >in &&\n+\tgit -C full commit-graph write --stdin-commits --no-progress err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph verify progress off for redirected stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify 2>err &&\n+\tgit -C full commit-graph verify 2>err &&\n \ttest_must_be_empty err\n '\n \n test_expect_success 'commit-graph verify force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tGIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git -C full commit-graph verify --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n test_expect_success 'commit-graph verify with the --no-progress option' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify --no-progress 2>err &&\n+\tgit -C full commit-graph verify --no-progress 2>err &&\n \ttest_must_be_empty err\n '\n \n@@ -194,10 +178,9 @@ test_expect_success 'commit-graph verify with the --no-progress option' '\n # 1\n \n test_expect_success 'write graph with merges' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"10\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 10 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'merge 1 vs 2' full merge/1 merge/2\n@@ -205,12 +188,11 @@ graph_git_behavior 'merge 1 vs 3' full merge/1 merge/3\n graph_git_behavior 'merge 2 vs 3' full merge/2 merge/3\n \n test_expect_success 'Add one more commit' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_commit 8 &&\n-\tgit branch commits/8 &&\n-\tls $objdir/pack | grep idx >existing-idx &&\n-\tgit repack &&\n-\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n+\ttest_commit -C full 8 &&\n+\tgit -C full branch commits/8 &&\n+\tls full/$objdir/pack | grep idx >existing-idx &&\n+\tgit -C full repack &&\n+\tls full/$objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n '\n \n # Current graph structure:\n@@ -229,114 +211,101 @@ graph_git_behavior 'mixed mode, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'mixed mode, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'write graph with new commit' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"11\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 11 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'full graph, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'full graph, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'write graph with nothing new' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"11\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write &&\n+\ttest_path_is_file full/$objdir/info/commit-graph &&\n+\tgraph_read_expect -C full 11 \"generation_data extra_edges\"\n '\n \n graph_git_behavior 'cleared graph, commit 8 vs merge 1' full commits/8 merge/1\n graph_git_behavior 'cleared graph, commit 8 vs merge 2' full commits/8 merge/2\n \n test_expect_success 'build graph from latest pack with closure' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tcat new-idx | git commit-graph write --stdin-packs &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"9\" \"generation_data extra_edges\"\n+\tgit -C full commit-graph write --stdin-packs commits-in &&\n-\tgit rev-parse merge/1 >>commits-in &&\n-\tcat commits-in | git commit-graph write --stdin-commits &&\n-\ttest_path_is_file $objdir/info/commit-graph &&\n-\tgraph_read_expect \"6\" \"generation_data\"\n+\tgit -C full tag -a -m \"merge\" tag/merge merge/2 &&\n+\tgit -C full rev-parse tag/merge >commits-in &&\n+\tgit -C full rev-parse merge/1 >>commits-in &&\n+\tgit -C full commit-graph write --stdin-commits in &&\n+\tgit -C full commit-graph write --stdin-commits --append output &&\n-\tgit show-ref -s commits/8 >expect &&\n+\tgit -C full checkout -b merge-5-to-8 commits/5 &&\n+\tgit -C full merge commits/8 &&\n+\tgit -C full show-ref -s merge-5-to-8 >output &&\n+\tgit -C full show-ref -s commits/8 >expect &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'check that gc computes commit-graph' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit --allow-empty -m \"blank\" &&\n-\tgit commit-graph write --reachable &&\n-\tcp $objdir/info/commit-graph commit-graph-before-gc &&\n-\tgit reset --hard HEAD~1 &&\n-\tgit config gc.writeCommitGraph true &&\n-\tgit gc &&\n-\tcp $objdir/info/commit-graph commit-graph-after-gc &&\n+\ttest_commit -C full --no-tag blank &&\n+\tgit -C full commit-graph write --reachable &&\n+\tcp full/$objdir/info/commit-graph commit-graph-before-gc &&\n+\tgit -C full reset --hard HEAD~1 &&\n+\ttest_config -C full gc.writeCommitGraph true &&\n+\tgit -C full gc &&\n+\tcp full/$objdir/info/commit-graph commit-graph-after-gc &&\n \t! test_cmp_bin commit-graph-before-gc commit-graph-after-gc &&\n-\tgit commit-graph write --reachable &&\n-\ttest_cmp_bin commit-graph-after-gc $objdir/info/commit-graph\n+\tgit -C full commit-graph write --reachable &&\n+\ttest_cmp_bin commit-graph-after-gc full/$objdir/info/commit-graph\n '\n \n test_expect_success 'replace-objects invalidates commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf replace &&\n \tgit clone full replace &&\n \t(\n@@ -359,7 +328,6 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n '\n \n test_expect_success 'commit grafts invalidate commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf graft &&\n \tgit clone --template= full graft &&\n \t(\n@@ -384,7 +352,6 @@ test_expect_success 'commit grafts invalidate commit-graph' '\n '\n \n test_expect_success 'replace-objects invalidates commit-graph' '\n-\tcd \"$TRASH_DIRECTORY\" &&\n \ttest_when_finished rm -rf shallow &&\n \tgit clone --depth 2 \"file://$TRASH_DIRECTORY/full\" shallow &&\n \t(\n@@ -427,24 +394,25 @@ test_expect_success 'warn on improper hash version' '\n '\n \n test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow chunk' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n \tUNIX_EPOCH_ZERO=\"@0 +0000\" &&\n \tFUTURE_DATE=\"@4147483646 +0000\" &&\n-\trm -f .git/objects/info/commit-graph &&\n-\ttest_commit --date \"$FUTURE_DATE\" future-1 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" old-1 &&\n-\tgit commit-graph write --reachable &&\n-\ttest_commit --date \"$FUTURE_DATE\" future-2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" old-2 &&\n-\tgit commit-graph write --reachable --split=no-merge &&\n-\ttest_commit extra &&\n-\tgit commit-graph write --reachable --split=no-merge &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 16 \"generation_data generation_data_overflow extra_edges\" &&\n-\tmv .git/objects/info/commit-graph commit-graph-upgraded &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 16 \"generation_data generation_data_overflow extra_edges\" &&\n-\ttest_cmp .git/objects/info/commit-graph commit-graph-upgraded\n+\trm -f full/.git/objects/info/commit-graph &&\n+\ttest_commit -C full --date \"$FUTURE_DATE\" future-1 &&\n+\ttest_commit -C full --date \"$UNIX_EPOCH_ZERO\" old-1 &&\n+\tgit -C full commit-graph write --reachable &&\n+\ttest_commit -C full --date \"$FUTURE_DATE\" future-2 &&\n+\ttest_commit -C full --date \"$UNIX_EPOCH_ZERO\" old-2 &&\n+\tgit -C full commit-graph write --reachable --split=no-merge &&\n+\ttest_commit -C full extra &&\n+\tgit -C full commit-graph write --reachable --split=no-merge &&\n+\tgit -C full commit-graph write --reachable &&\n+\tgraph_read_expect -C full 16 \\\n+\t\t\"generation_data generation_data_overflow extra_edges\" &&\n+\tmv full/.git/objects/info/commit-graph commit-graph-upgraded &&\n+\tgit -C full commit-graph write --reachable &&\n+\tgraph_read_expect -C full 16 \\\n+\t\t\"generation_data generation_data_overflow extra_edges\" &&\n+\ttest_cmp full/.git/objects/info/commit-graph commit-graph-upgraded\n '\n \n # the verify tests below expect the commit-graph to contain\n@@ -454,10 +422,11 @@ test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow ch\n # and the tests will likely break.\n \n test_expect_success 'git commit-graph verify' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/8 | git -c commitGraph.generationVersion=1 commit-graph write --stdin-commits &&\n-\tgit commit-graph verify >output &&\n-\tgraph_read_expect 9 extra_edges 1\n+\tgit -C full rev-parse commits/8 >in &&\n+\tgit -C full -c commitGraph.generationVersion=1 commit-graph write \\\n+\t\t--stdin-commits output &&\n+\tgraph_read_expect -C full 9 extra_edges 1\n '\n \n NUM_COMMITS=9\n@@ -495,25 +464,24 @@ GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))\n GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))\n \n corrupt_graph_setup() {\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n-\tcp $objdir/info/commit-graph commit-graph-backup &&\n-\tchmod u+w $objdir/info/commit-graph\n+\ttest_when_finished mv commit-graph-backup full/$objdir/info/commit-graph &&\n+\tcp full/$objdir/info/commit-graph commit-graph-backup &&\n+\tchmod u+w full/$objdir/info/commit-graph\n }\n \n corrupt_graph_verify() {\n \tgrepstr=$1\n-\ttest_must_fail git commit-graph verify 2>test_err &&\n+\ttest_must_fail git -C full commit-graph verify 2>test_err &&\n \tgrep -v \"^+\" test_err >err &&\n \ttest_i18ngrep \"$grepstr\" err &&\n \tif test \"$2\" != \"no-copy\"\n \tthen\n-\t\tcp $objdir/info/commit-graph commit-graph-pre-write-test\n+\t\tcp full/$objdir/info/commit-graph commit-graph-pre-write-test\n \tfi &&\n-\tgit status --short &&\n-\tGIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git commit-graph write &&\n-\tchmod u+w $objdir/info/commit-graph &&\n-\tgit commit-graph verify\n+\tgit -C full status --short &&\n+\tGIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git -C full commit-graph write &&\n+\tchmod u+w full/$objdir/info/commit-graph &&\n+\tgit -C full commit-graph verify\n }\n \n # usage: corrupt_graph_and_verify []\n@@ -527,24 +495,24 @@ corrupt_graph_and_verify() {\n \tdata=\"${2:-\\0}\"\n \tgrepstr=$3\n \tcorrupt_graph_setup &&\n-\torig_size=$(wc -c < $objdir/info/commit-graph) &&\n+\torig_size=$(wc -c >\"$objdir/info/commit-graph\" &&\n+\tprintf \"$data\" | dd of=\"full/$objdir/info/commit-graph\" bs=1 seek=\"$pos\" conv=notrunc &&\n+\tdd of=\"full/$objdir/info/commit-graph\" bs=1 seek=\"$zero_pos\" if=/dev/null &&\n+\ttest-tool genzeros $(($orig_size - $zero_pos)) >>\"full/$objdir/info/commit-graph\" &&\n \tcorrupt_graph_verify \"$grepstr\"\n \n }\n \n test_expect_success POSIXPERM,SANITY 'detect permission problem' '\n \tcorrupt_graph_setup &&\n-\tchmod 000 $objdir/info/commit-graph &&\n+\tchmod 000 full/$objdir/info/commit-graph &&\n \tcorrupt_graph_verify \"Could not open\" \"no-copy\"\n '\n \n test_expect_success 'detect too small' '\n \tcorrupt_graph_setup &&\n-\techo \"a small graph\" >$objdir/info/commit-graph &&\n+\techo \"a small graph\" >full/$objdir/info/commit-graph &&\n \tcorrupt_graph_verify \"too small\"\n '\n \n@@ -655,33 +623,30 @@ test_expect_success 'detect incorrect chunk count' '\n '\n \n test_expect_success 'git fsck (checks commit-graph when config set to true)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\ttest_must_fail git -c core.commitGraph=true fsck\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\ttest_must_fail git -C full -c core.commitGraph=true fsck\n '\n \n test_expect_success 'git fsck (ignores commit-graph when config set to false)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\tgit -c core.commitGraph=false fsck\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\tgit -C full -c core.commitGraph=false fsck\n '\n \n test_expect_success 'git fsck (checks commit-graph when config unset)' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\ttest_when_finished \"git config core.commitGraph true\" &&\n+\ttest_when_finished \"git -C full config core.commitGraph true\" &&\n \n-\tgit fsck &&\n+\tgit -C full fsck &&\n \tcorrupt_graph_and_verify $GRAPH_BYTE_FOOTER \"\\00\" \\\n \t\t\"incorrect checksum\" &&\n-\ttest_unconfig core.commitGraph &&\n-\tcp commit-graph-pre-write-test $objdir/info/commit-graph &&\n-\ttest_must_fail git fsck\n+\ttest_unconfig -C full core.commitGraph &&\n+\tcp commit-graph-pre-write-test full/$objdir/info/commit-graph &&\n+\ttest_must_fail git -C full fsck\n '\n \n test_expect_success 'git fsck shows commit-graph output with --progress' '\n@@ -792,32 +757,33 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '\n #\n \n test_expect_success 'set up and verify repo with generation data overflow chunk' '\n-\tobjdir=\".git/objects\" &&\n \tUNIX_EPOCH_ZERO=\"@0 +0000\" &&\n \tFUTURE_DATE=\"@2147483646 +0000\" &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\tmkdir repo &&\n-\tcd repo &&\n-\tgit init &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n-\ttest_commit 2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 3 generation_data &&\n-\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n-\ttest_commit 5 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n-\tgit branch left &&\n-\tgit reset --hard 3 &&\n-\ttest_commit 7 &&\n-\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n-\ttest_commit 9 &&\n-\tgit branch right &&\n-\tgit reset --hard 3 &&\n-\ttest_merge M left right &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n-\tgit commit-graph verify\n+\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n+\t\ttest_commit 2 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 3 generation_data &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n+\t\ttest_commit 5 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n+\t\tgit branch left &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_commit 7 &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n+\t\ttest_commit 9 &&\n+\t\tgit branch right &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_merge M left right &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n+\t\tgit commit-graph verify\n+\t)\n '\n \n graph_git_behavior 'generation data overflow chunk repo' repo left right\n-- \n2.41.0.399.g887006eab46\n\n"},{"id":"479816","messageId":"887006eab461908493d377c042dd1b7e74641170.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1690216758.git.me@ttaylorr.com","subject":"[PATCH v2 5/5] t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:34Z","receivedAt":"2023-07-24T16:39:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In a previous commit, we introduced a sub-shell in the implementation of\n`graph_git_behavior()`, in order to allow us to pass `-C \"$DIR\"`\ndirectly to the git processes spawned by `graph_git_two_modes()`.\n\nNow that its callers are always operating from the \"$TRASH_DIRECTORY\"\ninstead of one of its sub-directories, we can drop the inner sub-shell,\nas it is no longer required.\n\nSigned-off-by: Taylor Blau \n---\n t/lib-commit-graph.sh | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh\nindex c8bd76a7777..89b26676fbb 100755\n--- a/t/lib-commit-graph.sh\n+++ b/t/lib-commit-graph.sh\n@@ -27,14 +27,11 @@ graph_git_behavior() {\n \tBRANCH=$3\n \tCOMPARE=$4\n \ttest_expect_success \"check normal git operations: $MSG\" '\n-\t\t(\n-\t\t\tcd \"$TRASH_DIRECTORY\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n-\t\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n-\t\t)\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --oneline $BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --topo-order $BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} branch -vv\" &&\n+\t\tgraph_git_two_modes \"${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE\"\n \t'\n }\n \n-- \n2.41.0.399.g887006eab46\n"},{"id":"479817","messageId":"79b3444660fbc5f75a9dde023677b0858717b602.1690216758.git.me@ttaylorr.com","threadId":"60020","inReplyTo":"cover.1690216758.git.me@ttaylorr.com","subject":"[PATCH v2 4/5] t5328: avoid top-level directory changes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-07-24T16:39:31Z","receivedAt":"2023-07-24T16:40:00Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In a similar spirit as the last commit, avoid top-level directory\nchanges in the last remaining commit-graph related test, t5328.\n\nSigned-off-by: Taylor Blau \n---\n t/t5328-commit-graph-64bit-time.sh | 54 +++++++++++++++---------------\n 1 file changed, 27 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh\nindex 57e4d9c6998..e9c521c061c 100755\n--- a/t/t5328-commit-graph-64bit-time.sh\n+++ b/t/t5328-commit-graph-64bit-time.sh\n@@ -37,39 +37,39 @@ test_expect_success 'lower layers have overflow chunk' '\n graph_git_behavior 'overflow' '' HEAD~2 HEAD\n \n test_expect_success 'set up and verify repo with generation data overflow chunk' '\n-\tmkdir repo &&\n-\tcd repo &&\n-\tgit init &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n-\ttest_commit 2 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 3 generation_data &&\n-\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n-\ttest_commit 5 &&\n-\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n-\tgit branch left &&\n-\tgit reset --hard 3 &&\n-\ttest_commit 7 &&\n-\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n-\ttest_commit 9 &&\n-\tgit branch right &&\n-\tgit reset --hard 3 &&\n-\ttest_merge M left right &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n-\tgit commit-graph verify\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 1 &&\n+\t\ttest_commit 2 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 3 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 3 generation_data &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 4 &&\n+\t\ttest_commit 5 &&\n+\t\ttest_commit --date \"$UNIX_EPOCH_ZERO\" 6 &&\n+\t\tgit branch left &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_commit 7 &&\n+\t\ttest_commit --date \"$FUTURE_DATE\" 8 &&\n+\t\ttest_commit 9 &&\n+\t\tgit branch right &&\n+\t\tgit reset --hard 3 &&\n+\t\ttest_merge M left right &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tgraph_read_expect 10 \"generation_data generation_data_overflow\" &&\n+\t\tgit commit-graph verify\n+\t)\n '\n \n graph_git_behavior 'overflow 2' repo left right\n \n test_expect_success 'single commit with generation data exceeding UINT32_MAX' '\n \tgit init repo-uint32-max &&\n-\tcd repo-uint32-max &&\n-\ttest_commit --date \"@4294967297 +0000\" 1 &&\n-\tgit commit-graph write --reachable &&\n-\tgraph_read_expect 1 \"generation_data\" &&\n-\tgit commit-graph verify\n+\ttest_commit -C repo-uint32-max --date \"@4294967297 +0000\" 1 &&\n+\tgit -C repo-uint32-max commit-graph write --reachable &&\n+\tgraph_read_expect -C repo-uint32-max 1 \"generation_data\" &&\n+\tgit -C repo-uint32-max commit-graph verify\n '\n \n test_done\n-- \n2.41.0.399.g887006eab46\n\n"},{"id":"479833","messageId":"xmqq7cqpnfpr.fsf@gitster.g","threadId":"60020","inReplyTo":"12ce967bafecf993bb4eb1ed3c17878ae98c9fc4.1690216758.git.me@ttaylorr.com","subject":"Re: [PATCH v2 3/5] t5318: avoid top-level directory changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-24T21:48:32Z","receivedAt":"2023-07-24T21:48:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> test_expect_success 'exit with correct error on bad input to --stdin-packs' '\n> -\tcd \"$TRASH_DIRECTORY/full\" &&\n> \techo doesnotexist >in &&\n> -\ttest_expect_code 1 git commit-graph write --stdin-packs stderr &&\n> +\ttest_expect_code 1 git -C full commit-graph write --stdin-packs \\\n> +\t\tstderr &&\n> \ttest_i18ngrep \"error adding pack\" stderr\n> '\n\nStrictly speaking, this changes behaviour in that \"in\" and \"stderr\"\nfiles are now left in the top-level directory (they used to be in\nthe \"full\" directory). It is rather hard to make sure that this\ndifference (and similar ones the patch makes) does not matter in the\nrest of the tests only from the patch context, but I'll read through\n\"git diff -U999\" and did not spot anything questionable.\n"}]}