threads / rfc / 47707

RFC patch, 2 partsalternate hash test

Subject: [RFC PATCH 0/2] alternate hash test

## tl;dr

9 messages between Jan 28, 2018 and Feb 1, 2018. Diffs are folded; open one to read it.

replies: 8people: 5as markdown or json

brian m. carlson· Jan 28, 2018, 17:06 UTC · lore

This series wires up an alternate hash implementation, namely BLAKE2b-160. The goal is to allow us to identify tests which rely on the hash algorithm in use so that we can fix those tests.

For this test, I picked BLAKE2b-160 for a couple reasons:
* Debian ships a libb2-1 package which can be used easily (in other
  words, I was lazy and didn't want to add a crypto implementation just
  for test purposes);
* The API of the libb2 package easily allows arbitrary hash lengths, so
  I didn't have to manage truncation myself;
* Our codebase isn't yet ready for a hash function larger than 20 bytes,
  as there's still more work to do on the object_id conversions.

This isn't an endorsement for or against any particular algorithm choice, just an artifact of the tools that were easily available to me. Provoking discussion of which hash to pick for NewHash is explicitly *not* a goal for this series. I'm only interested in the ability to identify and fix tests.

The first patch does no feature detection and just assumes you have libb2 installed. For obvious reasons, this series is not meant for production use.

This series applies on top of the object_id part 11 series I just sent out.

Below is a list of the prove output indicating which tests failed on my system with these patches.

Test Summary Report
-------------------
t0002-gitfile.sh                                 (Wstat: 256 Tests: 14 Failed: 3)
  Failed tests:  12-14
  Non-zero exit status: 1
t0005-signals.sh                                 (Wstat: 256 Tests: 5 Failed: 2)
  Failed tests:  4-5
  Non-zero exit status: 1
t0000-basic.sh                                   (Wstat: 256 Tests: 78 Failed: 12)
  Failed tests:  47, 54, 56, 58, 60, 62, 64, 66, 71, 74-76
  Non-zero exit status: 1
t1007-hash-object.sh                             (Wstat: 256 Tests: 38 Failed: 15)
  Failed tests:  6, 8, 10-11, 19-29
  Non-zero exit status: 1
t1011-read-tree-sparse-checkout.sh               (Wstat: 256 Tests: 21 Failed: 3)
  Failed tests:  1-2, 5
  Non-zero exit status: 1
t1300-repo-config.sh                             (Wstat: 256 Tests: 146 Failed: 1)
  Failed test:  144
  Non-zero exit status: 1
