From c355b641769db757bc4187aa03f18510a289c86d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 24 Jul 2023 12:39:22 -0400 Subject: [PATCH 01/34] t/lib-commit-graph.sh: allow `graph_read_expect()` in sub-directories The `graph_read_expect()` function is used to ensure that the output of the "read-graph" test helper matches certain parameters (e.g., how many commits are in the graph, which chunks were written, etc.). It expects the Git repository being tested to be at the current working directory. However, a handful of t5318 tests use different repositories stored in sub-directories. To work around this, several tests in t5318 change into the relevant repository outside of a sub-shell, altering the context for the rest of the suite. Prepare to remove these globally-scoped directory changes by teaching `graph_read_expect()` to take an optional "-C dir" to specify where the repository containing the commit-graph being tested is. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/lib-commit-graph.sh | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh index 5d79e1a4e96761..4d3e7f0623e503 100755 --- a/t/lib-commit-graph.sh +++ b/t/lib-commit-graph.sh @@ -32,6 +32,13 @@ graph_git_behavior() { graph_read_expect() { OPTIONAL="" NUM_CHUNKS=3 + DIR="." + if test "$1" = -C + then + shift + DIR="$1" + shift + fi if test -n "$2" then OPTIONAL=" $2" @@ -47,12 +54,15 @@ graph_read_expect() { then OPTIONS=" read_generation_data" fi - cat >expect <<- EOF + cat >"$DIR/expect" <<-EOF header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0 num_commits: $1 chunks: oid_fanout oid_lookup commit_metadata$OPTIONAL options:$OPTIONS EOF - test-tool read-graph >output && - test_cmp expect output + ( + cd "$DIR" && + test-tool read-graph >output && + test_cmp expect output + ) } From a953d2b628952f8d225d337deb1c30e20f835689 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 24 Jul 2023 12:39:25 -0400 Subject: [PATCH 02/34] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()` The `graph_git_behavior()` helper asserts that a number of common Git operations (such as `git log --oneline`, `git log --topo-order`, etc.) produce identical output regardless of whether or not a commit-graph is in use. This helper takes as its second argument the location (relative to the `$TRASH_DIRECTORY`) of the Git repostiory under test. In order to run each of its commands within that repository, it first changes into that directory, without the use of a sub-shell. This pollutes future tests which expect to be run in the top-level `$TRASH_DIRECTORY` as usual. We could wrap `graph_git_behavior()` in a sub-shell, like: graph_git_behavior() { # ... ( cd "$TRASH_DIRECTORY/$DIR" && graph_git_two_modesl ) } , but since we're invoking git directly, we can pass along a "-C $DIR" when "$DIR" is non-empty. Note, however, that until the remaining callers are cleaned up to avoid changing working directories outside of a sub-shell, that we need to ensure that we are operating in the top-level $TRASH_DIRECTORY. The inner-subshell will go away in a future commit once it is no longer necessary. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/lib-commit-graph.sh | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh index 4d3e7f0623e503..c8bd76a7777607 100755 --- a/t/lib-commit-graph.sh +++ b/t/lib-commit-graph.sh @@ -14,18 +14,27 @@ graph_git_two_modes() { test_cmp expect output } +# graph_git_behavior +# +# Ensures that a handful of traversal operations produce the same +# results with and without the commit-graph in use. +# +# NOTE: it is a bug to call this function with containing +# any characters in $IFS. graph_git_behavior() { MSG=$1 DIR=$2 BRANCH=$3 COMPARE=$4 test_expect_success "check normal git operations: $MSG" ' - cd "$TRASH_DIRECTORY/$DIR" && - graph_git_two_modes "log --oneline $BRANCH" && - graph_git_two_modes "log --topo-order $BRANCH" && - graph_git_two_modes "log --graph $COMPARE..$BRANCH" && - graph_git_two_modes "branch -vv" && - graph_git_two_modes "merge-base -a $BRANCH $COMPARE" + ( + cd "$TRASH_DIRECTORY" && + graph_git_two_modes "${DIR:+-C $DIR} log --oneline $BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} log --topo-order $BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} branch -vv" && + graph_git_two_modes "${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE" + ) ' } From 51550d03e438bfcb83bec7665baec2378f931a80 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 24 Jul 2023 12:39:28 -0400 Subject: [PATCH 03/34] t5318: avoid top-level directory changes Avoid changing the current working directory from outside of a sub-shell during the tests in t5318. Each test has mostly straightforward changes, either: - Removing the top-level `cd "$TRASH_DIRECTORY/full"`, which is unnecessary after ensuring that other tests don't change their working directory outside of a sub-shell. - Changing any Git invocations which want to be in a sub-directory by either (a) adding a "-C $DIR" argument, or (b) moving the whole test into a sub-shell. While we're here, remove any explicit "git config core.commitGraph true" invocations which were designed to enable use of the commit-graph. These are unnecessary following 31b1de6a09b (commit-graph: turn on commit-graph by default, 2019-08-13). Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/t5318-commit-graph.sh | 378 ++++++++++++++++++---------------------- 1 file changed, 172 insertions(+), 206 deletions(-) diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index bf8a92317b3aa9..4df76173a8d774 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -24,12 +24,10 @@ test_expect_success 'usage shown with an error on unknown sub-command' ' test_cmp expect actual ' +objdir=".git/objects" + test_expect_success 'setup full repo' ' - mkdir full && - cd "$TRASH_DIRECTORY/full" && - git init && - git config core.commitGraph true && - objdir=".git/objects" + git init full ' test_expect_success POSIXPERM 'tweak umask for modebit tests' ' @@ -37,31 +35,28 @@ test_expect_success POSIXPERM 'tweak umask for modebit tests' ' ' test_expect_success 'verify graph with no graph file' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph verify + git -C full commit-graph verify ' test_expect_success 'write graph with no packs' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write --object-dir $objdir && - test_path_is_missing $objdir/info/commit-graph + git -C full commit-graph write --object-dir $objdir && + test_path_is_missing full/$objdir/info/commit-graph ' test_expect_success 'exit with correct error on bad input to --stdin-packs' ' - cd "$TRASH_DIRECTORY/full" && echo doesnotexist >in && - test_expect_code 1 git commit-graph write --stdin-packs stderr && + test_expect_code 1 git -C full commit-graph write --stdin-packs \ + stderr && test_i18ngrep "error adding pack" stderr ' test_expect_success 'create commits and repack' ' - cd "$TRASH_DIRECTORY/full" && for i in $(test_seq 3) do - test_commit $i && - git branch commits/$i || return 1 + test_commit -C full $i && + git -C full branch commits/$i || return 1 done && - git repack + git -C full repack ' . "$TEST_DIRECTORY"/lib-commit-graph.sh @@ -69,117 +64,106 @@ test_expect_success 'create commits and repack' ' graph_git_behavior 'no graph' full commits/3 commits/1 test_expect_success 'exit with correct error on bad input to --stdin-commits' ' - cd "$TRASH_DIRECTORY/full" && # invalid, non-hex OID - echo HEAD >in && - test_expect_code 1 git commit-graph write --stdin-commits stderr && + echo HEAD | test_expect_code 1 git -C full commit-graph write \ + --stdin-commits 2>stderr && test_i18ngrep "unexpected non-hex object ID: HEAD" stderr && # non-existent OID - echo $ZERO_OID >in && - test_expect_code 1 git commit-graph write --stdin-commits stderr && + echo $ZERO_OID | test_expect_code 1 git -C full commit-graph write \ + --stdin-commits 2>stderr && test_i18ngrep "invalid object" stderr && # valid commit and tree OID - git rev-parse HEAD HEAD^{tree} >in && - git commit-graph write --stdin-commits in && + git -C full commit-graph write --stdin-commits expect && - test_modebits $objdir/info/commit-graph >actual && + test_modebits full/$objdir/info/commit-graph >actual && test_cmp expect actual ' graph_git_behavior 'graph exists' full commits/3 commits/1 test_expect_success 'Add more commits' ' - cd "$TRASH_DIRECTORY/full" && - git reset --hard commits/1 && + git -C full reset --hard commits/1 && for i in $(test_seq 4 5) do - test_commit $i && - git branch commits/$i || return 1 + test_commit -C full $i && + git -C full branch commits/$i || return 1 done && - git reset --hard commits/2 && + git -C full reset --hard commits/2 && for i in $(test_seq 6 7) do - test_commit $i && - git branch commits/$i || return 1 + test_commit -C full $i && + git -C full branch commits/$i || return 1 done && - git reset --hard commits/2 && - git merge commits/4 && - git branch merge/1 && - git reset --hard commits/4 && - git merge commits/6 && - git branch merge/2 && - git reset --hard commits/3 && - git merge commits/5 commits/7 && - git branch merge/3 && - git repack + git -C full reset --hard commits/2 && + git -C full merge commits/4 && + git -C full branch merge/1 && + git -C full reset --hard commits/4 && + git -C full merge commits/6 && + git -C full branch merge/2 && + git -C full reset --hard commits/3 && + git -C full merge commits/5 commits/7 && + git -C full branch merge/3 && + git -C full repack ' test_expect_success 'commit-graph write progress off for redirected stderr' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write 2>err && + git -C full commit-graph write 2>err && test_must_be_empty err ' test_expect_success 'commit-graph write force progress on for stderr' ' - cd "$TRASH_DIRECTORY/full" && - GIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err && + GIT_PROGRESS_DELAY=0 git -C full commit-graph write --progress 2>err && test_file_not_empty err ' test_expect_success 'commit-graph write with the --no-progress option' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write --no-progress 2>err && + git -C full commit-graph write --no-progress 2>err && test_must_be_empty err ' test_expect_success 'commit-graph write --stdin-commits progress off for redirected stderr' ' - cd "$TRASH_DIRECTORY/full" && - git rev-parse commits/5 >in && - git commit-graph write --stdin-commits err && + git -C full rev-parse commits/5 >in && + git -C full commit-graph write --stdin-commits err && test_must_be_empty err ' test_expect_success 'commit-graph write --stdin-commits force progress on for stderr' ' - cd "$TRASH_DIRECTORY/full" && - git rev-parse commits/5 >in && - GIT_PROGRESS_DELAY=0 git commit-graph write --stdin-commits --progress err && + git -C full rev-parse commits/5 >in && + GIT_PROGRESS_DELAY=0 git -C full commit-graph write --stdin-commits \ + --progress err && test_i18ngrep "Collecting commits from input" err ' test_expect_success 'commit-graph write --stdin-commits with the --no-progress option' ' - cd "$TRASH_DIRECTORY/full" && - git rev-parse commits/5 >in && - git commit-graph write --stdin-commits --no-progress err && + git -C full rev-parse commits/5 >in && + git -C full commit-graph write --stdin-commits --no-progress err && test_must_be_empty err ' test_expect_success 'commit-graph verify progress off for redirected stderr' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph verify 2>err && + git -C full commit-graph verify 2>err && test_must_be_empty err ' test_expect_success 'commit-graph verify force progress on for stderr' ' - cd "$TRASH_DIRECTORY/full" && - GIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err && + GIT_PROGRESS_DELAY=0 git -C full commit-graph verify --progress 2>err && test_file_not_empty err ' test_expect_success 'commit-graph verify with the --no-progress option' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph verify --no-progress 2>err && + git -C full commit-graph verify --no-progress 2>err && test_must_be_empty err ' @@ -194,10 +178,9 @@ test_expect_success 'commit-graph verify with the --no-progress option' ' # 1 test_expect_success 'write graph with merges' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write && - test_path_is_file $objdir/info/commit-graph && - graph_read_expect "10" "generation_data extra_edges" + git -C full commit-graph write && + test_path_is_file full/$objdir/info/commit-graph && + graph_read_expect -C full 10 "generation_data extra_edges" ' graph_git_behavior 'merge 1 vs 2' full merge/1 merge/2 @@ -205,12 +188,11 @@ graph_git_behavior 'merge 1 vs 3' full merge/1 merge/3 graph_git_behavior 'merge 2 vs 3' full merge/2 merge/3 test_expect_success 'Add one more commit' ' - cd "$TRASH_DIRECTORY/full" && - test_commit 8 && - git branch commits/8 && - ls $objdir/pack | grep idx >existing-idx && - git repack && - ls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx + test_commit -C full 8 && + git -C full branch commits/8 && + ls full/$objdir/pack | grep idx >existing-idx && + git -C full repack && + ls full/$objdir/pack| grep idx | grep -v -f existing-idx >new-idx ' # Current graph structure: @@ -229,114 +211,101 @@ graph_git_behavior 'mixed mode, commit 8 vs merge 1' full commits/8 merge/1 graph_git_behavior 'mixed mode, commit 8 vs merge 2' full commits/8 merge/2 test_expect_success 'write graph with new commit' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write && - test_path_is_file $objdir/info/commit-graph && - graph_read_expect "11" "generation_data extra_edges" + git -C full commit-graph write && + test_path_is_file full/$objdir/info/commit-graph && + graph_read_expect -C full 11 "generation_data extra_edges" ' graph_git_behavior 'full graph, commit 8 vs merge 1' full commits/8 merge/1 graph_git_behavior 'full graph, commit 8 vs merge 2' full commits/8 merge/2 test_expect_success 'write graph with nothing new' ' - cd "$TRASH_DIRECTORY/full" && - git commit-graph write && - test_path_is_file $objdir/info/commit-graph && - graph_read_expect "11" "generation_data extra_edges" + git -C full commit-graph write && + test_path_is_file full/$objdir/info/commit-graph && + graph_read_expect -C full 11 "generation_data extra_edges" ' graph_git_behavior 'cleared graph, commit 8 vs merge 1' full commits/8 merge/1 graph_git_behavior 'cleared graph, commit 8 vs merge 2' full commits/8 merge/2 test_expect_success 'build graph from latest pack with closure' ' - cd "$TRASH_DIRECTORY/full" && - cat new-idx | git commit-graph write --stdin-packs && - test_path_is_file $objdir/info/commit-graph && - graph_read_expect "9" "generation_data extra_edges" + git -C full commit-graph write --stdin-packs commits-in && - git rev-parse merge/1 >>commits-in && - cat commits-in | git commit-graph write --stdin-commits && - test_path_is_file $objdir/info/commit-graph && - graph_read_expect "6" "generation_data" + git -C full tag -a -m "merge" tag/merge merge/2 && + git -C full rev-parse tag/merge >commits-in && + git -C full rev-parse merge/1 >>commits-in && + git -C full commit-graph write --stdin-commits in && + git -C full commit-graph write --stdin-commits --append output && - git show-ref -s commits/8 >expect && + git -C full checkout -b merge-5-to-8 commits/5 && + git -C full merge commits/8 && + git -C full show-ref -s merge-5-to-8 >output && + git -C full show-ref -s commits/8 >expect && test_cmp expect output ' test_expect_success 'check that gc computes commit-graph' ' - cd "$TRASH_DIRECTORY/full" && - git commit --allow-empty -m "blank" && - git commit-graph write --reachable && - cp $objdir/info/commit-graph commit-graph-before-gc && - git reset --hard HEAD~1 && - git config gc.writeCommitGraph true && - git gc && - cp $objdir/info/commit-graph commit-graph-after-gc && + test_commit -C full --no-tag blank && + git -C full commit-graph write --reachable && + cp full/$objdir/info/commit-graph commit-graph-before-gc && + git -C full reset --hard HEAD~1 && + test_config -C full gc.writeCommitGraph true && + git -C full gc && + cp full/$objdir/info/commit-graph commit-graph-after-gc && ! test_cmp_bin commit-graph-before-gc commit-graph-after-gc && - git commit-graph write --reachable && - test_cmp_bin commit-graph-after-gc $objdir/info/commit-graph + git -C full commit-graph write --reachable && + test_cmp_bin commit-graph-after-gc full/$objdir/info/commit-graph ' test_expect_success 'replace-objects invalidates commit-graph' ' - cd "$TRASH_DIRECTORY" && test_when_finished rm -rf replace && git clone full replace && ( @@ -359,7 +328,6 @@ test_expect_success 'replace-objects invalidates commit-graph' ' ' test_expect_success 'commit grafts invalidate commit-graph' ' - cd "$TRASH_DIRECTORY" && test_when_finished rm -rf graft && git clone --template= full graft && ( @@ -384,7 +352,6 @@ test_expect_success 'commit grafts invalidate commit-graph' ' ' test_expect_success 'replace-objects invalidates commit-graph' ' - cd "$TRASH_DIRECTORY" && test_when_finished rm -rf shallow && git clone --depth 2 "file://$TRASH_DIRECTORY/full" shallow && ( @@ -427,24 +394,25 @@ test_expect_success 'warn on improper hash version' ' ' test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow chunk' ' - cd "$TRASH_DIRECTORY/full" && UNIX_EPOCH_ZERO="@0 +0000" && FUTURE_DATE="@4147483646 +0000" && - rm -f .git/objects/info/commit-graph && - test_commit --date "$FUTURE_DATE" future-1 && - test_commit --date "$UNIX_EPOCH_ZERO" old-1 && - git commit-graph write --reachable && - test_commit --date "$FUTURE_DATE" future-2 && - test_commit --date "$UNIX_EPOCH_ZERO" old-2 && - git commit-graph write --reachable --split=no-merge && - test_commit extra && - git commit-graph write --reachable --split=no-merge && - git commit-graph write --reachable && - graph_read_expect 16 "generation_data generation_data_overflow extra_edges" && - mv .git/objects/info/commit-graph commit-graph-upgraded && - git commit-graph write --reachable && - graph_read_expect 16 "generation_data generation_data_overflow extra_edges" && - test_cmp .git/objects/info/commit-graph commit-graph-upgraded + rm -f full/.git/objects/info/commit-graph && + test_commit -C full --date "$FUTURE_DATE" future-1 && + test_commit -C full --date "$UNIX_EPOCH_ZERO" old-1 && + git -C full commit-graph write --reachable && + test_commit -C full --date "$FUTURE_DATE" future-2 && + test_commit -C full --date "$UNIX_EPOCH_ZERO" old-2 && + git -C full commit-graph write --reachable --split=no-merge && + test_commit -C full extra && + git -C full commit-graph write --reachable --split=no-merge && + git -C full commit-graph write --reachable && + graph_read_expect -C full 16 \ + "generation_data generation_data_overflow extra_edges" && + mv full/.git/objects/info/commit-graph commit-graph-upgraded && + git -C full commit-graph write --reachable && + graph_read_expect -C full 16 \ + "generation_data generation_data_overflow extra_edges" && + test_cmp full/.git/objects/info/commit-graph commit-graph-upgraded ' # the verify tests below expect the commit-graph to contain @@ -454,10 +422,11 @@ test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'lower layers have overflow ch # and the tests will likely break. test_expect_success 'git commit-graph verify' ' - cd "$TRASH_DIRECTORY/full" && - git rev-parse commits/8 | git -c commitGraph.generationVersion=1 commit-graph write --stdin-commits && - git commit-graph verify >output && - graph_read_expect 9 extra_edges 1 + git -C full rev-parse commits/8 >in && + git -C full -c commitGraph.generationVersion=1 commit-graph write \ + --stdin-commits output && + graph_read_expect -C full 9 extra_edges 1 ' NUM_COMMITS=9 @@ -495,25 +464,24 @@ GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4)) GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES)) corrupt_graph_setup() { - cd "$TRASH_DIRECTORY/full" && - test_when_finished mv commit-graph-backup $objdir/info/commit-graph && - cp $objdir/info/commit-graph commit-graph-backup && - chmod u+w $objdir/info/commit-graph + test_when_finished mv commit-graph-backup full/$objdir/info/commit-graph && + cp full/$objdir/info/commit-graph commit-graph-backup && + chmod u+w full/$objdir/info/commit-graph } corrupt_graph_verify() { grepstr=$1 - test_must_fail git commit-graph verify 2>test_err && + test_must_fail git -C full commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "$grepstr" err && if test "$2" != "no-copy" then - cp $objdir/info/commit-graph commit-graph-pre-write-test + cp full/$objdir/info/commit-graph commit-graph-pre-write-test fi && - git status --short && - GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git commit-graph write && - chmod u+w $objdir/info/commit-graph && - git commit-graph verify + git -C full status --short && + GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE=true git -C full commit-graph write && + chmod u+w full/$objdir/info/commit-graph && + git -C full commit-graph verify } # usage: corrupt_graph_and_verify [] @@ -527,24 +495,24 @@ corrupt_graph_and_verify() { data="${2:-\0}" grepstr=$3 corrupt_graph_setup && - orig_size=$(wc -c < $objdir/info/commit-graph) && + orig_size=$(wc -c >"$objdir/info/commit-graph" && + printf "$data" | dd of="full/$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc && + dd of="full/$objdir/info/commit-graph" bs=1 seek="$zero_pos" if=/dev/null && + test-tool genzeros $(($orig_size - $zero_pos)) >>"full/$objdir/info/commit-graph" && corrupt_graph_verify "$grepstr" } test_expect_success POSIXPERM,SANITY 'detect permission problem' ' corrupt_graph_setup && - chmod 000 $objdir/info/commit-graph && + chmod 000 full/$objdir/info/commit-graph && corrupt_graph_verify "Could not open" "no-copy" ' test_expect_success 'detect too small' ' corrupt_graph_setup && - echo "a small graph" >$objdir/info/commit-graph && + echo "a small graph" >full/$objdir/info/commit-graph && corrupt_graph_verify "too small" ' @@ -655,33 +623,30 @@ test_expect_success 'detect incorrect chunk count' ' ' test_expect_success 'git fsck (checks commit-graph when config set to true)' ' - cd "$TRASH_DIRECTORY/full" && - git fsck && + git -C full fsck && corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ "incorrect checksum" && - cp commit-graph-pre-write-test $objdir/info/commit-graph && - test_must_fail git -c core.commitGraph=true fsck + cp commit-graph-pre-write-test full/$objdir/info/commit-graph && + test_must_fail git -C full -c core.commitGraph=true fsck ' test_expect_success 'git fsck (ignores commit-graph when config set to false)' ' - cd "$TRASH_DIRECTORY/full" && - git fsck && + git -C full fsck && corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ "incorrect checksum" && - cp commit-graph-pre-write-test $objdir/info/commit-graph && - git -c core.commitGraph=false fsck + cp commit-graph-pre-write-test full/$objdir/info/commit-graph && + git -C full -c core.commitGraph=false fsck ' test_expect_success 'git fsck (checks commit-graph when config unset)' ' - cd "$TRASH_DIRECTORY/full" && - test_when_finished "git config core.commitGraph true" && + test_when_finished "git -C full config core.commitGraph true" && - git fsck && + git -C full fsck && corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ "incorrect checksum" && - test_unconfig core.commitGraph && - cp commit-graph-pre-write-test $objdir/info/commit-graph && - test_must_fail git fsck + test_unconfig -C full core.commitGraph && + cp commit-graph-pre-write-test full/$objdir/info/commit-graph && + test_must_fail git -C full fsck ' test_expect_success 'git fsck shows commit-graph output with --progress' ' @@ -792,32 +757,33 @@ test_expect_success 'corrupt commit-graph write (missing tree)' ' # test_expect_success 'set up and verify repo with generation data overflow chunk' ' - objdir=".git/objects" && UNIX_EPOCH_ZERO="@0 +0000" && FUTURE_DATE="@2147483646 +0000" && - cd "$TRASH_DIRECTORY" && - mkdir repo && - cd repo && - git init && - test_commit --date "$UNIX_EPOCH_ZERO" 1 && - test_commit 2 && - test_commit --date "$UNIX_EPOCH_ZERO" 3 && - git commit-graph write --reachable && - graph_read_expect 3 generation_data && - test_commit --date "$FUTURE_DATE" 4 && - test_commit 5 && - test_commit --date "$UNIX_EPOCH_ZERO" 6 && - git branch left && - git reset --hard 3 && - test_commit 7 && - test_commit --date "$FUTURE_DATE" 8 && - test_commit 9 && - git branch right && - git reset --hard 3 && - test_merge M left right && - git commit-graph write --reachable && - graph_read_expect 10 "generation_data generation_data_overflow" && - git commit-graph verify + + git init repo && + ( + cd repo && + + test_commit --date "$UNIX_EPOCH_ZERO" 1 && + test_commit 2 && + test_commit --date "$UNIX_EPOCH_ZERO" 3 && + git commit-graph write --reachable && + graph_read_expect 3 generation_data && + test_commit --date "$FUTURE_DATE" 4 && + test_commit 5 && + test_commit --date "$UNIX_EPOCH_ZERO" 6 && + git branch left && + git reset --hard 3 && + test_commit 7 && + test_commit --date "$FUTURE_DATE" 8 && + test_commit 9 && + git branch right && + git reset --hard 3 && + test_merge M left right && + git commit-graph write --reachable && + graph_read_expect 10 "generation_data generation_data_overflow" && + git commit-graph verify + ) ' graph_git_behavior 'generation data overflow chunk repo' repo left right From 749f126b296f1147c7994de41c93a0afb9a250dd Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 24 Jul 2023 12:39:31 -0400 Subject: [PATCH 04/34] t5328: avoid top-level directory changes In a similar spirit as the last commit, avoid top-level directory changes in the last remaining commit-graph related test, t5328. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/t5328-commit-graph-64bit-time.sh | 54 +++++++++++++++--------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh index 57e4d9c6998c2b..e9c521c061c3ea 100755 --- a/t/t5328-commit-graph-64bit-time.sh +++ b/t/t5328-commit-graph-64bit-time.sh @@ -37,39 +37,39 @@ test_expect_success 'lower layers have overflow chunk' ' graph_git_behavior 'overflow' '' HEAD~2 HEAD test_expect_success 'set up and verify repo with generation data overflow chunk' ' - mkdir repo && - cd repo && - git init && - test_commit --date "$UNIX_EPOCH_ZERO" 1 && - test_commit 2 && - test_commit --date "$UNIX_EPOCH_ZERO" 3 && - git commit-graph write --reachable && - graph_read_expect 3 generation_data && - test_commit --date "$FUTURE_DATE" 4 && - test_commit 5 && - test_commit --date "$UNIX_EPOCH_ZERO" 6 && - git branch left && - git reset --hard 3 && - test_commit 7 && - test_commit --date "$FUTURE_DATE" 8 && - test_commit 9 && - git branch right && - git reset --hard 3 && - test_merge M left right && - git commit-graph write --reachable && - graph_read_expect 10 "generation_data generation_data_overflow" && - git commit-graph verify + git init repo && + ( + cd repo && + test_commit --date "$UNIX_EPOCH_ZERO" 1 && + test_commit 2 && + test_commit --date "$UNIX_EPOCH_ZERO" 3 && + git commit-graph write --reachable && + graph_read_expect 3 generation_data && + test_commit --date "$FUTURE_DATE" 4 && + test_commit 5 && + test_commit --date "$UNIX_EPOCH_ZERO" 6 && + git branch left && + git reset --hard 3 && + test_commit 7 && + test_commit --date "$FUTURE_DATE" 8 && + test_commit 9 && + git branch right && + git reset --hard 3 && + test_merge M left right && + git commit-graph write --reachable && + graph_read_expect 10 "generation_data generation_data_overflow" && + git commit-graph verify + ) ' graph_git_behavior 'overflow 2' repo left right test_expect_success 'single commit with generation data exceeding UINT32_MAX' ' git init repo-uint32-max && - cd repo-uint32-max && - test_commit --date "@4294967297 +0000" 1 && - git commit-graph write --reachable && - graph_read_expect 1 "generation_data" && - git commit-graph verify + test_commit -C repo-uint32-max --date "@4294967297 +0000" 1 && + git -C repo-uint32-max commit-graph write --reachable && + graph_read_expect -C repo-uint32-max 1 "generation_data" && + git -C repo-uint32-max commit-graph verify ' test_done From f1b9cebc8bdc01c5b8643fac9f78f2962df82cf3 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 24 Jul 2023 12:39:34 -0400 Subject: [PATCH 05/34] t/lib-commit-graph.sh: avoid sub-shell in `graph_git_behavior()` In a previous commit, we introduced a sub-shell in the implementation of `graph_git_behavior()`, in order to allow us to pass `-C "$DIR"` directly to the git processes spawned by `graph_git_two_modes()`. Now that its callers are always operating from the "$TRASH_DIRECTORY" instead of one of its sub-directories, we can drop the inner sub-shell, as it is no longer required. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/lib-commit-graph.sh | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh index c8bd76a7777607..89b26676fbb94b 100755 --- a/t/lib-commit-graph.sh +++ b/t/lib-commit-graph.sh @@ -27,14 +27,11 @@ graph_git_behavior() { BRANCH=$3 COMPARE=$4 test_expect_success "check normal git operations: $MSG" ' - ( - cd "$TRASH_DIRECTORY" && - graph_git_two_modes "${DIR:+-C $DIR} log --oneline $BRANCH" && - graph_git_two_modes "${DIR:+-C $DIR} log --topo-order $BRANCH" && - graph_git_two_modes "${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH" && - graph_git_two_modes "${DIR:+-C $DIR} branch -vv" && - graph_git_two_modes "${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE" - ) + graph_git_two_modes "${DIR:+-C $DIR} log --oneline $BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} log --topo-order $BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH" && + graph_git_two_modes "${DIR:+-C $DIR} branch -vv" && + graph_git_two_modes "${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE" ' } From d089a06421c86d120f50f05020ca6b833b068dcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 29 Jul 2023 22:40:27 +0200 Subject: [PATCH 06/34] bundle: use OPT_PASSTHRU_ARGV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "git bundle" passes the progress control options to "git pack-objects" by parsing and then recreating them explicitly. Simplify that process by using OPT_PASSTHRU_ARGV instead. This also fixes --no-quiet, which has been doing the same as --quiet since its introduction by 79862b6b77 (bundle-create: progress output control, 2019-11-10) because it had been defined using OPT_SET_INT with a value of 0, which sets 0 when negated as well. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/bundle.c | 40 +++++++++++++++++----------------------- t/t6020-bundle-misc.sh | 6 ++++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index 44113389d7a34a..8b2acf47346c30 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -69,42 +69,36 @@ static int parse_options_cmd_bundle(int argc, } static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { - int all_progress_implied = 1; - int progress = isatty(STDERR_FILENO); - struct strvec pack_opts; + struct strvec pack_opts = STRVEC_INIT; int version = -1; int ret; struct option options[] = { - OPT_SET_INT('q', "quiet", &progress, - N_("do not show progress meter"), 0), - OPT_SET_INT(0, "progress", &progress, - N_("show progress meter"), 1), - OPT_SET_INT_F(0, "all-progress", &progress, - N_("historical; same as --progress"), 2, - PARSE_OPT_HIDDEN), - OPT_HIDDEN_BOOL(0, "all-progress-implied", - &all_progress_implied, - N_("historical; does nothing")), + OPT_PASSTHRU_ARGV('q', "quiet", &pack_opts, NULL, + N_("do not show progress meter"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV(0, "progress", &pack_opts, NULL, + N_("show progress meter"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV(0, "all-progress", &pack_opts, NULL, + N_("historical; same as --progress"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN), + OPT_PASSTHRU_ARGV(0, "all-progress-implied", &pack_opts, NULL, + N_("historical; does nothing"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN), OPT_INTEGER(0, "version", &version, N_("specify bundle format version")), OPT_END() }; char *bundle_file; + if (isatty(STDERR_FILENO)) + strvec_push(&pack_opts, "--progress"); + strvec_push(&pack_opts, "--all-progress-implied"); + argc = parse_options_cmd_bundle(argc, argv, prefix, builtin_bundle_create_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ - strvec_init(&pack_opts); - if (progress == 0) - strvec_push(&pack_opts, "--quiet"); - else if (progress == 1) - strvec_push(&pack_opts, "--progress"); - else if (progress == 2) - strvec_push(&pack_opts, "--all-progress"); - if (progress && all_progress_implied) - strvec_push(&pack_opts, "--all-progress-implied"); - if (!startup_info->have_repository) die(_("Need a repository to create a bundle.")); ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version); diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh index dface8bcfe2641..3e6bcbf30cdaf0 100755 --- a/t/t6020-bundle-misc.sh +++ b/t/t6020-bundle-misc.sh @@ -619,6 +619,12 @@ test_expect_success TTY 'create --quiet disables all bundle progress' ' test_must_be_empty err ' +test_expect_success 'bundle progress with --no-quiet' ' + GIT_PROGRESS_DELAY=0 \ + git bundle create --no-quiet out.bundle --all 2>err && + grep "%" err +' + test_expect_success 'read bundle over stdin' ' git bundle create some.bundle HEAD && From b4b85e41a74eaf61dfb490004541622e63df092b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 31 Jul 2023 12:08:06 +0000 Subject: [PATCH 07/34] sha256/gcrypt: fix build with SANITIZE=leak Non-static functions cause `undefined reference' errors when building with `SANITIZE=leak' due to the lack of prototypes. Mark all these functions as `static inline' as we do in sha256/nettle.h to avoid the need to maintain prototypes. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- sha256/gcrypt.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h index 501da5ed9197ec..68cf6b6a546a71 100644 --- a/sha256/gcrypt.h +++ b/sha256/gcrypt.h @@ -7,22 +7,22 @@ typedef gcry_md_hd_t gcrypt_SHA256_CTX; -inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx) +static inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx) { gcry_md_open(ctx, GCRY_MD_SHA256, 0); } -inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len) +static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len) { gcry_md_write(*ctx, data, len); } -inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx) +static inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx) { memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE); } -inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src) +static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src) { gcry_md_copy(dst, *src); } From 8b608f3fb84388bb1b6da70feb62e20a19390cb6 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 31 Jul 2023 12:08:07 +0000 Subject: [PATCH 08/34] sha256/gcrypt: fix memory leak with SHA-256 repos `gcry_md_open' needs to be paired with `gcry_md_close' to ensure resources are released. Since our internal APIs don't have separate close/release callbacks, sticking it into the finalization callback seems appropriate. Building with SANITIZE=leak and running `git fsck' on a SHA-256 repository no longer reports leaks. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- sha256/gcrypt.h | 1 + 1 file changed, 1 insertion(+) diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h index 68cf6b6a546a71..1d06a778af1bdc 100644 --- a/sha256/gcrypt.h +++ b/sha256/gcrypt.h @@ -20,6 +20,7 @@ static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data static inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx) { memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE); + gcry_md_close(*ctx); } static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src) From 823839bda1a72c54fe8ac025fb70dd3403c11f46 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 31 Jul 2023 12:08:08 +0000 Subject: [PATCH 09/34] sha256/gcrypt: die on gcry_md_open failures `gcry_md_open' allocates memory and must (like all allocation functions) be checked for failure. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- sha256/gcrypt.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h index 1d06a778af1bdc..17a90f1052526c 100644 --- a/sha256/gcrypt.h +++ b/sha256/gcrypt.h @@ -9,7 +9,9 @@ typedef gcry_md_hd_t gcrypt_SHA256_CTX; static inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx) { - gcry_md_open(ctx, GCRY_MD_SHA256, 0); + gcry_error_t err = gcry_md_open(ctx, GCRY_MD_SHA256, 0); + if (err) + die("gcry_md_open: %s", gcry_strerror(err)); } static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len) From 8e42eb0e9ae44f65c360cd95ce28e84496ad8247 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Mon, 31 Jul 2023 15:42:02 +0200 Subject: [PATCH 10/34] doc: sha256 is no longer experimental Remove scary wording that basically stops people using sha256 repositories not because of interoperability issues with sha1 repositories, but from fear that their work will suddenly become incompatible in some future version of git. We should be clear that currently sha256 repositories will not work with sha1 repositories but stop the scary words. Signed-off-by: Adam Majer Signed-off-by: Junio C Hamano --- Documentation/git.txt | 4 ++-- Documentation/object-format-disclaimer.txt | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index f0cafa22906d60..11228956cd5ec4 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -553,8 +553,8 @@ double-quotes and respecting backslash escapes. E.g., the value If this variable is set, the default hash algorithm for new repositories will be set to this value. This value is ignored when cloning and the setting of the remote repository - is always used. The default is "sha1". THIS VARIABLE IS - EXPERIMENTAL! See `--object-format` in linkgit:git-init[1]. + is always used. The default is "sha1". + See `--object-format` in linkgit:git-init[1]. Git Commits ~~~~~~~~~~~ diff --git a/Documentation/object-format-disclaimer.txt b/Documentation/object-format-disclaimer.txt index 4cb106f0d146e7..e561e6668c9e59 100644 --- a/Documentation/object-format-disclaimer.txt +++ b/Documentation/object-format-disclaimer.txt @@ -1,6 +1,9 @@ -THIS OPTION IS EXPERIMENTAL! SHA-256 support is experimental and still -in an early stage. A SHA-256 repository will in general not be able to -share work with "regular" SHA-1 repositories. It should be assumed -that, e.g., Git internal file formats in relation to SHA-256 -repositories may change in backwards-incompatible ways. Only use -`--object-format=sha256` for testing purposes. +Note: At present, there is no interoperability between SHA-256 +repositories and SHA-1 repositories. + +Historically, we warned that SHA-256 repositories may later need +backward incompatible changes when we introduce such interoperability +features. Today, we only expect compatible changes. Furthermore, if such +changes prove to be necessary, it can be expected that SHA-256 repositories +created with today's Git will be usable by future versions of Git +without data loss. From 3e440ea0aba0660f356a3e5b9fc366d5d6960847 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 1 Aug 2023 02:54:53 +0000 Subject: [PATCH 11/34] sha256: avoid functions deprecated in OpenSSL 3+ OpenSSL 3+ deprecates the SHA256_Init, SHA256_Update, and SHA256_Final functions, leading to errors when building with `DEVELOPER=1'. Use the newer EVP_* API with OpenSSL 3+ despite being more error-prone and less efficient due to heap allocations. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- Makefile | 3 +++ hash-ll.h | 6 +++++- sha256/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 sha256/openssl.h diff --git a/Makefile b/Makefile index e440728c2461b9..29f6d3b31eaf30 100644 --- a/Makefile +++ b/Makefile @@ -3215,6 +3215,9 @@ $(SP_OBJ): %.sp: %.c %.o sparse: $(SP_OBJ) EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% +ifndef OPENSSL_SHA256 + EXCEPT_HDRS += sha256/openssl.h +endif ifndef NETTLE_SHA256 EXCEPT_HDRS += sha256/nettle.h endif diff --git a/hash-ll.h b/hash-ll.h index 80509251370523..5173a2698cea5c 100644 --- a/hash-ll.h +++ b/hash-ll.h @@ -17,7 +17,11 @@ #define SHA256_NEEDS_CLONE_HELPER #include "sha256/gcrypt.h" #elif defined(SHA256_OPENSSL) -#include +# include +# if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3 +# define SHA256_NEEDS_CLONE_HELPER +# include "sha256/openssl.h" +# endif #else #include "sha256/block/sha256.h" #endif diff --git a/sha256/openssl.h b/sha256/openssl.h new file mode 100644 index 00000000000000..c1083d94914621 --- /dev/null +++ b/sha256/openssl.h @@ -0,0 +1,49 @@ +/* wrappers for the EVP API of OpenSSL 3+ */ +#ifndef SHA256_OPENSSL_H +#define SHA256_OPENSSL_H +#include + +struct openssl_SHA256_CTX { + EVP_MD_CTX *ectx; +}; + +typedef struct openssl_SHA256_CTX openssl_SHA256_CTX; + +static inline void openssl_SHA256_Init(struct openssl_SHA256_CTX *ctx) +{ + const EVP_MD *type = EVP_sha256(); + + ctx->ectx = EVP_MD_CTX_new(); + if (!ctx->ectx) + die("EVP_MD_CTX_new: out of memory"); + + EVP_DigestInit_ex(ctx->ectx, type, NULL); +} + +static inline void openssl_SHA256_Update(struct openssl_SHA256_CTX *ctx, + const void *data, + size_t len) +{ + EVP_DigestUpdate(ctx->ectx, data, len); +} + +static inline void openssl_SHA256_Final(unsigned char *digest, + struct openssl_SHA256_CTX *ctx) +{ + EVP_DigestFinal_ex(ctx->ectx, digest, NULL); + EVP_MD_CTX_free(ctx->ectx); +} + +static inline void openssl_SHA256_Clone(struct openssl_SHA256_CTX *dst, + const struct openssl_SHA256_CTX *src) +{ + EVP_MD_CTX_copy_ex(dst->ectx, src->ectx); +} + +#define platform_SHA256_CTX openssl_SHA256_CTX +#define platform_SHA256_Init openssl_SHA256_Init +#define platform_SHA256_Clone openssl_SHA256_Clone +#define platform_SHA256_Update openssl_SHA256_Update +#define platform_SHA256_Final openssl_SHA256_Final + +#endif /* SHA256_OPENSSL_H */ From bda9c12073e786e2ffa2c3ec479c7fe098d49999 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 1 Aug 2023 02:54:54 +0000 Subject: [PATCH 12/34] avoid SHA-1 functions deprecated in OpenSSL 3+ OpenSSL 3+ deprecates the SHA1_Init, SHA1_Update, and SHA1_Final functions, leading to errors when building with `DEVELOPER=1'. Use the newer EVP_* API with OpenSSL 3+ (only) despite being more error-prone and less efficient due to heap allocations. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- Makefile | 3 +++ hash-ll.h | 12 +++++++++++- sha1/openssl.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 sha1/openssl.h diff --git a/Makefile b/Makefile index 29f6d3b31eaf30..e9197b4acc11e5 100644 --- a/Makefile +++ b/Makefile @@ -3215,6 +3215,9 @@ $(SP_OBJ): %.sp: %.c %.o sparse: $(SP_OBJ) EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% +ifndef OPENSSL_SHA1 + EXCEPT_HDRS += sha1/openssl.h +endif ifndef OPENSSL_SHA256 EXCEPT_HDRS += sha256/openssl.h endif diff --git a/hash-ll.h b/hash-ll.h index 5173a2698cea5c..0cc73dffc5e84b 100644 --- a/hash-ll.h +++ b/hash-ll.h @@ -4,7 +4,11 @@ #if defined(SHA1_APPLE) #include #elif defined(SHA1_OPENSSL) -#include +# include +# if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3 +# define SHA1_NEEDS_CLONE_HELPER +# include "sha1/openssl.h" +# endif #elif defined(SHA1_DC) #include "sha1dc_git.h" #else /* SHA1_BLK */ @@ -45,6 +49,10 @@ #define git_SHA1_Update platform_SHA1_Update #define git_SHA1_Final platform_SHA1_Final +#ifdef platform_SHA1_Clone +#define git_SHA1_Clone platform_SHA1_Clone +#endif + #ifndef platform_SHA256_CTX #define platform_SHA256_CTX SHA256_CTX #define platform_SHA256_Init SHA256_Init @@ -67,10 +75,12 @@ #define git_SHA1_Update git_SHA1_Update_Chunked #endif +#ifndef SHA1_NEEDS_CLONE_HELPER static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src) { memcpy(dst, src, sizeof(*dst)); } +#endif #ifndef SHA256_NEEDS_CLONE_HELPER static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *src) diff --git a/sha1/openssl.h b/sha1/openssl.h new file mode 100644 index 00000000000000..006c1f4ba541cb --- /dev/null +++ b/sha1/openssl.h @@ -0,0 +1,49 @@ +/* wrappers for the EVP API of OpenSSL 3+ */ +#ifndef SHA1_OPENSSL_H +#define SHA1_OPENSSL_H +#include + +struct openssl_SHA1_CTX { + EVP_MD_CTX *ectx; +}; + +typedef struct openssl_SHA1_CTX openssl_SHA1_CTX; + +static inline void openssl_SHA1_Init(struct openssl_SHA1_CTX *ctx) +{ + const EVP_MD *type = EVP_sha1(); + + ctx->ectx = EVP_MD_CTX_new(); + if (!ctx->ectx) + die("EVP_MD_CTX_new: out of memory"); + + EVP_DigestInit_ex(ctx->ectx, type, NULL); +} + +static inline void openssl_SHA1_Update(struct openssl_SHA1_CTX *ctx, + const void *data, + size_t len) +{ + EVP_DigestUpdate(ctx->ectx, data, len); +} + +static inline void openssl_SHA1_Final(unsigned char *digest, + struct openssl_SHA1_CTX *ctx) +{ + EVP_DigestFinal_ex(ctx->ectx, digest, NULL); + EVP_MD_CTX_free(ctx->ectx); +} + +static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst, + const struct openssl_SHA1_CTX *src) +{ + EVP_MD_CTX_copy_ex(dst->ectx, src->ectx); +} + +#define platform_SHA_CTX openssl_SHA1_CTX +#define platform_SHA1_Init openssl_SHA1_Init +#define platform_SHA1_Clone openssl_SHA1_Clone +#define platform_SHA1_Update openssl_SHA1_Update +#define platform_SHA1_Final openssl_SHA1_Final + +#endif /* SHA1_OPENSSL_H */ From 1c04cb0744d2acdcaebc77b0e78c47efbba67fd5 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Wed, 2 Aug 2023 09:49:32 -0700 Subject: [PATCH 13/34] ident: don't consider '.' a crud When we process a user's name (as in user.name), we strip all leading and trailing crud from it. Right now, we consider a dot a crud character, and strip it off. However, this is unsuitable for many personal names because humans frequently have abbreviated suffixes, such as "Jr." or "Sr." at the end of their names, and this corrupts them. Some other users may wish to use an abbreviated name or initial, which will pose a problem especially in cultures that write the family name first, followed by the personal name. Since the current approach causes lots of practical problems, let's avoid it by no longer considering a dot to be crud. Note that "." in the name forces the entire name to be quoted to please mailers, but stripping "." only at the beginning and the end does not help a name with "." in the middle (like "brian m. carlson") so this change will not make it much worse. A name like "Given Family, Jr." that did not have to be quoted now would need to be, in order to be placed on the e-mail headers, though. This is based on a weather-balloon patch by Jeff King sent in Aug 2021 https://lore.kernel.org/git/YSKm8Q8nyTavQaox@coredump.intra.peff.net/ Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- ident.c | 1 - t/t4203-mailmap.sh | 4 ++-- t/t7518-ident-corner-cases.sh | 11 ++++++++++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ident.c b/ident.c index 8fad92d7007e21..8d490df7d53710 100644 --- a/ident.c +++ b/ident.c @@ -203,7 +203,6 @@ void reset_ident_date(void) static int crud(unsigned char c) { return c <= 32 || - c == '.' || c == ',' || c == ':' || c == ';' || diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index fa7f987284b117..2016132f516174 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -466,7 +466,7 @@ test_expect_success 'gitmailmap(5) example output: example #1' ' Author Jane Doe maps to Jane Doe Committer C O Mitter maps to C O Mitter - Author Jane D maps to Jane Doe + Author Jane D. maps to Jane Doe Committer C O Mitter maps to C O Mitter EOF git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual && @@ -494,7 +494,7 @@ test_expect_success 'gitmailmap(5) example output: example #2' ' Author Jane Doe maps to Jane Doe Committer C O Mitter maps to C O Mitter - Author Jane D maps to Jane Doe + Author Jane D. maps to Jane Doe Committer C O Mitter maps to C O Mitter EOF git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual && diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh index fffdb6ff2e751d..9ab2ae2f3b2380 100755 --- a/t/t7518-ident-corner-cases.sh +++ b/t/t7518-ident-corner-cases.sh @@ -20,10 +20,19 @@ test_expect_success 'empty name and missing email' ' ' test_expect_success 'commit rejects all-crud name' ' - test_must_fail env GIT_AUTHOR_NAME=" .;<>" \ + test_must_fail env GIT_AUTHOR_NAME=" ,;<>" \ git commit --allow-empty -m foo ' +test_expect_success 'commit does not strip trailing dot' ' + author_name="Pat Doe Jr." && + env GIT_AUTHOR_NAME="$author_name" \ + git commit --allow-empty -m foo && + git log -1 --format=%an >actual && + echo "$author_name" >expected && + test_cmp actual expected +' + # We must test the actual error message here, as an unwanted # auto-detection could fail for other reasons. test_expect_success 'empty configured name does not auto-detect' ' From 6ce7afe16384b741f1ee4c5f310fa4a9f66348ba Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 3 Aug 2023 13:09:35 +0000 Subject: [PATCH 14/34] rebase --skip: fix commit message clean up when skipping squash During a series of "fixup" and/or "squash" commands, the interactive rebase accumulates a commit message from all the commits that are being squashed together. If one of the commits has conflicts when it is picked and the user chooses to skip that commit then we need to remove that commit's message from accumulated messages. To do this 15ef69314d5 (rebase --skip: clean up commit message after a failed fixup/squash, 2018-04-27) updated commit_staged_changes() to reset the accumulated message to the commit message of HEAD (which does not contain the message from the skipped commit) when the last command was "fixup" or "squash" and there are no staged changes. Unfortunately the code to do this contains two bugs. (1) If parse_head() fails we pass an invalid pointer to unuse_commit_buffer(). (2) The reconstructed message uses the entire commit buffer from HEAD including the headers, rather than just the commit message. The first issue is fixed by splitting up the "if" condition into several statements each with its own error handling. The second issue is fixed by finding the start of the commit message within the commit buffer using find_commit_subject(). The existing test added by 15ef69314d5 is modified to show the effect of this bug. The bug is triggered when skipping the first command in the chain (as the test does before this commit) but the effect is hidden because opts->current_fixup_count is set to zero which leads update_squash_messages() to recreate the squash message file from scratch overwriting the bad message created by commit_staged_changes(). The test is also updated to explicitly check the commit messages rather than relying on grep to ensure they do not contain any stray commit headers. To check the commit message the function test_commit_message() is moved from t3437-rebase-fixup-options.sh to test-lib.sh. As the function is now publicly available it is updated to provide better error detection and avoid overwriting the commonly used files "actual" and "expect". Support for reading the expected commit message from stdin is also added. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 26 +++++++++++---- t/t3418-rebase-continue.sh | 58 +++++++++++++++++++++++---------- t/t3437-rebase-fixup-options.sh | 15 --------- t/test-lib-functions.sh | 33 +++++++++++++++++++ 4 files changed, 93 insertions(+), 39 deletions(-) diff --git a/sequencer.c b/sequencer.c index bceb6abcb6c733..af271ab6fbdee6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5038,19 +5038,31 @@ static int commit_staged_changes(struct repository *r, * We need to update the squash message to skip * the latest commit message. */ + int res = 0; struct commit *commit; + const char *msg; const char *path = rebase_path_squash_msg(); const char *encoding = get_commit_output_encoding(); - if (parse_head(r, &commit) || - !(p = repo_logmsg_reencode(r, commit, NULL, encoding)) || - write_message(p, strlen(p), path, 0)) { - repo_unuse_commit_buffer(r, commit, p); - return error(_("could not write file: " + if (parse_head(r, &commit)) + return error(_("could not parse HEAD")); + + p = repo_logmsg_reencode(r, commit, NULL, encoding); + if (!p) { + res = error(_("could not parse commit %s"), + oid_to_hex(&commit->object.oid)); + goto unuse_commit_buffer; + } + find_commit_subject(p, &msg); + if (write_message(msg, strlen(msg), path, 0)) { + res = error(_("could not write file: " "'%s'"), path); + goto unuse_commit_buffer; } - repo_unuse_commit_buffer(r, - commit, p); + unuse_commit_buffer: + repo_unuse_commit_buffer(r, commit, p); + if (res) + return res; } } diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 2d0789e554bf00..fb7b68990cc33a 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -115,15 +115,23 @@ test_expect_success '--skip after failed fixup cleans commit message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b with-conflicting-fixup && test_commit wants-fixup && - test_commit "fixup! wants-fixup" wants-fixup.t 1 wants-fixup-1 && - test_commit "fixup! wants-fixup" wants-fixup.t 2 wants-fixup-2 && - test_commit "fixup! wants-fixup" wants-fixup.t 3 wants-fixup-3 && + test_commit "fixup 1" wants-fixup.t 1 wants-fixup-1 && + test_commit "fixup 2" wants-fixup.t 2 wants-fixup-2 && + test_commit "fixup 3" wants-fixup.t 3 wants-fixup-3 && test_must_fail env FAKE_LINES="1 fixup 2 squash 4" \ git rebase -i HEAD~4 && : now there is a conflict, and comments in the commit message && - git show HEAD >out && - grep "fixup! wants-fixup" out && + test_commit_message HEAD <<-\EOF && + # This is a combination of 2 commits. + # This is the 1st commit message: + + wants-fixup + + # The commit message #2 will be skipped: + + # fixup 1 + EOF : skip and continue && echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh && @@ -133,33 +141,49 @@ test_expect_success '--skip after failed fixup cleans commit message' ' test_path_is_missing .git/copy.txt && : now the comments in the commit message should have been cleaned up && - git show HEAD >out && - ! grep "fixup! wants-fixup" out && + test_commit_message HEAD -m wants-fixup && : now, let us ensure that "squash" is handled correctly && git reset --hard wants-fixup-3 && - test_must_fail env FAKE_LINES="1 squash 4 squash 2 squash 4" \ + test_must_fail env FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1" \ git rebase -i HEAD~4 && - : the first squash failed, but there are two more in the chain && + : the second squash failed, but there are two more in the chain && (test_set_editor "$PWD/copy-editor.sh" && test_must_fail git rebase --skip) && : not the final squash, no need to edit the commit message && test_path_is_missing .git/copy.txt && - : The first squash was skipped, therefore: && - git show HEAD >out && - test_i18ngrep "# This is a combination of 2 commits" out && - test_i18ngrep "# This is the commit message #2:" out && + : The first and third squashes succeeded, therefore: && + test_commit_message HEAD <<-\EOF && + # This is a combination of 3 commits. + # This is the 1st commit message: + + wants-fixup + + # This is the commit message #2: + + fixup 1 + + # This is the commit message #3: + + fixup 2 + EOF (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) && - git show HEAD >out && - test_i18ngrep ! "# This is a combination" out && + test_commit_message HEAD <<-\EOF && + wants-fixup + + fixup 1 + + fixup 2 + EOF : Final squash failed, but there was still a squash && - test_i18ngrep "# This is a combination of 2 commits" .git/copy.txt && - test_i18ngrep "# This is the commit message #2:" .git/copy.txt + head -n1 .git/copy.txt >first-line && + test_i18ngrep "# This is a combination of 3 commits" first-line && + test_i18ngrep "# This is the commit message #3:" .git/copy.txt ' test_expect_success 'setup rerere database' ' diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh index dd3b301fa7ab2b..7929e2e2e3a8fc 100755 --- a/t/t3437-rebase-fixup-options.sh +++ b/t/t3437-rebase-fixup-options.sh @@ -21,21 +21,6 @@ TEST_PASSES_SANITIZE_LEAK=true EMPTY="" -# test_commit_message -m -# test_commit_message -# Verify that the commit message of matches -# or the content of . -test_commit_message () { - git show --no-patch --pretty=format:%B "$1" >actual && - case "$2" in - -m) - echo "$3" >expect && - test_cmp expect actual ;; - *) - test_cmp "$2" actual ;; - esac -} - get_author () { rev="$1" && git log -1 --pretty=format:"%an %ae %at" "$rev" diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 6e19ebc922a416..d8a52334eebc16 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1273,6 +1273,39 @@ test_cmp_rev () { fi } +# Tests that a commit message matches the expected text +# +# Usage: test_commit_message [-m | ] +# +# When using "-m" will have a line feed appended. If the second +# argument is omitted then the expected message is read from stdin. + +test_commit_message () { + local msg_file=expect.msg + + case $# in + 3) + if test "$2" = "-m" + then + printf "%s\n" "$3" >"$msg_file" + else + BUG "Usage: test_commit_message [-m | ]" + fi + ;; + 2) + msg_file="$2" + ;; + 1) + cat >"$msg_file" + ;; + *) + BUG "Usage: test_commit_message [-m | ]" + ;; + esac + git show --no-patch --pretty=format:%B "$1" -- >actual.msg && + test_cmp "$msg_file" actual.msg +} + # Compare paths respecting core.ignoreCase test_cmp_fspath () { if test "x$1" = "x$2" From bb532b534547d0a6b203baaf2f3379d987f611b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Fri, 4 Aug 2023 04:08:42 +0000 Subject: [PATCH 15/34] run-command: conditionally define locate_in_PATH() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit doesn't change any behaviour by itself, but allows us to easily define compat replacements for locate_in_PATH(). It prepares us for the next commit that adds a native Windows implementation of locate_in_PATH(). Signed-off-by: Matthias Aßhauer Signed-off-by: Junio C Hamano --- run-command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/run-command.c b/run-command.c index 60c94198664738..85fc15072888e9 100644 --- a/run-command.c +++ b/run-command.c @@ -170,6 +170,7 @@ int is_executable(const char *name) return st.st_mode & S_IXUSR; } +#ifndef locate_in_PATH /* * Search $PATH for a command. This emulates the path search that * execvp would perform, without actually executing the command so it @@ -218,6 +219,7 @@ static char *locate_in_PATH(const char *file) strbuf_release(&buf); return NULL; } +#endif int exists_in_PATH(const char *command) { From 2bf46a9f62159ced3a84ab8bc9ba151778414bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Fri, 4 Aug 2023 04:08:43 +0000 Subject: [PATCH 16/34] compat/mingw: implement a native locate_in_PATH() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit since 5e1f28d (bisect--helper: reimplement `bisect_visualize()` shell function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH() to check wether it should call `gitk`, but exists_in_PATH() relies on locate_in_PATH() which currently only understands POSIX-ish PATH variables (a list of paths, separated by colons) on native Windows executables we encounter Windows PATH variables (a list of paths that often contain drive letters (and thus colons), separated by semicolons). Luckily we do already have a function that can lookup executables on windows PATHs: path_lookup(). Implement a small replacement for the existing locate_in_PATH() based on path_lookup(). Reported-by: Louis Strous Signed-off-by: Matthias Aßhauer Signed-off-by: Junio C Hamano --- compat/mingw.c | 5 +++++ compat/mingw.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index d06cdc6254fbc3..bc3669d2986d88 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1347,6 +1347,11 @@ static char *path_lookup(const char *cmd, int exe_only) return prog; } +char *mingw_locate_in_PATH(const char *cmd) +{ + return path_lookup(cmd, 0); +} + static const wchar_t *wcschrnul(const wchar_t *s, wchar_t c) { while (*s && *s != c) diff --git a/compat/mingw.h b/compat/mingw.h index 209cf7cebadd17..b5262205965829 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -175,6 +175,9 @@ pid_t waitpid(pid_t pid, int *status, int options); #define kill mingw_kill int mingw_kill(pid_t pid, int sig); +#define locate_in_PATH mingw_locate_in_PATH +char *mingw_locate_in_PATH(const char *cmd); + #ifndef NO_OPENSSL #include static inline int mingw_SSL_set_fd(SSL *ssl, int fd) From fff1594fa77372ea7a51f6b445267f23fdbf3089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Fri, 4 Aug 2023 04:08:44 +0000 Subject: [PATCH 17/34] docs: update when `git bisect visualize` uses `gitk` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This check has involved more environment variables than just `DISPLAY` since 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11, 2008-02-14), so let's update the documentation accordingly. Signed-off-by: Matthias Aßhauer Signed-off-by: Junio C Hamano --- Documentation/git-bisect.txt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index fbb39fbdf5d62a..7872dba3aefa3d 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -204,9 +204,14 @@ as an alternative to `visualize`): $ git bisect visualize ------------ -If the `DISPLAY` environment variable is not set, 'git log' is used -instead. You can also give command-line options such as `-p` and -`--stat`. +Git detects a graphical environment through various environment variables: +`DISPLAY`, which is set in X Window System environments on Unix systems. +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions. +`MSYSTEM`, which is set under Msys2 and Git for Windows. +`SECURITYSESSIONID`, which may be set on macOS in interactive desktop sessions. + +If none of these environment variables is set, 'git log' is used instead. +You can also give command-line options such as `-p` and `--stat`. ------------ $ git bisect visualize --stat From a5c01603b397f7f99b013a1334e0792d70be641c Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Fri, 4 Aug 2023 17:13:28 +0000 Subject: [PATCH 18/34] gitignore: ignore clangd .cache directory In at least some versions of clangd, including version 15 in Ubuntu 23.04, a directory, .cache, is written in the root of the repository with index information about the files in the repository. Since clangd is the most common language server protocol (LSP) implementation for C, and we already support it using the GENERATE_COMPILATION_DATABASE flags to make it functional, it's likely many users are using or will want to use it. As a result, ignore the ".cache" directory to help avoid users accidentally committing the data. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e875c590545588..5e56e471b344ef 100644 --- a/.gitignore +++ b/.gitignore @@ -222,6 +222,7 @@ /TAGS /cscope* /compile_commands.json +/.cache/ *.hcc *.obj *.lib From 1221e94bd0ad7f5734cdbfe1ebc5b922bea106fa Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Mon, 7 Aug 2023 18:41:50 +0000 Subject: [PATCH 19/34] mailmap: change primary address for Glen Choo Glen will lose access to his work email soon. Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 733e047aa82507..dc31d70b8c1e98 100644 --- a/.mailmap +++ b/.mailmap @@ -80,6 +80,7 @@ Frank Lichtenheld Fredrik Kuivinen Frédéric Heitzmann Garry Dolley +Glen Choo Greg Price Greg Price Heiko Voigt From a82fb66fed250e16d3010c75404503bea3f0ab61 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 7 Aug 2023 10:18:48 -0700 Subject: [PATCH 20/34] A few more topics before -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.42.0.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/RelNotes/2.42.0.txt b/Documentation/RelNotes/2.42.0.txt index 0d31aed9f9d0e9..f2262f84bf998f 100644 --- a/Documentation/RelNotes/2.42.0.txt +++ b/Documentation/RelNotes/2.42.0.txt @@ -38,6 +38,12 @@ UI, Workflows & Features being bisected or rebased. The message was reworded to say the branch was "in use". + * Tone down the warning on SHA-256 repositories being an experimental + curiosity. We do not have support for them to interoperate with + traditional SHA-1 repositories, but at this point, we do not plan + to make breaking changes to SHA-256 repositories and there is no + longer need for such a strongly phrased warning. + Performance, Internal Implementation, Development Support etc. @@ -286,3 +292,5 @@ Fixes since v2.41 (merge c95ae3ff9c rs/describe-parseopt-fix later to maint). (merge 36f76d2a25 rs/pack-objects-parseopt-fix later to maint). (merge 30c8c55cbf jc/tree-walk-drop-base-offset later to maint). + (merge d089a06421 rs/bundle-parseopt-cleanup later to maint). + (merge 823839bda1 ew/sha256-gcrypt-leak-fixes later to maint). From ff29a61cbbc83f0948606df37dbc734436b62c17 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Mon, 7 Aug 2023 19:09:35 +0200 Subject: [PATCH 21/34] advice: handle "rebase" in error_resolve_conflict() This makes sure that we get a properly translated message rather than inserting the command (which we failed to translate) into a generic fallback message. The function is called indirectly via die_resolve_conflict() with fixed strings, and directly with the string obtained via action_name(), which in turn returns a string from a fixed set. Hence we know that the now covered set of strings is exhausitive, and will therefore BUG() out when encountering an unexpected string. We also know that all covered strings are actually used. Arguably, the above suggests that it would be cleaner to pass the command as an enum in the first place, but that's left for another time. Signed-off-by: Oswald Buddenhagen Signed-off-by: Junio C Hamano --- advice.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/advice.c b/advice.c index e5a9bb9b446e95..50c79443ba749f 100644 --- a/advice.c +++ b/advice.c @@ -191,9 +191,10 @@ int error_resolve_conflict(const char *me) error(_("Pulling is not possible because you have unmerged files.")); else if (!strcmp(me, "revert")) error(_("Reverting is not possible because you have unmerged files.")); + else if (!strcmp(me, "rebase")) + error(_("Rebasing is not possible because you have unmerged files.")); else - error(_("It is not possible to %s because you have unmerged files."), - me); + BUG("Unhandled conflict reason '%s'", me); if (advice_enabled(ADVICE_RESOLVE_CONFLICT)) /* From 12009a182b51c1dd1f8020a3d88a1813e0af5f33 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 8 Aug 2023 10:37:34 +0000 Subject: [PATCH 22/34] t0040: declare non-tab indentation to be okay in this script By necessity, this script needs to verify that certain Git output matches expectations, including text indented with spaces instead of tabs. Most recently, such a check was introduced in 448abbba6347 (short help: allow multi-line opthelp, 2023-07-18) which is reported by `git diff --check 448abbba6347^!` as having whitespace issues. Let's not complain about this because it is intentional. Signed-off-by: Johannes Schindelin Acked-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/.gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/t/.gitattributes b/t/.gitattributes index 9930e283512b66..b9cea1795d84e3 100644 --- a/t/.gitattributes +++ b/t/.gitattributes @@ -22,3 +22,4 @@ t[0-9][0-9][0-9][0-9]/* -whitespace /t7500/* eol=lf /t8005/*.txt eol=lf /t9*/*.dump eol=lf +/t0040*.sh whitespace=-indent-with-non-tab From dfd46bae92ec25d71e7c8a2887e9508aaf211ee8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 8 Aug 2023 14:14:36 -0400 Subject: [PATCH 23/34] send-email: drop FakeTerm hack Back in 280242d1cc (send-email: do not barf when Term::ReadLine does not like your terminal, 2006-07-02), we added a fallback for when Term::ReadLine's constructor failed: we'd have a FakeTerm object instead, which would then die if anybody actually tried to call readline() on it. Since we instantiated the $term variable at program startup, we needed this workaround to let the program run in modes when we did not prompt the user. But later, in f4dc9432fd (send-email: lazily load modules for a big speedup, 2021-05-28), we started loading Term::ReadLine lazily only when ask() is called. So at that point we know we're trying to prompt the user, and we can just die if ReadLine instantiation fails, rather than making this fake object to lazily delay showing the error. This should be OK even if there is no tty (e.g., we're in a cron job), because Term::ReadLine will return a stub object in that case whose "IN" and "OUT" functions return undef. And since 5906f54e47 (send-email: don't attempt to prompt if tty is closed, 2009-03-31), we check for that case and skip prompting. And we can be sure that FakeTerm was not kicking in for such a situation, because it has actually been broken since that commit! It does not define "IN" or "OUT" methods, so perl would barf with an error. If FakeTerm was in use, we were neither honoring what 5906f54e47 tried to do, nor producing the readable message that 280242d1cc intended. So we're better off just dropping FakeTerm entirely, and letting the error reported by constructing Term::ReadLine through. Signed-off-by: Jeff King Acked-by: Taylor Blau Signed-off-by: Junio C Hamano --- git-send-email.perl | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index affbb88509b839..4cb6ee93271098 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -26,18 +26,6 @@ Getopt::Long::Configure qw/ pass_through /; -package FakeTerm; -sub new { - my ($class, $reason) = @_; - return bless \$reason, shift; -} -sub readline { - my $self = shift; - die "Cannot use readline on FakeTerm: $$self"; -} -package main; - - sub usage { print <] @@ -972,16 +960,10 @@ sub get_patch_subject { } sub term { - my $term = eval { - require Term::ReadLine; - $ENV{"GIT_SEND_EMAIL_NOTTY"} + require Term::ReadLine; + return $ENV{"GIT_SEND_EMAIL_NOTTY"} ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) : Term::ReadLine->new('git-send-email'); - }; - if ($@) { - $term = FakeTerm->new("$@: going non-interactive"); - } - return $term; } sub ask { From c016726c2deb84bcc6a7418efad92ef0562a8af3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 8 Aug 2023 14:15:31 -0400 Subject: [PATCH 24/34] send-email: avoid creating more than one Term::ReadLine object Every time git-send-email calls its ask() function to prompt the user, we call term(), which instantiates a new Term::ReadLine object. But in v1.46 of Term::ReadLine::Gnu (which provides the Term::ReadLine interface on some platforms), its constructor refuses to create a second instance[1]. So on systems with that version of the module, most git-send-email instances will fail (as we usually prompt for both "to" and "in-reply-to" unless the user provided them on the command line). We can fix this by keeping a single instance variable and returning it for each call to term(). In perl 5.10 and up, we could do that with a "state" variable. But since we only require 5.008, we'll do it the old-fashioned way, with a lexical "my" in its own scope. Note that the tests in t9001 detect this problem as-is, since the failure mode is for the program to die. But let's also beef up the "Prompting works" test to check that it correctly handles multiple inputs (if we had chosen to keep our FakeTerm hack in the previous commit, then the failure mode would be incorrectly ignoring prompts after the first). [1] For discussion of why multiple instances are forbidden, see: https://github.com/hirooih/perl-trg/issues/16 Signed-off-by: Jeff King Acked-by: Taylor Blau Signed-off-by: Junio C Hamano --- git-send-email.perl | 18 +++++++++++++----- t/t9001-send-email.sh | 5 +++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 4cb6ee93271098..897cea6564fb50 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -959,11 +959,19 @@ sub get_patch_subject { do_edit(@files); } -sub term { - require Term::ReadLine; - return $ENV{"GIT_SEND_EMAIL_NOTTY"} - ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) - : Term::ReadLine->new('git-send-email'); +{ + # Only instantiate one $term per program run, since some + # Term::ReadLine providers refuse to create a second instance. + my $term; + sub term { + require Term::ReadLine; + if (!defined $term) { + $term = $ENV{"GIT_SEND_EMAIL_NOTTY"} + ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) + : Term::ReadLine->new('git-send-email'); + } + return $term; + } } sub ask { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 22fc9080242c2a..b5be8251ee1f1b 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -337,13 +337,14 @@ test_expect_success $PREREQ 'Show all headers' ' test_expect_success $PREREQ 'Prompting works' ' clean_fake_sendmail && (echo "to@example.com" && - echo "" + echo "my-message-id@example.com" ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ --smtp-server="$(pwd)/fake.sendmail" \ $patches \ 2>errors && grep "^From: A U Thor \$" msgtxt1 && - grep "^To: to@example.com\$" msgtxt1 + grep "^To: to@example.com\$" msgtxt1 && + grep "^In-Reply-To: " msgtxt1 ' test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' ' From cb888bb6991bb10bddedf9ddc9651ec25da6137d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 8 Aug 2023 14:50:23 -0400 Subject: [PATCH 25/34] repack: free geometry struct When the program is ending, we call clear_pack_geometry() to free any resources in the pack_geometry struct. But the struct itself is allocated on the heap, and leak-checkers will complain about the resulting small leak. This one was marked by Coverity as a "new" leak, though it has existed since 0fabafd0b9 (builtin/repack.c: add '--geometric' option, 2021-02-22). This might be because recent unrelated changes in the file confused it about what is new and what is not. But regardless, it is worth addressing. We can fix it easily by free-ing the struct. We'll convert our "clear" function to "free", since the allocation happens in the matching init() function (though since there is only one call to each, and the struct is local to this file, it's mostly academic). Another option would be to put the struct on the stack rather than the heap. However, this gets tricky, as we check the pointer against NULL in several places to decide whether we're in geometric mode. Signed-off-by: Jeff King Acked-by: Taylor Blau Signed-off-by: Junio C Hamano --- builtin/repack.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index aea5ca9d441657..97051479e49bf1 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -492,15 +492,13 @@ static struct packed_git *get_preferred_pack(struct pack_geometry *geometry) return NULL; } -static void clear_pack_geometry(struct pack_geometry *geometry) +static void free_pack_geometry(struct pack_geometry *geometry) { if (!geometry) return; free(geometry->pack); - geometry->pack_nr = 0; - geometry->pack_alloc = 0; - geometry->split = 0; + free(geometry); } struct midx_snapshot_ref_data { @@ -1228,7 +1226,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) string_list_clear(&names, 1); string_list_clear(&existing_nonkept_packs, 0); string_list_clear(&existing_kept_packs, 0); - clear_pack_geometry(geometry); + free_pack_geometry(geometry); return ret; } From 3284b93862b0aaea9d8e708f0aabd53e3e94409e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 8 Aug 2023 22:05:57 +0200 Subject: [PATCH 26/34] parse-options: disallow negating OPTION_SET_INT 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An option of type OPTION_SET_INT can be defined to set its variable to zero. It's negated variant will do the same, though, which is confusing. Several such options were fixed by disabling negation, changing the value to set or using a different option type: 991c552916 (ls-tree: fix --no-full-name, 2023-07-18) e12cb98e1e (branch: reject "--no-all" and "--no-remotes" early, 2023-07-18) 68cbb20e73 (show-branch: reject --[no-](topo|date)-order, 2023-07-19) 3821eb6c3d (reset: reject --no-(mixed|soft|hard|merge|keep) option, 2023-07-19) 36f76d2a25 (pack-objects: fix --no-quiet, 2023-07-21) 3a5f308741 (pack-objects: fix --no-keep-true-parents, 2023-07-21) c95ae3ff9c (describe: fix --no-exact-match, 2023-07-21) d089a06421 (bundle: use OPT_PASSTHRU_ARGV, 2023-07-29) Check for such options that allow negation in parse_options_check() and report them to find future cases quicker. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- parse-options.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/parse-options.c b/parse-options.c index 87c9fae6349650..60224cf8d03fb1 100644 --- a/parse-options.c +++ b/parse-options.c @@ -480,6 +480,9 @@ static void parse_options_check(const struct option *opts) opts->long_name)) optbug(opts, "uses feature " "not supported for dashless options"); + if (opts->type == OPTION_SET_INT && !opts->defval && + opts->long_name && !(opts->flags & PARSE_OPT_NONEG)) + optbug(opts, "OPTION_SET_INT 0 should not be negatable"); switch (opts->type) { case OPTION_COUNTUP: case OPTION_BIT: From 72695d8214791161a943086e894874b4fd71ba9f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 9 Aug 2023 07:47:41 +0000 Subject: [PATCH 27/34] mv: handle lstat() failure correctly When moving a directory onto another with `git mv` various checks are performed. One of of these validates that the destination is not existing. When calling `lstat` on the destination path and it fails as the path doesn't exist, some environments seem to overwrite the passed in `stat` memory nonetheless (I observed this issue on debian 12 of x86_64, running on OrbStack on ARM, emulated with Rosetta). This would affect the code that followed as it would still acccess a now modified `st` structure, which now seems to contain uninitialized memory. `S_ISDIR(st_dir_mode)` would then typically return false causing the code to run into a bad case. The fix avoids overwriting the existing `st` structure, providing an alternative that exists only for that purpose. Note that this patch minimizes complexity instead of stack-frame size. Signed-off-by: Sebastian Thiel Signed-off-by: Junio C Hamano --- builtin/mv.c | 4 ++-- t/t7001-mv.sh | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 665bd274485f6c..5213d993811134 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -183,7 +183,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) int src_dir_nr = 0, src_dir_alloc = 0; struct strbuf a_src_dir = STRBUF_INIT; enum update_mode *modes, dst_mode = 0; - struct stat st; + struct stat st, dest_st; struct string_list src_for_dst = STRING_LIST_INIT_NODUP; struct lock_file lock_file = LOCK_INIT; struct cache_entry *ce; @@ -303,7 +303,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) goto act_on_entry; } if (S_ISDIR(st.st_mode) - && lstat(dst, &st) == 0) { + && lstat(dst, &dest_st) == 0) { bad = _("cannot move directory over file"); goto act_on_entry; } diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 898a92053282d7..f136ea76f7f8a0 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -174,6 +174,13 @@ test_expect_success 'do not move directory over existing directory' ' test_must_fail git mv path2 path0 ' +test_expect_success 'rename directory to non-existing directory' ' + mkdir dir-a && + >dir-a/f && + git add dir-a && + git mv dir-a non-existing-dir +' + test_expect_success 'move into "."' ' git mv path1/path2/ . ' From b3dcd24b8a186093cd821697b191080b8d53915b Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Wed, 9 Aug 2023 19:15:31 +0200 Subject: [PATCH 28/34] t9001: remove excessive GIT_SEND_EMAIL_NOTTY=1 This was added by 3ece9bf0f9 (send-email: clear the $message_id after validation, 2023-05-17) for no apparent reason, as this is required only in cases when git's stdin is (must be) redirected, which isn't the case here. Signed-off-by: Oswald Buddenhagen Signed-off-by: Junio C Hamano --- t/t9001-send-email.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 20511032263ae1..2020e0ce69e673 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -592,7 +592,6 @@ test_expect_success $PREREQ 'clear message-id before parsing a new message' ' clean_fake_sendmail && echo true | write_script my-hooks/sendemail-validate && test_config core.hooksPath my-hooks && - GIT_SEND_EMAIL_NOTTY=1 \ git send-email --validate --to=recipient@example.com \ --smtp-server="$(pwd)/fake.sendmail" \ $patches $threaded_patches && From 4b8a2717bb5ea6b787e0037174dcbf53ccf4985c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Aug 2023 16:54:46 +0000 Subject: [PATCH 29/34] win32: add a helper to run `git.exe` without a foreground window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, there are two kinds of executables, console ones and non-console ones. Git's executables are all console ones. When launching the former e.g. in a scheduled task, a CMD window pops up. This is not what we want for the tasks installed via the `git maintenance` command. To work around this, let's introduce `headless-git.exe`, which is a non-console program that does _not_ pop up any window. All it does is to re-launch `git.exe`, suppressing that console window, passing through all command-line arguments as-are. Helped-by: Carlo Marcelo Arenas Belón Helped-by: Yuyi Wang Signed-off-by: Johannes Schindelin Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Makefile | 9 ++ compat/win32/headless.c | 115 +++++++++++++++++++++ config.mak.uname | 3 + contrib/buildsystems/CMakeLists.txt | 9 ++ contrib/buildsystems/Generators/Vcxproj.pm | 4 +- contrib/buildsystems/engine.pl | 1 + 6 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 compat/win32/headless.c diff --git a/Makefile b/Makefile index e440728c2461b9..c2e2fe44ac6f05 100644 --- a/Makefile +++ b/Makefile @@ -2778,6 +2778,13 @@ compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \ compat/nedmalloc/nedmalloc.sp: SP_EXTRA_FLAGS += -Wno-non-pointer-null endif +headless-git.o: compat/win32/headless.c GIT-CFLAGS + $(QUIET_CC)$(CC) $(ALL_CFLAGS) $(COMPAT_CFLAGS) \ + -fno-stack-protector -o $@ -c -Wall -Wwrite-strings $< + +headless-git$X: headless-git.o git.res GIT-LDFLAGS + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) $(ALL_LDFLAGS) -mwindows -o $@ $< git.res + git-%$X: %.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) @@ -3651,6 +3658,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) po/git.pot po/git-core.pot $(RM) git.res $(RM) $(OBJECTS) + $(RM) headless-git.o $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(REFTABLE_TEST_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) $(RM) $(TEST_PROGRAMS) @@ -3679,6 +3687,7 @@ endif $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS ifdef MSVC $(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS)) + $(RM) headless-git.o.pdb $(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS)) $(RM) $(patsubst %.exe,%.iobj,$(OTHER_PROGRAMS)) $(RM) $(patsubst %.exe,%.ipdb,$(OTHER_PROGRAMS)) diff --git a/compat/win32/headless.c b/compat/win32/headless.c new file mode 100644 index 00000000000000..8b00dfe3bd5d00 --- /dev/null +++ b/compat/win32/headless.c @@ -0,0 +1,115 @@ +/* + * headless Git - run Git without opening a console window on Windows + */ + +#define STRICT +#define WIN32_LEAN_AND_MEAN +#define UNICODE +#define _UNICODE +#include +#include +#include +#include + +/* + * If `dir` contains the path to a Git exec directory, extend `PATH` to + * include the corresponding `bin/` directory (which is where all those + * `.dll` files needed by `git.exe` are, on Windows). + */ +static int extend_path(wchar_t *dir, size_t dir_len) +{ + const wchar_t *suffix = L"\\libexec\\git-core"; + size_t suffix_len = wcslen(suffix); + wchar_t *env; + DWORD len; + + if (dir_len < suffix_len) + return 0; + + dir_len -= suffix_len; + if (memcmp(dir + dir_len, suffix, suffix_len * sizeof(wchar_t))) + return 0; + + len = GetEnvironmentVariableW(L"PATH", NULL, 0); + if (!len) + return 0; + + env = _alloca((dir_len + 5 + len) * sizeof(wchar_t)); + wcsncpy(env, dir, dir_len); + wcscpy(env + dir_len, L"\\bin;"); + if (!GetEnvironmentVariableW(L"PATH", env + dir_len + 5, len)) + return 0; + + SetEnvironmentVariableW(L"PATH", env); + return 1; +} + +int WINAPI wWinMain(_In_ HINSTANCE instance, + _In_opt_ HINSTANCE previous_instance, + _In_ LPWSTR command_line, _In_ int show) +{ + wchar_t git_command_line[32768]; + size_t size = sizeof(git_command_line) / sizeof(wchar_t); + const wchar_t *needs_quotes = L""; + int slash = 0, i; + + STARTUPINFO startup_info = { + .cb = sizeof(STARTUPINFO), + .dwFlags = STARTF_USESHOWWINDOW, + .wShowWindow = SW_HIDE, + }; + PROCESS_INFORMATION process_info = { 0 }; + DWORD creation_flags = CREATE_UNICODE_ENVIRONMENT | + CREATE_NEW_CONSOLE | CREATE_NO_WINDOW; + DWORD exit_code; + + /* First, determine the full path of argv[0] */ + for (i = 0; _wpgmptr[i]; i++) + if (_wpgmptr[i] == L' ') + needs_quotes = L"\""; + else if (_wpgmptr[i] == L'\\') + slash = i; + + if (slash >= size - 11) + return 127; /* Too long path */ + + /* If it is in Git's exec path, add the bin/ directory to the PATH */ + extend_path(_wpgmptr, slash); + + /* Then, add the full path of `git.exe` as argv[0] */ + i = swprintf_s(git_command_line, size, L"%ls%.*ls\\git.exe%ls", + needs_quotes, slash, _wpgmptr, needs_quotes); + if (i < 0) + return 127; /* Too long path */ + + if (*command_line) { + /* Now, append the command-line arguments */ + i = swprintf_s(git_command_line + i, size - i, + L" %ls", command_line); + if (i < 0) + return 127; + } + + startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE); + startup_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + + if (!CreateProcess(NULL, /* infer argv[0] from the command line */ + git_command_line, /* modified command line */ + NULL, /* inherit process handles? */ + NULL, /* inherit thread handles? */ + FALSE, /* handles inheritable? */ + creation_flags, + NULL, /* use this process' environment */ + NULL, /* use this process' working directory */ + &startup_info, &process_info)) + return 129; /* could not start */ + WaitForSingleObject(process_info.hProcess, INFINITE); + if (!GetExitCodeProcess(process_info.hProcess, &exit_code)) + exit_code = 130; /* Could not determine exit code? */ + + CloseHandle(process_info.hProcess); + CloseHandle(process_info.hThread); + + return (int)exit_code; +} diff --git a/config.mak.uname b/config.mak.uname index 64c44db8058e0c..3bb03f423a0810 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -526,6 +526,8 @@ else endif X = .exe + EXTRA_PROGRAMS += headless-git$X + compat/msvc.o: compat/msvc.c compat/mingw.c GIT-CFLAGS endif ifeq ($(uname_S),Interix) @@ -705,6 +707,7 @@ ifeq ($(uname_S),MINGW) COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \ -fstack-protector-strong EXTLIBS += -lntdll + EXTRA_PROGRAMS += headless-git$X INSTALL = /bin/install INTERNAL_QSORT = YesPlease HAVE_LIBCHARSET_H = YesPlease diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 2f6e0197ffa489..be177998d6044f 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -738,6 +738,15 @@ if(WIN32) else() message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}") endif() + + add_executable(headless-git ${CMAKE_SOURCE_DIR}/compat/win32/headless.c) + if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID STREQUAL "Clang") + target_link_options(headless-git PUBLIC -municode -Wl,-subsystem,windows) + elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC") + target_link_options(headless-git PUBLIC /NOLOGO /ENTRY:wWinMainCRTStartup /SUBSYSTEM:WINDOWS) + else() + message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}") + endif() elseif(UNIX) target_link_libraries(common-main pthread rt) endif() diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm index 1a25789d28513b..b2e68a16715e39 100644 --- a/contrib/buildsystems/Generators/Vcxproj.pm +++ b/contrib/buildsystems/Generators/Vcxproj.pm @@ -76,7 +76,7 @@ sub createProject { my $libs_release = "\n "; my $libs_debug = "\n "; - if (!$static_library) { + if (!$static_library && $name ne 'headless-git') { $libs_release = join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib\.lib|vcs-svn\/lib\.lib|reftable\/libreftable\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}})); $libs_debug = $libs_release; $libs_debug =~ s/zlib\.lib/zlibd\.lib/g; @@ -230,7 +230,7 @@ EOM print F << "EOM"; EOM - if (!$static_library || $target =~ 'vcs-svn' || $target =~ 'xdiff') { + if ((!$static_library || $target =~ 'vcs-svn' || $target =~ 'xdiff') && !($name =~ /headless-git/)) { my $uuid_libgit = $$build_structure{"LIBS_libgit_GUID"}; my $uuid_libreftable = $$build_structure{"LIBS_reftable/libreftable_GUID"}; my $uuid_xdiff_lib = $$build_structure{"LIBS_xdiff/lib_GUID"}; diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl index ed6c45988a38b0..069be7e4befcd7 100755 --- a/contrib/buildsystems/engine.pl +++ b/contrib/buildsystems/engine.pl @@ -371,6 +371,7 @@ sub handleLinkLine # exit(1); foreach (@objfiles) { my $sourcefile = $_; + $sourcefile =~ s/^headless-git\.o$/compat\/win32\/headless.c/; $sourcefile =~ s/\.o$/.c/; push(@sources, $sourcefile); push(@cflags, @{$compile_options{"${sourcefile}_CFLAGS"}}); From 0050f8e401a70b1eda9e7358b04e6d6ef3d64ab1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Aug 2023 16:54:47 +0000 Subject: [PATCH 30/34] git maintenance: avoid console window in scheduled tasks on Windows We just introduced a helper to avoid showing a console window when the scheduled task runs `git.exe`. Let's actually use it. Signed-off-by: Johannes Schindelin Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index f3942188a614c9..fa6e52ddde109e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -2068,7 +2068,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority "\n" "\n" "\n" - "\"%s\\git.exe\"\n" + "\"%s\\headless-git.exe\"\n" "--exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\n" "\n" "\n" From fac96dfbb1c24369ba7d37a5affd8adfe6c650fd Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 9 Aug 2023 16:17:27 -0700 Subject: [PATCH 31/34] Git 2.42-rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.42.0.txt | 20 ++++++++++++++++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.42.0.txt b/Documentation/RelNotes/2.42.0.txt index f2262f84bf998f..62665697ff5eca 100644 --- a/Documentation/RelNotes/2.42.0.txt +++ b/Documentation/RelNotes/2.42.0.txt @@ -258,6 +258,25 @@ Fixes since v2.41 submodule..update configuration variable. (merge 7cebc5bd78 pv/doc-submodule-update-settings later to maint). + * Adjust to OpenSSL 3+, which deprecates its SHA-1 functions based on + its traditional API, by using its EVP API instead. + (merge bda9c12073 ew/hash-with-openssl-evp later to maint). + + * Exclude "." from the set of characters to be removed from the + beginning and the end of the human-readable name. + (merge 1c04cb0744 bc/ident-dot-is-no-longer-crud-letter later to maint). + + * "git bisect visualize" stopped running "gitk" on Git for Windows + when the command was reimplemented in C around Git 2.34 timeframe. + This has been corrected. + (merge fff1594fa7 ma/locate-in-path-for-windows later to maint). + + * "git rebase -i" with a series of squash/fixup, when one of the + steps stopped in conflicts and ended up getting skipped, did not + handle the accumulated commit log messages, which has been + corrected. + (merge 6ce7afe163 pw/rebase-skip-commit-message-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 51f9d2e563 sa/doc-ls-remote later to maint). (merge c6d26a9dda jk/format-patch-message-id-unleak later to maint). @@ -294,3 +313,4 @@ Fixes since v2.41 (merge 30c8c55cbf jc/tree-walk-drop-base-offset later to maint). (merge d089a06421 rs/bundle-parseopt-cleanup later to maint). (merge 823839bda1 ew/sha256-gcrypt-leak-fixes later to maint). + (merge a5c01603b3 bc/ignore-clangd-cache later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 7313f8aa3ec8c4..a1afa6a2bd84f6 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.42.0-rc0 +DEF_VER=v2.42.0-rc1 LF=' ' From 231e86c10c674235cf28447a8486f7955d5f4dd9 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 10 Aug 2023 14:33:13 +0000 Subject: [PATCH 32/34] t4053: avoid race when killing background processes The test 'diff --no-index reads from pipes' starts a couple of background processes that write to the pipes that are passed to "diff --no-index". If the test passes then we expect these processes to exit as all their output will have been read. However if the test fails then we want to make sure they do not hang about on the users machine and the test remembers they should be killed by calling test_when_finished "! kill $!" after each background process is created. Unfortunately there is a race where test_when_finished may run before the background process exits even when all its output has been read resulting in the kill command succeeding which causes the test to fail. Fix this by ignoring the exit status of the kill command. If the diff is successful we could instead wait for the background process to exit and check their status but that feels like it is testing the platform's printf implementation rather than git's code. Reported-by: Jeff King Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- t/t4053-diff-no-index.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index a28b9ff243404f..1fb7d334620767 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -248,11 +248,11 @@ test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' ' { (test_write_lines a b c >old) & } && - test_when_finished "! kill $!" && + test_when_finished "kill $! || :" && { (test_write_lines a x c >new) & } && - test_when_finished "! kill $!" && + test_when_finished "kill $! || :" && cat >expect <<-EOF && diff --git a/old b/new-link From e5cb1e3f093986815945b85e61de112b8e3ccdd6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 13 Aug 2023 12:24:40 -0400 Subject: [PATCH 33/34] t4053: avoid writing to unopened pipe This fixes an occasional hang I see when running t4053 with --verbose-log using dash. Commit 1e3f26542a (diff --no-index: support reading from named pipes, 2023-07-05) added a test that "diff --no-index" will complain when comparing a named pipe and a directory. The minimum we need to test this is to mkfifo the pipe, and then run "git diff --no-index pipe some_dir". But the test does one thing more: it spawns a background shell process that opens the pipe for writing, like this: { (>pipe) & } && This extra writer _could_ be useful if Git misbehaves and tries to open the pipe for reading. Without the writer, Git would block indefinitely and the test would never end. But since we do not have such a bug, Git does not open the pipe and it is the writing process which will block indefinitely, since there are no readers. The test addresses this by running "kill $!" in a test_when_finished block. Since the writer should be blocking forever, this kill command will reliably find it waiting. However, this seems to be somewhat racy, in that the writing process sometimes hangs around even after the "kill". In a normal run of the test script without options, this doesn't have any effect; the main test script completes anyway. But with --verbose-log, we spawn a "tee" process that reads the script output, and it won't end until all descriptors pointing to its input pipe are closed. And the background process that is hanging around still has its stderr, etc, pointed into that pipe. You can reproduce the situation like this: cd t ./t4053-diff-no-index.sh --verbose-log --stress Let that run for a few minutes, and then you'll find that some of the runs have hung. For example, at 11:53, I ran: $ ps xk start o pid,start,command | grep tee | head 713459 11:48:06 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-9.out 713527 11:48:06 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-15.out 719434 11:48:07 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-1.out 728117 11:48:08 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-5.out 738738 11:48:09 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-31.out 739457 11:48:09 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-27.out 744432 11:48:10 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-21.out 744471 11:48:10 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-29.out 761961 11:48:12 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-0.out 812299 11:48:19 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-8.out All of these have been hung for several minutes. We can investigate one and see that it's waiting to get EOF on its input: $ strace -p 713459 strace: Process 713459 attached read(0, ^C Who else has that descriptor open? $ lsof -a -p 713459 -d 0 +E COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME tee 713459 peff 0r FIFO 0,13 0t0 3943636 pipe 719203,sh,5w 719203,sh,7w 719203,sh,12w 719203,sh,13w sh 719203 peff 5w FIFO 0,13 0t0 3943636 pipe 713459,tee,0r 719203,sh,7w 719203,sh,12w 719203,sh,13w sh 719203 peff 7w FIFO 0,13 0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,12w 719203,sh,13w sh 719203 peff 12w FIFO 0,13 0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,7w 719203,sh,13w sh 719203 peff 13w FIFO 0,13 0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,7w 719203,sh,12w It's a shell, presumably a subshell spawned by the main script. Though it may seem odd, having the same descriptor open several times is not unreasonable (they're all basically the original stdout/stderr of the script that has been copied). And they should all close when the process exits. So what's it doing? Curiously, it will exit as soon as we strace it: $ strace -s 64 -p 719203 strace: Process 719203 attached openat(AT_FDCWD, "pipe", O_WRONLY|O_CREAT|O_TRUNC, 0666) = -1 ENOENT (No such file or directory) write(2, "./t4053-diff-no-index.sh: 7: eval: ", 35) = 35 write(2, "cannot create pipe: Directory nonexistent", 41) = 41 write(2, "\n", 1) = 1 exit_group(2) = ? +++ exited with 2 +++ I think what happens is this: - it is blocking in the openat() call for the pipe, as we expect (so this is definitely the backgrounded subshell mentioned above) - strace sends signals (probably STOP/CONT); those cause the kernel to stop blocking, but libc will restart the system call automatically - by this time, the "pipe" fifo is gone, so we'll actually try to create a regular file. But of course the surrounding directory is gone, too! So we get ENOENT, and then exit as normal. So the blocking is something we expect to happen. But what we didn't expect is for the process to still exist at all! It should have been killed earlier when the parent process called "kill", but it wasn't. And we can't catch the race at this point, because it happened much earlier. One can guess, though, that there is some race with the shell setting up the signal handling in the backgrounded subshell, and possibly blocking or ignoring signals at the time that the "kill" is received. Curiously, the race does not seem to happen if I use "bash" instead of "dash", so presumably bash's setup here is more atomic. One fix might be to try killing the subshell more aggressively, either using SIGKILL, or looping on kill/wait. But that seems complex and likely to introduce new problems/races. Instead, we can observe that the writer is not needed at all. Git will notice the pipe via stat() before it is ever opened. So we can simply drop the writer subshell entirely. If we ever changed Git to open the path and fstat() it, this would result in the test hanging. But we're not likely to do that. After all, we have to stat() paths to see if they are openable at all (e.g., it could be a directory), so this seems like a low risk. And anybody who does make such a change will immediately see the issue, as Git would hang consistently. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4053-diff-no-index.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 1fb7d334620767..6781cc90786e3e 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -232,10 +232,6 @@ test_expect_success 'diff --no-index refuses to diff stdin and a directory' ' test_expect_success PIPE 'diff --no-index refuses to diff a named pipe and a directory' ' test_when_finished "rm -f pipe" && mkfifo pipe && - { - (>pipe) & - } && - test_when_finished "kill $!" && test_must_fail git diff --no-index -- pipe a 2>err && grep "fatal: cannot compare a named pipe to a directory" err ' From f1ed9d7dc0e49dc1a044941d821c9d2342313c26 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 15 Aug 2023 10:20:02 -0700 Subject: [PATCH 34/34] Git 2.42-rc2 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.42.0.txt | 13 +++++++++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.42.0.txt b/Documentation/RelNotes/2.42.0.txt index 62665697ff5eca..8ae2d7d5d02f24 100644 --- a/Documentation/RelNotes/2.42.0.txt +++ b/Documentation/RelNotes/2.42.0.txt @@ -277,6 +277,17 @@ Fixes since v2.41 corrected. (merge 6ce7afe163 pw/rebase-skip-commit-message-fix later to maint). + * Adjust to newer Term::ReadLine to prevent it from breaking + the interactive prompt code in send-email. + (merge c016726c2d jk/send-email-with-new-readline later to maint). + + * Windows updates. + (merge 0050f8e401 ds/maintenance-on-windows-fix later to maint). + + * Correct use of lstat() that assumed a failing call would not + clobber the statbuf. + (merge 72695d8214 st/mv-lstat-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 51f9d2e563 sa/doc-ls-remote later to maint). (merge c6d26a9dda jk/format-patch-message-id-unleak later to maint). @@ -314,3 +325,5 @@ Fixes since v2.41 (merge d089a06421 rs/bundle-parseopt-cleanup later to maint). (merge 823839bda1 ew/sha256-gcrypt-leak-fixes later to maint). (merge a5c01603b3 bc/ignore-clangd-cache later to maint). + (merge 12009a182b js/allow-t4000-to-be-indented-with-spaces later to maint). + (merge b3dcd24b8a jc/send-email-pre-process-fix later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index a1afa6a2bd84f6..bbfb1418e363da 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.42.0-rc1 +DEF_VER=v2.42.0-rc2 LF=' '