@matheusbsbs / matheus-bagliano / commits / 4a1c95931f

builtin/patch-id: fix uninitialized hash function

In c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07), we have adapted `initialize_repository()` to no longer set up a default hash function. As this function is also used to set up `the_repository`, the consequence is that `the_hash_algo` will now by default be a `NULL` pointer unless the hash algorithm was configured properly. This is done as a mechanism to detect cases where we may be using the wrong hash function by accident. This change now causes git-patch-id(1) to segfault when it's run outside of a repository. As this command can read diffs from stdin, it does not necessarily need a repository, but then relies on `the_hash_algo` to compute the patch ID itself. It is somewhat dubious that git-patch-id(1) relies on `the_hash_algo` in the first place. Quoting its manpage: A "patch ID" is nothing but a sum of SHA-1 of the file diffs associated with a patch, with line numbers ignored. As such, it’s "reasonably stable", but at the same time also reasonably unique, i.e., two patches that have the same "patch ID" are almost guaranteed to be the same thing. We explicitly document patch IDs to be using SHA-1. Furthermore, patch IDs are supposed to be stable for most of the part. But even with the same input, the patch IDs will now be different depending on the repo's configured object hash. Work around the issue by setting up SHA-1 when there was no startup repository for now. This is arguably not the correct fix, but for now we rather want to focus on getting the segfault fixed. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 20, 2024 at 16:14 UTC 4a1c95931f5f0dc541925703b7d9ad9409cd4067

3 files changed +48 -1

builtin/patch-id.c

+13

@@ -5,6 +5,7 @@

5 #include "hash.h"

6 #include "hex.h"

7 #include "parse-options.h"

8 +#include "setup.h"

10 static void flush_current_id(int patchlen, struct object_id *id, struct object_id *result)

11 {

@@ -237,6 +238,18 @@ int cmd_patch_id(int argc, const char **argv, const char *prefix)

238 argc = parse_options(argc, argv, prefix, builtin_patch_id_options,

239 patch_id_usage, 0);

240

241 + /*

242 + * We rely on `the_hash_algo` to compute patch IDs. This is dubious as

243 + * it means that the hash algorithm now depends on the object hash of

244 + * the repository, even though git-patch-id(1) clearly defines that

245 + * patch IDs always use SHA1.

246 + *

247 + * NEEDSWORK: This hack should be removed in favor of converting

248 + * the code that computes patch IDs to always use SHA1.

249 + */

250 + if (!the_hash_algo)

251 + repo_set_hash_algo(the_repository, GIT_HASH_SHA1);

252 +

253 generate_id_list(opts ? opts > 1 : config.stable,

254 opts ? opts == 3 : config.verbatim);

255 return 0;

t/t1517-outside-repo.sh

+1 -1

@@ -21,7 +21,7 @@ test_expect_success 'set up a non-repo directory and test file' '

21 git diff >sample.patch

22 '

24 -test_expect_failure 'compute a patch-id outside repository (uses SHA-1)' '

24 +test_expect_success 'compute a patch-id outside repository (uses SHA-1)' '

25 nongit env GIT_DEFAULT_HASH=sha1 \

26 git patch-id <sample.patch >patch-id.expect &&

27 nongit \

t/t4204-patch-id.sh

+34

@@ -310,4 +310,38 @@ test_expect_success 'patch-id handles diffs with one line of before/after' '

310 test_config patchid.stable true &&

311 calc_patch_id diffu1stable <diffu1

312 '

313 +

314 +test_expect_failure 'patch-id computes same ID with different object hashes' '

315 + test_when_finished "rm -rf repo-sha1 repo-sha256" &&

316 +

317 + cat >diff <<-\EOF &&

318 + diff --git a/bar b/bar

319 + index bdaf90f..31051f6 100644

320 + --- a/bar

321 + +++ b/bar

322 + @@ -2 +2,2 @@

323 + b

324 + +c

325 + EOF

326 +

327 + git init --object-format=sha1 repo-sha1 &&

328 + git -C repo-sha1 patch-id <diff >patch-id-sha1 &&

329 + git init --object-format=sha256 repo-sha256 &&

330 + git -C repo-sha256 patch-id <diff >patch-id-sha256 &&

331 + test_cmp patch-id-sha1 patch-id-sha256

332 +'

333 +

334 +test_expect_success 'patch-id without repository' '

335 + cat >diff <<-\EOF &&

336 + diff --git a/bar b/bar

337 + index bdaf90f..31051f6 100644

338 + --- a/bar

339 + +++ b/bar

340 + @@ -2 +2,2 @@

341 + b

342 + +c

343 + EOF

344 + nongit git patch-id <diff

345 +'

346 +

347 test_done