t1304-default-acl.sh                             (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
t1405-main-ref-store.sh                          (Wstat: 256 Tests: 17 Failed: 1)
  Failed test:  10
  Non-zero exit status: 1
t1411-reflog-show.sh                             (Wstat: 256 Tests: 18 Failed: 6)
  Failed tests:  3-5, 7, 10, 13
  Non-zero exit status: 1
t1450-fsck.sh                                    (Wstat: 256 Tests: 71 Failed: 3)
  Failed tests:  18, 26, 59
  Non-zero exit status: 1
t1507-rev-parse-upstream.sh                      (Wstat: 256 Tests: 28 Failed: 2)
  Failed tests:  25-26
  Non-zero exit status: 1
t1512-rev-parse-disambiguation.sh                (Wstat: 256 Tests: 33 Failed: 19)
  Failed tests:  2-5, 7-12, 16, 22, 26-32
  Non-zero exit status: 1
t1700-split-index.sh                             (Wstat: 256 Tests: 22 Failed: 9)
  Failed tests:  1-2, 5-7, 13-16
  Non-zero exit status: 1
t2011-checkout-invalid-head.sh                   (Wstat: 256 Tests: 10 Failed: 5)
  Failed tests:  3, 6-7, 9-10
  Non-zero exit status: 1
t2015-checkout-unborn.sh                         (Wstat: 256 Tests: 6 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 1
t1013-read-tree-submodule.sh                     (Wstat: 256 Tests: 64 Failed: 15)
  Failed tests:  1-2, 4-7, 19-20, 22-25, 30, 34-35
  Non-zero exit status: 1
t2017-checkout-orphan.sh                         (Wstat: 256 Tests: 13 Failed: 7)
  Failed tests:  7-13
  Non-zero exit status: 1
t2020-checkout-detach.sh                         (Wstat: 256 Tests: 24 Failed: 2)
  Failed tests:  23-24
  Non-zero exit status: 1
t2101-update-index-reupdate.sh                   (Wstat: 256 Tests: 7 Failed: 6)
  Failed tests:  1-3, 5-7
  Non-zero exit status: 1
t2107-update-index-basic.sh                      (Wstat: 256 Tests: 9 Failed: 1)
  Failed test:  9
  Non-zero exit status: 1
t2203-add-intent.sh                              (Wstat: 256 Tests: 16 Failed: 4)
  Failed tests:  3, 12, 15-16
  Non-zero exit status: 1
t3033-merge-toplevel.sh                          (Wstat: 256 Tests: 13 Failed: 11)
  Failed tests:  3-13
  Non-zero exit status: 1
t3103-ls-tree-misc.sh                            (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t3201-branch-contains.sh                         (Wstat: 256 Tests: 19 Failed: 1)
  Failed test:  18
  Non-zero exit status: 1
t3301-notes.sh                                   (Wstat: 256 Tests: 134 Failed: 50)
  Failed tests:  10, 18-23, 42-44, 46, 55-57, 60, 65-78
                80, 83-84, 86-91, 93-104
  Non-zero exit status: 1
t2013-checkout-submodule.sh                      (Wstat: 256 Tests: 70 Failed: 16)
  Failed tests:  7-8, 10-13, 19, 25-26, 28-31, 36, 40-41
  Non-zero exit status: 1
t3306-notes-prune.sh                             (Wstat: 256 Tests: 12 Failed: 8)
  Failed tests:  1-2, 4-5, 7, 9, 11-12
  Non-zero exit status: 1
t3308-notes-merge.sh                             (Wstat: 256 Tests: 19 Failed: 13)
  Failed tests:  2, 5-6, 10-19
  Non-zero exit status: 1
t3309-notes-merge-auto-resolve.sh                (Wstat: 256 Tests: 31 Failed: 27)
  Failed tests:  3, 5, 7-31
  Non-zero exit status: 1
t3310-notes-merge-manual-resolve.sh              (Wstat: 256 Tests: 22 Failed: 16)
  Failed tests:  2-6, 8-9, 13-21
  Non-zero exit status: 1
t3305-notes-fanout.sh                            (Wstat: 256 Tests: 6 Failed: 1)
  Failed test:  6
  Non-zero exit status: 1
t3402-rebase-merge.sh                            (Wstat: 256 Tests: 12 Failed: 4)
  Failed tests:  8, 10-12
  Non-zero exit status: 1
t3311-notes-merge-fanout.sh                      (Wstat: 256 Tests: 24 Failed: 8)
  Failed tests:  2, 7, 9, 12, 14, 16, 18, 23
  Non-zero exit status: 1
t3404-rebase-interactive.sh                      (Wstat: 256 Tests: 98 Failed: 11)
  Failed tests:  15, 68-71, 89-92, 94, 96
  Non-zero exit status: 1
t3421-rebase-topology-linear.sh                  (Wstat: 256 Tests: 76 Failed: 4)
  Failed tests:  73-76
  Non-zero exit status: 1
t3503-cherry-pick-root.sh                        (Wstat: 256 Tests: 6 Failed: 3)
  Failed tests:  3-5
  Non-zero exit status: 1
t3501-revert-cherry-pick.sh                      (Wstat: 256 Tests: 12 Failed: 2)
  Failed tests:  8-9
  Non-zero exit status: 1
t3506-cherry-pick-ff.sh                          (Wstat: 256 Tests: 11 Failed: 2)
  Failed tests:  10-11
  Non-zero exit status: 1
t3702-add-edit.sh                                (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t3701-add-interactive.sh                         (Wstat: 256 Tests: 46 Failed: 9)
  Failed tests:  2, 4-5, 9, 21, 29, 31, 33, 37
  Non-zero exit status: 1
t3905-stash-include-untracked.sh                 (Wstat: 256 Tests: 18 Failed: 2)
  Failed tests:  3, 8
  Non-zero exit status: 1
t4002-diff-basic.sh                              (Wstat: 256 Tests: 63 Failed: 14)
  Failed tests:  45-58
  Non-zero exit status: 1
t4007-rename-3.sh                                (Wstat: 256 Tests: 8 Failed: 5)
  Failed tests:  3-5, 7-8
  Non-zero exit status: 1
t4008-diff-break-rewrite.sh                      (Wstat: 256 Tests: 14 Failed: 9)
  Failed tests:  3-4, 6-7, 9-11, 13-14
  Non-zero exit status: 1
t4010-diff-pathspec.sh                           (Wstat: 256 Tests: 17 Failed: 6)
  Failed tests:  3-6, 9-10
  Non-zero exit status: 1
t4011-diff-symlink.sh                            (Wstat: 256 Tests: 8 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
t4013-diff-various.sh                            (Wstat: 256 Tests: 172 Failed: 131)
  Failed tests:  6-13, 15, 17, 19, 21, 26-30, 32, 34, 36
                38, 40, 42-46, 51-79, 81-134, 136-139, 142-148
                153-154, 158-162, 169-171
  Non-zero exit status: 1
t4014-format-patch.sh                            (Wstat: 256 Tests: 142 Failed: 2)
  Failed tests:  53-54
  Non-zero exit status: 1
t4015-diff-whitespace.sh                         (Wstat: 256 Tests: 77 Failed: 11)
  Failed tests:  1-2, 53, 57-58, 60-65
  Non-zero exit status: 1
t4020-diff-external.sh                           (Wstat: 256 Tests: 25 Failed: 2)
  Failed tests:  18, 23
  Non-zero exit status: 1
t4022-diff-rewrite.sh                            (Wstat: 256 Tests: 11 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 1
t4029-diff-trailing-space.sh                     (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
t4030-diff-textconv.sh                           (Wstat: 256 Tests: 19 Failed: 1)
  Failed test:  19
  Non-zero exit status: 1
t4033-diff-patience.sh                           (Wstat: 256 Tests: 4 Failed: 2)
  Failed tests:  2, 4
  Non-zero exit status: 1
t4034-diff-words.sh                              (Wstat: 256 Tests: 41 Failed: 33)
  Failed tests:  3-8, 10-11, 13-15, 17-20, 22-39
  Non-zero exit status: 1
t4039-diff-assume-unchanged.sh                   (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t4042-diff-textconv-caching.sh                   (Wstat: 256 Tests: 7 Failed: 5)
  Failed tests:  2-6
  Non-zero exit status: 1
t4044-diff-index-unique-abbrev.sh                (Wstat: 256 Tests: 2 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 1
t4045-diff-relative.sh                           (Wstat: 256 Tests: 17 Failed: 8)
  Failed tests:  2-5, 14-17
  Non-zero exit status: 1
t4048-diff-combined-binary.sh                    (Wstat: 256 Tests: 14 Failed: 11)
  Failed tests:  2-4, 6-8, 10-14
  Non-zero exit status: 1
t4050-diff-histogram.sh                          (Wstat: 256 Tests: 3 Failed: 2)
  Failed tests:  1, 3
  Non-zero exit status: 1
t4054-diff-bogus-tree.sh                         (Wstat: 256 Tests: 14 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 1
t4060-diff-submodule-option-diff-format.sh       (Wstat: 256 Tests: 47 Failed: 28)
  Failed tests:  2-3, 6-7, 10-14, 16, 22-23, 26-30, 32-34
                36-37, 39-41, 43, 46-47
  Non-zero exit status: 1
t3903-stash.sh                                   (Wstat: 256 Tests: 90 Failed: 18)
  Failed tests:  2, 14, 20-21, 24-29, 32, 35, 38, 46, 48
                57, 62-63
  Non-zero exit status: 1
t4151-am-abort.sh                                (Wstat: 256 Tests: 18 Failed: 7)
  Failed tests:  12-18
  Non-zero exit status: 1
t4202-log.sh                                     (Wstat: 256 Tests: 80 Failed: 11)
  Failed tests:  8, 19-23, 62-63, 78-80
  Non-zero exit status: 1
t4205-log-pretty-formats.sh                      (Wstat: 256 Tests: 57 Failed: 2)
  Failed tests:  51-52
  Non-zero exit status: 1
t4208-log-magic-pathspec.sh                      (Wstat: 256 Tests: 17 Failed: 1)
  Failed test:  9
  Non-zero exit status: 1
t4211-line-log.sh                                (Wstat: 256 Tests: 37 Failed: 17)
  Failed tests:  2-18
  Non-zero exit status: 1
t4300-merge-tree.sh                              (Wstat: 256 Tests: 20 Failed: 12)
  Failed tests:  2, 5, 7, 9-10, 12, 14-15, 17-20
  Non-zero exit status: 1
t5150-request-pull.sh                            (Wstat: 256 Tests: 7 Failed: 6)
  Failed tests:  1, 3-7
  Non-zero exit status: 1
t5300-pack-object.sh                             (Wstat: 256 Tests: 38 Failed: 3)
  Failed tests:  36-38
  Non-zero exit status: 1
t5306-pack-nobase.sh                             (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
t5308-pack-detect-duplicates.sh                  (Wstat: 256 Tests: 5 Failed: 3)
  Failed tests:  1-2, 4
  Non-zero exit status: 1
t5309-pack-delta-cycles.sh                       (Wstat: 256 Tests: 6 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 1
t5313-pack-bounds-checks.sh                      (Wstat: 256 Tests: 8 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
t5504-fetch-receive-strict.sh                    (Wstat: 256 Tests: 12 Failed: 2)
  Failed tests:  10-11
  Non-zero exit status: 1
t5512-ls-remote.sh                               (Wstat: 256 Tests: 26 Failed: 3)
  Failed tests:  21-22, 24
  Non-zero exit status: 1
t5515-fetch-merge-logic.sh                       (Wstat: 256 Tests: 65 Failed: 64)
  Failed tests:  2-65
  Non-zero exit status: 1
t5521-pull-options.sh                            (Wstat: 256 Tests: 18 Failed: 10)
  Failed tests:  2-11
  Non-zero exit status: 1
t5516-fetch-push.sh                              (Wstat: 256 Tests: 89 Failed: 9)
  Failed tests:  34, 78-83, 88-89
  Non-zero exit status: 1
t5520-pull.sh                                    (Wstat: 256 Tests: 64 Failed: 4)
  Failed tests:  2-3, 6, 58
  Non-zero exit status: 1
t6000-rev-list-misc.sh                           (Wstat: 256 Tests: 10 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
t6024-recursive-merge.sh                         (Wstat: 256 Tests: 6 Failed: 3)
  Failed tests:  3-5
  Non-zero exit status: 1
t6300-for-each-ref.sh                            (Wstat: 256 Tests: 205 Failed: 10)
  Failed tests:  83, 189-196, 199
  Non-zero exit status: 1
t6500-gc.sh                                      (Wstat: 256 Tests: 8 Failed: 3)
  Failed tests:  6-8
  Non-zero exit status: 1
t6030-bisect-porcelain.sh                        (Wstat: 256 Tests: 71 Failed: 18)
  Failed tests:  19, 22-27, 30, 32, 37, 41, 45-47, 53-56
  Non-zero exit status: 1
t7102-reset.sh                                   (Wstat: 256 Tests: 30 Failed: 13)
  Failed tests:  11, 13-19, 23-24, 26-28
  Non-zero exit status: 1
t7106-reset-unborn-branch.sh                     (Wstat: 256 Tests: 7 Failed: 4)
  Failed tests:  2, 4-5, 7
  Non-zero exit status: 1
t7003-filter-branch.sh                           (Wstat: 256 Tests: 42 Failed: 2)
  Failed tests:  34-35
  Non-zero exit status: 1
t7201-co.sh                                      (Wstat: 256 Tests: 36 Failed: 33)
  Failed tests:  2, 4-25, 27-36
  Non-zero exit status: 1
t7063-status-untracked-cache.sh                  (Wstat: 256 Tests: 43 Failed: 14)
  Failed tests:  5, 7, 9, 11, 13-14, 16-17, 19, 22, 26, 30
                36, 40
  Non-zero exit status: 1
t7112-reset-submodule.sh                         (Wstat: 256 Tests: 78 Failed: 15)
  Failed tests:  1-2, 4-7, 19-20, 22-25, 30, 34-35
  Non-zero exit status: 1
t7507-commit-verbose.sh                          (Wstat: 256 Tests: 44 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t7506-status-submodule.sh                        (Wstat: 256 Tests: 40 Failed: 2)
  Failed tests:  31-32
  Non-zero exit status: 1
t7600-merge.sh                                   (Wstat: 256 Tests: 56 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
t7607-merge-overwrite.sh                         (Wstat: 256 Tests: 18 Failed: 2)
  Failed tests:  14, 18
  Non-zero exit status: 1
t9300-fast-import.sh                             (Wstat: 256 Tests: 183 Failed: 19)
  Failed tests:  17, 32, 35, 50, 59, 61-66, 68-69, 71-72
                74, 174, 176, 178
  Non-zero exit status: 1
t9500-gitweb-standalone-no-errors.sh             (Wstat: 256 Tests: 118 Failed: 1)
  Failed test:  118
  Non-zero exit status: 1
t9903-bash-prompt.sh                             (Wstat: 256 Tests: 66 Failed: 53)
  Failed tests:  4, 6-10, 14-34, 36, 39-62, 65
  Non-zero exit status: 1

brian m. carlson (2):
  Base test implementation for short BLAKE2b support
  Switch default hash algorithm to short BLAKE2b for testing

 Makefile                 |  3 +++
 cache.h                  | 16 ++++++++++++++
 hash.h                   |  9 +++++++-
 repository.c             |  2 +-
 setup.c                  |  2 +-
 sha1_file.c              | 33 ++++++++++++++++++++++++++++
 t/helper/test-sblake2b.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh            |  4 ++--
 8 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 t/helper/test-sblake2b.c

brian m. carlson· Jan 28, 2018, 17:06 UTC · re: brian m. carlson · lore

[RFC PATCH 2/2] Switch default hash algorithm to short BLAKE2b for testing

Set the default hash algorithm to short (160-bit) BLAKE2b so we can identify tests that rely on a given hash algorithm.

This is a test commit and should not be used in production.  Widespread
test breakage occurs when using this commit.
---
 repository.c  | 2 +-
 setup.c       | 2 +-
 t/test-lib.sh | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

Show changes to 3 files +4 −4

repository.c, setup.c, t/test-lib.sh

diff --git a/repository.c b/repository.c
index f66fcb1342..95c0ff74d6 100644
--- a/repository.c
+++ b/repository.c
@@ -5,7 +5,7 @@
 
 /* The main repository */
 static struct repository the_repo = {
-	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0
+	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SBLAKE2], 0, 0
 };
 struct repository *the_repository = &the_repo;
 
diff --git a/setup.c b/setup.c
index 8cc34186ce..72eaee9c4e 100644
--- a/setup.c
+++ b/setup.c
@@ -488,7 +488,7 @@ int read_repository_format(struct repository_format *format, const char *path)
 	memset(format, 0, sizeof(*format));
 	format->version = -1;
 	format->is_bare = -1;
-	format->hash_algo = GIT_HASH_SHA1;
+	format->hash_algo = GIT_HASH_SBLAKE2;
 	string_list_init(&format->unknown_extensions, 1);
 	git_config_from_file(check_repo_format, path, format);
 	return format->version;
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9a0a21f49a..cd257040ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -183,8 +183,8 @@ _x40="$_x35$_x05"
 # Zero SHA-1
 _z40=0000000000000000000000000000000000000000
 
-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+EMPTY_TREE=f44422a644bfa5212387098f253a1e89eba94548
+EMPTY_BLOB=a706650a477f63b9b00eba41272bf36ef5a7dfa2
 
 # Line feed
 LF='

brian m. carlson· Jan 28, 2018, 17:06 UTC · re: brian m. carlson · lore

[RFC PATCH 1/2] Base test implementation for short BLAKE2b support

Test the implementation of a second hash algorithm with "short" (160-bit) BLAKE2b support. Simply add an additional algorithm to the codebase without changing the use of any hash function and add a test helper.

BLAKE2b was chosen simply because it provides a readily accessible hash algorithm with arbitrary length support. A 160-bit (20-byte) hash was chosen to allow identification of tests that depend on the hash algorithm in use while not causing buffer overflows, since parts of the codebase still assume a 20-byte hash.

This is a preliminary commit for test purposes only and should not be
used in production in any way.
---
 Makefile                 |  3 +++
 cache.h                  | 16 ++++++++++++++
 hash.h                   |  9 +++++++-
 sha1_file.c              | 33 ++++++++++++++++++++++++++++
 t/helper/test-sblake2b.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-sblake2b.c

Show changes to 5 files +116 −1

Makefile, cache.h, hash.h, sha1_file.c, t/helper/test-sblake2b.c

diff --git a/Makefile b/Makefile
index 1a9b23b679..d46247586b 100644
--- a/Makefile
+++ b/Makefile
@@ -677,6 +677,7 @@ TEST_PROGRAMS_NEED_X += test-ref-store
 TEST_PROGRAMS_NEED_X += test-regex
 TEST_PROGRAMS_NEED_X += test-revision-walking
 TEST_PROGRAMS_NEED_X += test-run-command
+TEST_PROGRAMS_NEED_X += test-sblake2b
 TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sha1-array
@@ -1539,6 +1540,8 @@ endif
 endif
 endif
 
+EXTLIBS += -lb2
+
 ifdef SHA1_MAX_BLOCK_SIZE
 	LIB_OBJS += compat/sha1-chunked.o
 	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
diff --git a/cache.h b/cache.h
index bfde6f757a..638d832350 100644
--- a/cache.h
+++ b/cache.h
@@ -45,6 +45,10 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);
 #define GIT_SHA1_RAWSZ 20
 #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
 
+/* The length in bytes and in hex digits of an object name (short BLAKE2b value). */
+#define GIT_SBLAKE2B_RAWSZ 20
+#define GIT_SBLAKE2B_HEXSZ (2 * GIT_SBLAKE2B_RAWSZ)
+
 /* The length in byte and in hex digits of the largest possible hash value. */
 #define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
 #define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
@@ -1013,7 +1017,13 @@ static inline void oidclr(struct object_id *oid)
 #define EMPTY_TREE_SHA1_BIN_LITERAL \
 	 "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
 	 "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
+#define EMPTY_TREE_SBLAKE2B_HEX \
+	"f44422a644bfa5212387098f253a1e89eba94548"
+#define EMPTY_TREE_SBLAKE2B_BIN_LITERAL \
+	"\xf4\x44\x22\xa6\x44\xbf\xa5\x21\x23\x87" \
+	"\x09\x8f\x25\x3a\x1e\x89\xeb\xa9\x45\x48"
 extern const struct object_id empty_tree_oid;
+const struct object_id empty_tree_oid_sblake2b;
 #define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
 
 #define EMPTY_BLOB_SHA1_HEX \
@@ -1021,7 +1031,13 @@ extern const struct object_id empty_tree_oid;
 #define EMPTY_BLOB_SHA1_BIN_LITERAL \
 	"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
 	"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
+#define EMPTY_BLOB_SBLAKE2B_HEX \
+	"a706650a477f63b9b00eba41272bf36ef5a7dfa2"
+#define EMPTY_BLOB_SBLAKE2B_BIN_LITERAL \
+	"\xa7\x06\x65\x0a\x47\x7f\x63\xb9\xb0\x0e" \
+	"\xba\x41\x27\x2b\xf3\x6e\xf5\xa7\xdf\xa2"
 extern const struct object_id empty_blob_oid;
+const struct object_id empty_blob_oid_sblake2b;
 #define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
 
 
diff --git a/hash.h b/hash.h
index 365846a6b5..ef510bd51a 100644
--- a/hash.h
+++ b/hash.h
@@ -15,6 +15,8 @@
 #include "block-sha1/sha1.h"
 #endif
 
+#include <blake2.h>
+
 #ifndef platform_SHA_CTX
 /*
  * platform's underlying implementation of SHA-1; could be OpenSSL,
@@ -40,6 +42,8 @@
 #define git_SHA1_Update		git_SHA1_Update_Chunked
 #endif
 
+#define git_BLAKE2B_CTX		blake2b_state
+
 /*
  * Note that these constants are suitable for indexing the hash_algos array and
  * comparing against each other, but are otherwise arbitrary, so they should not
@@ -52,12 +56,15 @@
 #define GIT_HASH_UNKNOWN 0
 /* SHA-1 */
 #define GIT_HASH_SHA1 1
+/* 20-byte BLAKE2b ("short" BLAKE2b) */
+#define GIT_HASH_SBLAKE2 2
 /* Number of algorithms supported (including unknown). */
-#define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)
+#define GIT_HASH_NALGOS (GIT_HASH_SBLAKE2 + 1)
 
 /* A suitably aligned type for stack allocations of hash contexts. */
 union git_hash_ctx {
 	git_SHA_CTX sha1;
+	git_BLAKE2B_CTX blake2b;
 };
 typedef union git_hash_ctx git_hash_ctx;
 
diff --git a/sha1_file.c b/sha1_file.c
index d9e2b1f285..5067b22a30 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -38,6 +38,12 @@ const struct object_id empty_tree_oid = {
 const struct object_id empty_blob_oid = {
 	EMPTY_BLOB_SHA1_BIN_LITERAL
 };
+const struct object_id empty_tree_oid_sblake2b = {
+	EMPTY_TREE_SBLAKE2B_BIN_LITERAL
+};
+const struct object_id empty_blob_oid_sblake2b = {
+	EMPTY_BLOB_SBLAKE2B_BIN_LITERAL
+};
 
 static void git_hash_sha1_init(void *ctx)
 {
@@ -54,6 +60,21 @@ static void git_hash_sha1_final(unsigned char *hash, void *ctx)
 	git_SHA1_Final(hash, (git_SHA_CTX *)ctx);
 }
 
+static void git_hash_sblake2b_init(void *ctx)
+{
+	blake2b_init((git_BLAKE2B_CTX *)ctx, GIT_SBLAKE2B_RAWSZ);
+}
+
+static void git_hash_sblake2b_update(void *ctx, const void *data, size_t len)
+{
+	blake2b_update((git_BLAKE2B_CTX *)ctx, data, len);
+}
+
+static void git_hash_sblake2b_final(unsigned char *hash, void *ctx)
+{
+	blake2b_final((git_BLAKE2B_CTX *)ctx, hash, GIT_SBLAKE2B_RAWSZ);
+}
+
 static void git_hash_unknown_init(void *ctx)
 {
 	die("trying to init unknown hash");
@@ -93,6 +114,18 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		&empty_tree_oid,
 		&empty_blob_oid,
 	},
+	{
+		"sblake2b",
+		/* "sb2b", big-endian */
+		0x73623262,
+		GIT_SBLAKE2B_RAWSZ,
+		GIT_SBLAKE2B_HEXSZ,
+		git_hash_sblake2b_init,
+		git_hash_sblake2b_update,
+		git_hash_sblake2b_final,
+		&empty_tree_oid_sblake2b,
+		&empty_blob_oid_sblake2b,
+	}
 };
 
 /*
diff --git a/t/helper/test-sblake2b.c b/t/helper/test-sblake2b.c
new file mode 100644
index 0000000000..6623aaa4d5
--- /dev/null
+++ b/t/helper/test-sblake2b.c
@@ -0,0 +1,56 @@
+#include "cache.h"
+
+int cmd_main(int ac, const char **av)
+{
+	git_hash_ctx ctx;
+	unsigned char hash[GIT_MAX_RAWSZ];
+	unsigned bufsz = 8192;
+	int binary = 0;
+	char *buffer;
+
+	if (ac == 2) {
+		if (!strcmp(av[1], "-b"))
+			binary = 1;
+		else
+			bufsz = strtoul(av[1], NULL, 10) * 1024 * 1024;
+	}
+
+	if (!bufsz)
+		bufsz = 8192;
+
+	while ((buffer = malloc(bufsz)) == NULL) {
+		fprintf(stderr, "bufsz %u is too big, halving...\n", bufsz);
+		bufsz /= 2;
+		if (bufsz < 1024)
+			die("OOPS");
+	}
+
+	hash_algos[GIT_HASH_SBLAKE2].init_fn(&ctx);
+
+	while (1) {
+		ssize_t sz, this_sz;
+		char *cp = buffer;
+		unsigned room = bufsz;
+		this_sz = 0;
+		while (room) {
+			sz = xread(0, cp, room);
+			if (sz == 0)
+				break;
+			if (sz < 0)
+				die_errno("test-sha1");
+			this_sz += sz;
+			cp += sz;
+			room -= sz;
+		}
+		if (this_sz == 0)
+			break;
+		hash_algos[GIT_HASH_SBLAKE2].update_fn(&ctx, buffer, this_sz);
+	}
+	hash_algos[GIT_HASH_SBLAKE2].final_fn(hash, &ctx);
+
+	if (binary)
+		fwrite(hash, 1, hash_algos[GIT_HASH_SBLAKE2].rawsz, stdout);
+	else
+		puts(sha1_to_hex(hash));
+	exit(0);
+}

SZEDER Gábor· Jan 28, 2018, 18:32 UTC · re: brian m. carlson · lore

Re: [RFC PATCH 0/2] alternate hash test

> This series wires up an alternate hash implementation, namely
> BLAKE2b-160.  The goal is to allow us to identify tests which rely on
> the hash algorithm in use so that we can fix those tests.

> t9903-bash-prompt.sh                             (Wstat: 256 Tests: 66 Failed: 53)
>   Failed tests:  4, 6-10, 14-34, 36, 39-62, 65
>   Non-zero exit status: 1

I didn't recall seeing hardcoded SHA-1s in the bash prompt tests, so went to have a look. And indeed, these failures are not the test's fault, but the result of a segfault in 'git checkout' while trying to return from an orphan branch in test 4. The rest of the failures are mostly fallout caused by the wrong branch being checked out or the leftover index.lock.

$ ./t9903-bash-prompt.sh -v -i
<...>
expecting success: 
      printf " (unborn)" >expected &&
      git checkout --orphan unborn &&
      test_when_finished "git checkout master" &&
      __git_ps1 >"$actual" &&
      test_cmp expected "$actual"

Switched to a new branch 'unborn' ./test-lib.sh: line 609: 29342 Segmentation fault git checkout master not ok 4 - prompt - unborn branch

$ gdb --args ../../git checkout master
<...>
Program received signal SIGSEGV, Segmentation fault.
0x000000000041b636 in merge_working_tree (opts=0x7fffffffd200, 
    old=0x7fffffffd0d0, new=0x7fffffffd1e0,
writeout_error=0x7fffffffd0c0)
    at builtin/checkout.c:525
525                     init_tree_desc(&trees[0], tree->buffer,
tree->size);
(gdb) p tree 
$1 = (struct tree *) 0x0

The root cause is that patch 2/2 misses places where EMPTY_TREE_SHA1_* constants should have been replaced with their EMPTY_TREE_SBLAKE2B_* counterparts.

Ævar Arnfjörð Bjarmason· Jan 28, 2018, 18:58 UTC · re: brian m. carlson · lore

Re: [RFC PATCH 0/2] alternate hash test

On Sun, Jan 28, 2018 at 6:06 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 10 quoted lines

> For this test, I picked BLAKE2b-160 for a couple reasons:
> * Debian ships a libb2-1 package which can be used easily (in other
>   words, I was lazy and didn't want to add a crypto implementation just
>   for test purposes);
> [...]
> This isn't an endorsement for or against any particular algorithm
> choice, just an artifact of the tools that were easily available to me.
> Provoking discussion of which hash to pick for NewHash is explicitly
> *not* a goal for this series.  I'm only interested in the ability to
> identify and fix tests.

If the goal is to smoke out hardcoded SHA1s in tests, isn't it easier to instrument SHA-1 (e.g. our blk_sha1 copy, or our wrappers) to pretend that whenever we ask for the hash for STRING to pretend we asked for SOME_PREFIX + STRING?

Such an approach would have the advantage of being more portable (easier to run these mock test), and also that if we ever move to NewHash we could still test for this, we'd just always set the prefix to compilation time(), and could thus guarantee that the hashes would change every time git was built.

brian m. carlson· Jan 28, 2018, 19:16 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [RFC PATCH 0/2] alternate hash test

On Sun, Jan 28, 2018 at 07:58:10PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> On Sun, Jan 28, 2018 at 6:06 PM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> If the goal is to smoke out hardcoded SHA1s in tests, isn't it easier
> to instrument SHA-1 (e.g. our blk_sha1 copy, or our wrappers) to
> pretend that whenever we ask for the hash for STRING to pretend we
> asked for SOME_PREFIX + STRING?
> 
> Such an approach would have the advantage of being more portable
> (easier to run these mock test), and also that if we ever move to
> NewHash we could still test for this, we'd just always set the prefix
> to compilation time(), and could thus guarantee that the hashes would
> change every time git was built.

That's certainly a possibility. We could simply call the update function from the init function and prepend a NUL byte or something like that, which would definitely produce different results.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Junio C Hamano· Jan 30, 2018, 21:09 UTC · re: brian m. carlson · lore

Re: [RFC PATCH 0/2] alternate hash test

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> This series wires up an alternate hash implementation, namely
> BLAKE2b-160.  The goal is to allow us to identify tests which rely on
> the hash algorithm in use so that we can fix those tests.

Yay.

> Provoking discussion of which hash to pick for NewHash is explicitly
> *not* a goal for this series.  I'm only interested in the ability to
> identify and fix tests.

Double yay.

Stefan Beller· Jan 31, 2018, 01:04 UTC · re: brian m. carlson · lore

Re: [RFC PATCH 0/2] alternate hash test

On Sun, Jan 28, 2018 at 9:06 AM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 22 quoted lines

> This series wires up an alternate hash implementation, namely
> BLAKE2b-160.  The goal is to allow us to identify tests which rely on
> the hash algorithm in use so that we can fix those tests.
>
> For this test, I picked BLAKE2b-160 for a couple reasons:
> * Debian ships a libb2-1 package which can be used easily (in other
>   words, I was lazy and didn't want to add a crypto implementation just
>   for test purposes);
> * The API of the libb2 package easily allows arbitrary hash lengths, so
>   I didn't have to manage truncation myself;
> * Our codebase isn't yet ready for a hash function larger than 20 bytes,
>   as there's still more work to do on the object_id conversions.
>
> This isn't an endorsement for or against any particular algorithm
> choice, just an artifact of the tools that were easily available to me.
> Provoking discussion of which hash to pick for NewHash is explicitly
> *not* a goal for this series.  I'm only interested in the ability to
> identify and fix tests.
>
> The first patch does no feature detection and just assumes you have
> libb2 installed.  For obvious reasons, this series is not meant for
> production use.

Thanks for writing this, I chose a slightly different approach at https://public-inbox.org/git/20170728171817.21458-2-sbeller@google.com/ which might be quicker for local testing.

Thanks for bringing this discussion back on the list, Stefan

brian m. carlson· Feb 1, 2018, 01:25 UTC · re: Stefan Beller · lore

Re: [RFC PATCH 0/2] alternate hash test

On Tue, Jan 30, 2018 at 05:04:52PM -0800, Stefan Beller wrote:

> Thanks for writing this, I chose a slightly different approach at
> https://public-inbox.org/git/20170728171817.21458-2-sbeller@google.com/
> which might be quicker for local testing.

I'm actually going to rework the patches to be much more similar to that. I've been convinced that's an easier approach to the series, and I like the simplicity.

My goal is not only to identify failing tests, but to exercise the hash algorithm logic so that we find any hidden dependencies outside of that code. For example, I noticed some of our Perl scripts have hard-coded empty-tree object IDs in them, which obviously will need to be fixed.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204