Patrick Steinhardt <ps@xxxxxx> writes: > with c8aed5e8da (repository: stop setting SHA1 as the default object > hash, 2024-05-07), we have stopped setting up the default hash function > for `the_repository`. This change was done so that we stop implicitly > using SHA1 in places where we don't really intend to. Instead, code > where we try to access `the_hash_algo` without having `the_repository` > properly initialized will now crash hard. > > I have found two more cases where this can now be triggered: > > - git-patch-id(1) can read diffs from stdin. > > - git-hash-object(1) can hash data from stdin. > > Both cases can work without a repository, and if they don't have one > they will now crash. Perhaps we should double-check with all commands that are designed to be able to work outside a repository, e.g. "git apply", "git grep --no-index", "git diff --no-index" (tried to be exhausitive without consulting documentation, so the list is not exhausitive at all). > I still consider it a good thing that we did the change regardless of > those crashes. In the case of git-patch-id(1) I would claim that using > `the_hash_algo` is wrong in the first place, as patch IDs should be > stable and are documented to always use SHA1. Thus, patch IDs in SHA256 > repos are essentially broken. And in the case of git-hash-object(1), we > should expose a command line option to let the user specify the object > hash. So both cases demonstrate that there is room for improvement. It is good that the topic is kept outside 'master' (and it is in 'next' to give the topic a bit wider exposure than merely in 'seen' and the list archive). We may want a test file that explicitly make commands that ought to work outside a repository actually run outside a repository, making use of the GIT_CEILING_DIRECTORIES mechanism, something along the lines of the attached. t/t1517-outside-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git c/t/t1517-outside-repo.sh w/t/t1517-outside-repo.sh new file mode 100755 index 0000000000..4c595c2ff7 --- /dev/null +++ w/t/t1517-outside-repo.sh @@ -0,0 +1,42 @@ +#!/bin/sh + +test_description='check random commands outside repo' + +TEST_PASSES_SANITIZE_LEAK=true +. ./test-lib.sh + +test_expect_success 'set up a non-repo directory and test file' ' + GIT_CEILING_DIRECTORIES=$(pwd) && + export GIT_CEILING_DIRECTORIES && + mkdir non-repo && + ( + cd non-repo && + # confirm that git does not find a repo + test_must_fail git rev-parse --git-dir + ) && + test_write_lines one two three four >nums && + git add nums && + cp nums nums.old && + test_write_lines five >>nums && + git diff >sample.patch +' + +test_expect_success 'apply a patch outside repository' ' + ( + cd non-repo && + cp ../nums.old nums && + git apply ../sample.patch + ) && + test_cmp nums non-repo/nums +' + +test_expect_success 'compute a patch-id outside repository' ' + git patch-id <sample.patch >patch-id.expect && + ( + cd non-repo && + git patch-id <../sample.patch >../patch-id.actual + ) && + test_cmp patch-id.expect patch-id.actual +' + +test_done
