{"thread":{"id":"49566","subject":"[PATCH v2 01/13] sha1-file: rename algorithm to \"sha1\"","startedAt":"2018-10-15T02:19:12Z","lastAt":"2018-10-19T22:21:21Z","messageCount":44,"participants":["brian m. carlson","Duy Nguyen","Derrick Stolee","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":13},"messages":[{"id":"360459","messageId":"20181015021900.1030041-2-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 01/13] sha1-file: rename algorithm to \"sha1\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:48Z","receivedAt":"2018-10-15T02:19:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The transition plan anticipates us using a syntax such as \"^{sha1}\" for\ndisambiguation.  Since this is a syntax some people will be typing a\nlot, it makes sense to provide a short, easy-to-type syntax.  Omitting\nthe dash doesn't create any ambiguity, but it does make it shorter and\neasier to type, especially for touch typists.  In addition, the\ntransition plan already uses \"sha1\" in this context.\n\nRename the name of SHA-1 implementation to \"sha1\".\n\nNote that this change creates no backwards compatibility concerns, since\nwe haven't yet used this field in any serialized data formats.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n sha1-file.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sha1-file.c b/sha1-file.c\nindex a4367b8f04..e29825f259 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -97,7 +97,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\tNULL,\n \t},\n \t{\n-\t\t\"sha-1\",\n+\t\t\"sha1\",\n \t\t/* \"sha1\", big-endian */\n \t\t0x73686131,\n \t\tGIT_SHA1_RAWSZ,\n"},{"id":"360460","messageId":"20181015021900.1030041-1-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":null,"subject":"[PATCH v2 00/13] Base SHA-256 implementation","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:47Z","receivedAt":"2018-10-15T02:19:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series provides an actual SHA-256 implementation and wires it up,\nalong with some housekeeping patches to make it suitable for testing.\n\nNew in this version is a patch which reverts the change to limit hashcmp\nto 20 bytes.  I've taken care to permit the compiler to inline as much\nas possible for efficiency, but it would be helpful to see what the\nperformance impact of these changes is on real-life workflows, or with\nMSVC and other non-GCC and non-clang compilers.  The resulting change is\nuglier and more duplicative than I wanted, but oh, well.\n\nI didn't attempt to pull in the full complement of code from libtomcrypt\nto try to show the changes made, since that would have involved\nimporting a significant quantity of code in order to make things work.\n\nI realize the increase to GIT_MAX_HEXSZ will result in an increase in\nmemory usage, but we're going to need to do it at some point, and the\nsooner the code is in the codebase, the sooner people can play around\nwith it and test it.\n\nThis piece should be the final piece of preparatory work required for a\nfully functioning SHA-256-only Git.  Additional work should be able to\ncome into the testsuite and codebase without needing to build on work in\nany series after this one.\n\nv1, which was RFC, can be seen at\nhttps://public-inbox.org/git/20180829005857.980820-1-sandals@crustytoothpaste.net/.\n\nChanges from v1:\n* Add a hash_to_hex function mirroring sha1_to_hex, but for\n  the_hash_algo.\n* Strip commit message explanation about why we chose SHA-256.\n* Rebase on master\n* Strip leading whitespace from commit message.\n* Improve commit-graph patch to cover new code added since v1.\n* Be more honest about the scope of work involved in porting the SHA-256\n  implementation out of libtomcrypt.\n* Revert change to limit hashcmp to 20 bytes.\n\nbrian m. carlson (13):\n  sha1-file: rename algorithm to \"sha1\"\n  sha1-file: provide functions to look up hash algorithms\n  hex: introduce functions to print arbitrary hashes\n  cache: make hashcmp and hasheq work with larger hashes\n  t: add basic tests for our SHA-1 implementation\n  t: make the sha1 test-tool helper generic\n  sha1-file: add a constant for hash block size\n  t/helper: add a test helper to compute hash speed\n  commit-graph: convert to using the_hash_algo\n  Add a base implementation of SHA-256 support\n  sha256: add an SHA-256 implementation using libgcrypt\n  hash: add an SHA-256 implementation using OpenSSL\n  commit-graph: specify OID version for SHA-256\n\n Makefile                              |  22 ++++\n cache.h                               |  51 +++++---\n commit-graph.c                        |  40 +++---\n hash.h                                |  41 +++++-\n hex.c                                 |  37 ++++--\n sha1-file.c                           |  70 +++++++++-\n sha256/block/sha256.c                 | 180 ++++++++++++++++++++++++++\n sha256/block/sha256.h                 |  26 ++++\n sha256/gcrypt.h                       |  30 +++++\n t/helper/test-hash-speed.c            |  61 +++++++++\n t/helper/{test-sha1.c => test-hash.c} |  19 +--\n t/helper/test-sha1.c                  |  52 +-------\n t/helper/test-sha256.c                |   7 +\n t/helper/test-tool.c                  |   2 +\n t/helper/test-tool.h                  |   4 +\n t/t0014-hash.sh                       |  54 ++++++++\n 16 files changed, 592 insertions(+), 104 deletions(-)\n create mode 100644 sha256/block/sha256.c\n create mode 100644 sha256/block/sha256.h\n create mode 100644 sha256/gcrypt.h\n create mode 100644 t/helper/test-hash-speed.c\n copy t/helper/{test-sha1.c => test-hash.c} (66%)\n create mode 100644 t/helper/test-sha256.c\n create mode 100755 t/t0014-hash.sh\n\nRange-diff against v1:\n 1:  0a96c59452 =  1:  804ec2fd27 sha1-file: rename algorithm to \"sha1\"\n 2:  65f9feba41 =  2:  5196e00b26 sha1-file: provide functions to look up hash algorithms\n 3:  b6d960121d !  3:  5873510d0a hex: introduce functions to print arbitrary hashes\n    @@ -13,9 +13,12 @@\n         it.\n     \n         We use the variant taking the algorithm structure pointer as the\n    -    internal variant, since in the future we'll want to replace sha1_to_hex\n    -    with a hash_to_hex that handles the_hash_algo, and taking an algorithm\n    -    pointer is the easiest way to handle all of the variants in use.\n    +    internal variant, since taking an algorithm pointer is the easiest way\n    +    to handle all of the variants in use.\n    +\n    +    Note that we maintain these functions because there are hashes which\n    +    must change based on the hash algorithm in use but are not object IDs\n    +    (such as pack checksums).\n     \n         Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n     \n    @@ -47,6 +50,7 @@\n     +char *oid_to_hex_r(char *out, const struct object_id *oid);\n     +char *hash_to_hex_algo(const unsigned char *hash, int algo);\t/* static buffer result! */\n     +char *sha1_to_hex(const unsigned char *sha1);\t\t\t/* same static buffer */\n    ++char *hash_to_hex(const unsigned char *hash);\t\t\t/* same static buffer */\n     +char *oid_to_hex(const struct object_id *oid);\t\t\t/* same static buffer */\n      \n      /*\n    @@ -108,6 +112,11 @@\n     +char *sha1_to_hex(const unsigned char *sha1)\n     +{\n     +\treturn hash_to_hex_algo(sha1, GIT_HASH_SHA1);\n    ++}\n    ++\n    ++char *hash_to_hex(const unsigned char *hash)\n    ++{\n    ++\treturn hash_to_hex_algo(hash, hash_algo_by_ptr(the_hash_algo));\n      }\n      \n      char *oid_to_hex(const struct object_id *oid)\n -:  ---------- >  4:  e22bf99c93 cache: make hashcmp and hasheq work with larger hashes\n 4:  984698c42a =  5:  02cbcae270 t: add basic tests for our SHA-1 implementation\n 5:  61632644bd !  6:  e9ac5f265a t: make the sha1 test-tool helper generic\n    @@ -14,7 +14,7 @@\n      --- a/Makefile\n      +++ b/Makefile\n     @@\n    - TEST_BUILTINS_OBJS += test-dump-split-index.o\n    + TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n      TEST_BUILTINS_OBJS += test-example-decorate.o\n      TEST_BUILTINS_OBJS += test-genrandom.o\n     +TEST_BUILTINS_OBJS += test-hash.o\n    @@ -147,7 +147,7 @@\n      --- a/t/helper/test-tool.h\n      +++ b/t/helper/test-tool.h\n     @@\n    - int cmd__wildmatch(int argc, const char **argv);\n    + #endif\n      int cmd__write_cache(int argc, const char **argv);\n      \n     +int cmd_hash_impl(int ac, const char **av, int algo);\n 6:  79d5246af6 =  7:  44c9f27f9d sha1-file: add a constant for hash block size\n 7:  a83ceb35a8 =  8:  59be7f1f37 t/helper: add a test helper to compute hash speed\n 8:  ef3c5f3370 !  9:  09f1286769  commit-graph: convert to using the_hash_algo\n    @@ -1,6 +1,6 @@\n     Author: brian m. carlson <sandals@crustytoothpaste.net>\n     \n    -     commit-graph: convert to using the_hash_algo\n    +    commit-graph: convert to using the_hash_algo\n     \n         Instead of using hard-coded constants for object sizes, use\n         the_hash_algo to look them up.  In addition, use a function call to look\n    @@ -12,6 +12,12 @@\n      --- a/commit-graph.c\n      +++ b/commit-graph.c\n     @@\n    + #define GRAPH_CHUNKID_DATA 0x43444154 /* \"CDAT\" */\n    + #define GRAPH_CHUNKID_LARGEEDGES 0x45444745 /* \"EDGE\" */\n    + \n    +-#define GRAPH_DATA_WIDTH 36\n    ++#define GRAPH_DATA_WIDTH (the_hash_algo->rawsz + 16)\n    + \n      #define GRAPH_VERSION_1 0x1\n      #define GRAPH_VERSION GRAPH_VERSION_1\n      \n 9:  298b15b577 ! 10:  cdec8c1b49 Add a base implementation of SHA-256 support\n    @@ -3,39 +3,14 @@\n         Add a base implementation of SHA-256 support\n     \n         SHA-1 is weak and we need to transition to a new hash function.  For\n    -    some time, we have referred to this new function as NewHash.\n    -\n    -    The selection criteria for NewHash specify that it should (a) be 256\n    -    bits in length, (b) have high quality implementations available, (c)\n    -    should match Git's needs in terms of security, and (d) ideally, be fast\n    -    to compute.\n    -\n    -    SHA-256 has a variety of high quality implementations across various\n    -    libraries.  It is implemented by every cryptographic library we support\n    -    and is available on every platform and in almost every programming\n    -    language.  It is often highly optimized, since it is commonly used in\n    -    TLS and elsewhere.  Additionally, there are various command line\n    -    utilities that implement it, which is useful for educational and testing\n    -    purposes.\n    -\n    -    SHA-256 is presently considered secure and has received a reasonable\n    -    amount of cryptanalysis in the literature.  It is, admittedly, not\n    -    resistant to length extension attacks, but Git object storage is immune\n    -    to those due to the length field at the beginning.\n    -\n    -    SHA-256 is somewhat slower to compute than SHA-1 in software.  However,\n    -    since our default SHA-1 implementation is collision-detecting, a\n    -    reasonable cryptographic library implementation of SHA-256 will actually\n    -    be faster than SHA-256.  In addition, modern ARM and AMD processors (and\n    -    some Intel processors) contain instructions for implementing SHA-256 in\n    -    hardware, making it the fastest possible option.\n    -\n    -    There are other reasons to select SHA-256.  With signed commits and\n    -    tags, it's possible to use SHA-256 for signatures and therefore have to\n    -    rely on only one hash algorithm for security.\n    +    some time, we have referred to this new function as NewHash.  Recently,\n    +    we decided to pick SHA-256 as NewHash.\n     \n         Add a basic implementation of SHA-256 based off libtomcrypt, which is in\n    -    the public domain.  Optimize it and tidy it somewhat.\n    +    the public domain.  Optimize it and restructure it to meet our coding\n    +    standards.  Place it in a directory called \"sha256\" where it and any\n    +    future implementations can live so as to avoid a proliferation of\n    +    implementation directories.\n     \n         Wire up SHA-256 in the list of hash algorithms, and add a test that the\n         algorithm works correctly.\n    @@ -51,8 +26,8 @@\n      +++ b/Makefile\n     @@\n      TEST_BUILTINS_OBJS += test-scrap-cache-tree.o\n    - TEST_BUILTINS_OBJS += test-sha1-array.o\n      TEST_BUILTINS_OBJS += test-sha1.o\n    + TEST_BUILTINS_OBJS += test-sha1-array.o\n     +TEST_BUILTINS_OBJS += test-sha256.o\n      TEST_BUILTINS_OBJS += test-sigchain.o\n      TEST_BUILTINS_OBJS += test-strcmp-offset.o\n    @@ -459,8 +434,8 @@\n      +++ b/t/helper/test-tool.c\n     @@\n      \t{ \"scrap-cache-tree\", cmd__scrap_cache_tree },\n    - \t{ \"sha1-array\", cmd__sha1_array },\n      \t{ \"sha1\", cmd__sha1 },\n    + \t{ \"sha1-array\", cmd__sha1_array },\n     +\t{ \"sha256\", cmd__sha256 },\n      \t{ \"sigchain\", cmd__sigchain },\n      \t{ \"strcmp-offset\", cmd__strcmp_offset },\n    @@ -471,8 +446,8 @@\n      +++ b/t/helper/test-tool.h\n     @@\n      int cmd__scrap_cache_tree(int argc, const char **argv);\n    - int cmd__sha1_array(int argc, const char **argv);\n      int cmd__sha1(int argc, const char **argv);\n    + int cmd__sha1_array(int argc, const char **argv);\n     +int cmd__sha256(int argc, const char **argv);\n      int cmd__sigchain(int argc, const char **argv);\n      int cmd__strcmp_offset(int argc, const char **argv);\n10:  c52b4be13c = 11:  d085802a89 sha256: add an SHA-256 implementation using libgcrypt\n11:  225ec075d1 = 12:  4e4daf4000 hash: add an SHA-256 implementation using OpenSSL\n12:  04a3ac6f29 = 13:  f206f45426 commit-graph: specify OID version for SHA-256\n"},{"id":"360461","messageId":"20181015021900.1030041-3-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 02/13] sha1-file: provide functions to look up hash algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:49Z","receivedAt":"2018-10-15T02:19:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There are several ways we might refer to a hash algorithm: by name, such\nas in the config file; by format ID, such as in a pack; or internally,\nby a pointer to the hash_algos array.  Provide functions to look up hash\nalgorithms based on these various forms and return the internal constant\nused for them.  If conversion to another form is necessary, this\ninternal constant can be used to look up the proper data in the\nhash_algos array.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n hash.h      | 13 +++++++++++++\n sha1-file.c | 21 +++++++++++++++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/hash.h b/hash.h\nindex 7c8238bc2e..90f4344619 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -98,4 +98,17 @@ struct git_hash_algo {\n };\n extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n \n+/*\n+ * Return a GIT_HASH_* constant based on the name.  Returns GIT_HASH_UNKNOWN if\n+ * the name doesn't match a known algorithm.\n+ */\n+int hash_algo_by_name(const char *name);\n+/* Identical, except based on the format ID. */\n+int hash_algo_by_id(uint32_t format_id);\n+/* Identical, except for a pointer to struct git_hash_algo. */\n+inline int hash_algo_by_ptr(const struct git_hash_algo *p)\n+{\n+\treturn p - hash_algos;\n+}\n+\n #endif\ndiff --git a/sha1-file.c b/sha1-file.c\nindex e29825f259..3a75d515eb 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -122,6 +122,27 @@ const char *empty_blob_oid_hex(void)\n \treturn oid_to_hex_r(buf, the_hash_algo->empty_blob);\n }\n \n+int hash_algo_by_name(const char *name)\n+{\n+\tint i;\n+\tif (!name)\n+\t\treturn GIT_HASH_UNKNOWN;\n+\tfor (i = 1; i < GIT_HASH_NALGOS; i++)\n+\t\tif (!strcmp(name, hash_algos[i].name))\n+\t\t\treturn i;\n+\treturn GIT_HASH_UNKNOWN;\n+}\n+\n+int hash_algo_by_id(uint32_t format_id)\n+{\n+\tint i;\n+\tfor (i = 1; i < GIT_HASH_NALGOS; i++)\n+\t\tif (format_id == hash_algos[i].format_id)\n+\t\t\treturn i;\n+\treturn GIT_HASH_UNKNOWN;\n+}\n+\n+\n /*\n  * This is meant to hold a *small* number of objects that you would\n  * want read_sha1_file() to be able to return, but yet you do not want\n"},{"id":"360462","messageId":"20181015021900.1030041-5-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 04/13] cache: make hashcmp and hasheq work with larger hashes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:51Z","receivedAt":"2018-10-15T02:19:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In 183a638b7d (\"hashcmp: assert constant hash size\", 2018-08-23), we\nmodified hashcmp to assert that the hash size was always 20 to help it\noptimize and inline calls to memcmp.  In a future series, we replaced\nmany calls to hashcmp and oidcmp with calls to hasheq and oideq to\nimprove inlining further.\n\nHowever, we want to support hash algorithms other than SHA-1, namely\nSHA-256.  When doing so, we must handle the case where these values are\n32 bytes long as well as 20.  Adjust hashcmp to handle two cases:\n20-byte matches, and maximum-size matches.  Therefore, when we include\nSHA-256, we'll automatically handle it properly, while at the same time\nteaching the compiler that there are only two possible options to\nconsider.  This will allow the compiler to write the most efficient\npossible code.\n\nCopy similar code into hasheq and perform an identical transformation.\nAt least with GCC 8.2.0, making hasheq defer to hashcmp when there are\ntwo branches prevents the compiler from inlining the comparison, while\nthe code in this patch is inlined properly.  Add a comment to avoid an\naccidental performance regression from well-intentioned refactoring.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n cache.h | 22 ++++++++++++----------\n 1 file changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex a13d14ce0a..0b88c3a344 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1024,16 +1024,12 @@ extern const struct object_id null_oid;\n static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)\n {\n \t/*\n-\t * This is a temporary optimization hack. By asserting the size here,\n-\t * we let the compiler know that it's always going to be 20, which lets\n-\t * it turn this fixed-size memcmp into a few inline instructions.\n-\t *\n-\t * This will need to be extended or ripped out when we learn about\n-\t * hashes of different sizes.\n+\t * Teach the compiler that there are only two possibilities of hash size\n+\t * here, so that it can optimize for this case as much as possible.\n \t */\n-\tif (the_hash_algo->rawsz != 20)\n-\t\tBUG(\"hash size not yet supported by hashcmp\");\n-\treturn memcmp(sha1, sha2, the_hash_algo->rawsz);\n+\tif (the_hash_algo->rawsz == GIT_MAX_RAWSZ)\n+\t\treturn memcmp(sha1, sha2, GIT_MAX_RAWSZ);\n+\treturn memcmp(sha1, sha2, GIT_SHA1_RAWSZ);\n }\n \n static inline int oidcmp(const struct object_id *oid1, const struct object_id *oid2)\n@@ -1043,7 +1039,13 @@ static inline int oidcmp(const struct object_id *oid1, const struct object_id *o\n \n static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2)\n {\n-\treturn !hashcmp(sha1, sha2);\n+\t/*\n+\t * We write this here instead of deferring to hashcmp so that the\n+\t * compiler can properly inline it and avoid calling memcmp.\n+\t */\n+\tif (the_hash_algo->rawsz == GIT_MAX_RAWSZ)\n+\t\treturn !memcmp(sha1, sha2, GIT_MAX_RAWSZ);\n+\treturn !memcmp(sha1, sha2, GIT_SHA1_RAWSZ);\n }\n \n static inline int oideq(const struct object_id *oid1, const struct object_id *oid2)\n"},{"id":"360463","messageId":"20181015021900.1030041-7-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 06/13] t: make the sha1 test-tool helper generic","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:53Z","receivedAt":"2018-10-15T02:19:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Since we're going to have multiple hash algorithms to test, it makes\nsense to share as much of the test code as possible.  Convert the sha1\nhelper for the test-tool to be generic and move it out into its own\nmodule.  This will allow us to share most of this code with our NewHash\nimplementation.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                              |  1 +\n t/helper/{test-sha1.c => test-hash.c} | 19 +++++-----\n t/helper/test-sha1.c                  | 52 +--------------------------\n t/helper/test-tool.h                  |  2 ++\n 4 files changed, 14 insertions(+), 60 deletions(-)\n copy t/helper/{test-sha1.c => test-hash.c} (66%)\n\ndiff --git a/Makefile b/Makefile\nindex 5c8307b7c4..324967410d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -714,6 +714,7 @@ TEST_BUILTINS_OBJS += test-dump-split-index.o\n TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n TEST_BUILTINS_OBJS += test-example-decorate.o\n TEST_BUILTINS_OBJS += test-genrandom.o\n+TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\n TEST_BUILTINS_OBJS += test-index-version.o\n TEST_BUILTINS_OBJS += test-json-writer.o\ndiff --git a/t/helper/test-sha1.c b/t/helper/test-hash.c\nsimilarity index 66%\ncopy from t/helper/test-sha1.c\ncopy to t/helper/test-hash.c\nindex 1ba0675c75..9992de2212 100644\n--- a/t/helper/test-sha1.c\n+++ b/t/helper/test-hash.c\n@@ -1,13 +1,14 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n \n-int cmd__sha1(int ac, const char **av)\n+int cmd_hash_impl(int ac, const char **av, int algo)\n {\n-\tgit_SHA_CTX ctx;\n-\tunsigned char sha1[20];\n+\tgit_hash_ctx ctx;\n+\tunsigned char hash[GIT_MAX_HEXSZ];\n \tunsigned bufsz = 8192;\n \tint binary = 0;\n \tchar *buffer;\n+\tconst struct git_hash_algo *algop = &hash_algos[algo];\n \n \tif (ac == 2) {\n \t\tif (!strcmp(av[1], \"-b\"))\n@@ -26,7 +27,7 @@ int cmd__sha1(int ac, const char **av)\n \t\t\tdie(\"OOPS\");\n \t}\n \n-\tgit_SHA1_Init(&ctx);\n+\talgop->init_fn(&ctx);\n \n \twhile (1) {\n \t\tssize_t sz, this_sz;\n@@ -38,20 +39,20 @@ int cmd__sha1(int ac, const char **av)\n \t\t\tif (sz == 0)\n \t\t\t\tbreak;\n \t\t\tif (sz < 0)\n-\t\t\t\tdie_errno(\"test-sha1\");\n+\t\t\t\tdie_errno(\"test-hash\");\n \t\t\tthis_sz += sz;\n \t\t\tcp += sz;\n \t\t\troom -= sz;\n \t\t}\n \t\tif (this_sz == 0)\n \t\t\tbreak;\n-\t\tgit_SHA1_Update(&ctx, buffer, this_sz);\n+\t\talgop->update_fn(&ctx, buffer, this_sz);\n \t}\n-\tgit_SHA1_Final(sha1, &ctx);\n+\talgop->final_fn(hash, &ctx);\n \n \tif (binary)\n-\t\tfwrite(sha1, 1, 20, stdout);\n+\t\tfwrite(hash, 1, algop->rawsz, stdout);\n \telse\n-\t\tputs(sha1_to_hex(sha1));\n+\t\tputs(hash_to_hex_algo(hash, algo));\n \texit(0);\n }\ndiff --git a/t/helper/test-sha1.c b/t/helper/test-sha1.c\nindex 1ba0675c75..d860c387c3 100644\n--- a/t/helper/test-sha1.c\n+++ b/t/helper/test-sha1.c\n@@ -3,55 +3,5 @@\n \n int cmd__sha1(int ac, const char **av)\n {\n-\tgit_SHA_CTX ctx;\n-\tunsigned char sha1[20];\n-\tunsigned bufsz = 8192;\n-\tint binary = 0;\n-\tchar *buffer;\n-\n-\tif (ac == 2) {\n-\t\tif (!strcmp(av[1], \"-b\"))\n-\t\t\tbinary = 1;\n-\t\telse\n-\t\t\tbufsz = strtoul(av[1], NULL, 10) * 1024 * 1024;\n-\t}\n-\n-\tif (!bufsz)\n-\t\tbufsz = 8192;\n-\n-\twhile ((buffer = malloc(bufsz)) == NULL) {\n-\t\tfprintf(stderr, \"bufsz %u is too big, halving...\\n\", bufsz);\n-\t\tbufsz /= 2;\n-\t\tif (bufsz < 1024)\n-\t\t\tdie(\"OOPS\");\n-\t}\n-\n-\tgit_SHA1_Init(&ctx);\n-\n-\twhile (1) {\n-\t\tssize_t sz, this_sz;\n-\t\tchar *cp = buffer;\n-\t\tunsigned room = bufsz;\n-\t\tthis_sz = 0;\n-\t\twhile (room) {\n-\t\t\tsz = xread(0, cp, room);\n-\t\t\tif (sz == 0)\n-\t\t\t\tbreak;\n-\t\t\tif (sz < 0)\n-\t\t\t\tdie_errno(\"test-sha1\");\n-\t\t\tthis_sz += sz;\n-\t\t\tcp += sz;\n-\t\t\troom -= sz;\n-\t\t}\n-\t\tif (this_sz == 0)\n-\t\t\tbreak;\n-\t\tgit_SHA1_Update(&ctx, buffer, this_sz);\n-\t}\n-\tgit_SHA1_Final(sha1, &ctx);\n-\n-\tif (binary)\n-\t\tfwrite(sha1, 1, 20, stdout);\n-\telse\n-\t\tputs(sha1_to_hex(sha1));\n-\texit(0);\n+\treturn cmd_hash_impl(ac, av, GIT_HASH_SHA1);\n }\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex e4890566da..29ac7b0b0d 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -50,4 +50,6 @@ int cmd__windows_named_pipe(int argc, const char **argv);\n #endif\n int cmd__write_cache(int argc, const char **argv);\n \n+int cmd_hash_impl(int ac, const char **av, int algo);\n+\n #endif\n"},{"id":"360464","messageId":"20181015021900.1030041-8-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 07/13] sha1-file: add a constant for hash block size","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:54Z","receivedAt":"2018-10-15T02:19:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There is one place we need the hash algorithm block size: the HMAC code\nfor push certs.  Expose this constant in struct git_hash_algo and expose\nvalues for SHA-1 and for the largest value of any hash.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n cache.h     | 4 ++++\n hash.h      | 3 +++\n sha1-file.c | 2 ++\n 3 files changed, 9 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 0b88c3a344..48e736b0d5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -45,10 +45,14 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);\n /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n #define GIT_SHA1_RAWSZ 20\n #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)\n+/* The block size of SHA-1. */\n+#define GIT_SHA1_BLKSZ 64\n \n /* The length in byte and in hex digits of the largest possible hash value. */\n #define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ\n #define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ\n+/* The largest possible block size for any supported hash. */\n+#define GIT_MAX_BLKSZ GIT_SHA1_BLKSZ\n \n struct object_id {\n \tunsigned char hash[GIT_MAX_RAWSZ];\ndiff --git a/hash.h b/hash.h\nindex 90f4344619..46dff69eb3 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -81,6 +81,9 @@ struct git_hash_algo {\n \t/* The length of the hash in hex characters. */\n \tsize_t hexsz;\n \n+\t/* The block size of the hash. */\n+\tsize_t blksz;\n+\n \t/* The hash initialization function. */\n \tgit_hash_init_fn init_fn;\n \ndiff --git a/sha1-file.c b/sha1-file.c\nindex 3a75d515eb..9aadaf0c8d 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -90,6 +90,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\t0x00000000,\n \t\t0,\n \t\t0,\n+\t\t0,\n \t\tgit_hash_unknown_init,\n \t\tgit_hash_unknown_update,\n \t\tgit_hash_unknown_final,\n@@ -102,6 +103,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\t0x73686131,\n \t\tGIT_SHA1_RAWSZ,\n \t\tGIT_SHA1_HEXSZ,\n+\t\tGIT_SHA1_BLKSZ,\n \t\tgit_hash_sha1_init,\n \t\tgit_hash_sha1_update,\n \t\tgit_hash_sha1_final,\n"},{"id":"360465","messageId":"20181015021900.1030041-4-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 03/13] hex: introduce functions to print arbitrary hashes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:50Z","receivedAt":"2018-10-15T02:19:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently, we have functions that turn an arbitrary SHA-1 value or an\nobject ID into hex format, either using a static buffer or with a\nuser-provided buffer.  Add variants of these functions that can handle\nan arbitrary hash algorithm, specified by constant.  Update the\ndocumentation as well.\n\nWhile we're at it, remove the \"extern\" declaration from this family of\nfunctions, since it's not needed and our style now recommends against\nit.\n\nWe use the variant taking the algorithm structure pointer as the\ninternal variant, since taking an algorithm pointer is the easiest way\nto handle all of the variants in use.\n\nNote that we maintain these functions because there are hashes which\nmust change based on the hash algorithm in use but are not object IDs\n(such as pack checksums).\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n cache.h | 15 +++++++++------\n hex.c   | 37 +++++++++++++++++++++++++++++--------\n 2 files changed, 38 insertions(+), 14 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d508f3d4f8..a13d14ce0a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1361,9 +1361,9 @@ extern int get_oid_hex(const char *hex, struct object_id *sha1);\n extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n \n /*\n- * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant,\n+ * Convert a binary hash to its hex equivalent. The `_r` variant is reentrant,\n  * and writes the NUL-terminated output to the buffer `out`, which must be at\n- * least `GIT_SHA1_HEXSZ + 1` bytes, and returns a pointer to out for\n+ * least `GIT_MAX_HEXSZ + 1` bytes, and returns a pointer to out for\n  * convenience.\n  *\n  * The non-`_r` variant returns a static buffer, but uses a ring of 4\n@@ -1371,10 +1371,13 @@ extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n  *\n  *   printf(\"%s -> %s\", sha1_to_hex(one), sha1_to_hex(two));\n  */\n-extern char *sha1_to_hex_r(char *out, const unsigned char *sha1);\n-extern char *oid_to_hex_r(char *out, const struct object_id *oid);\n-extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n-extern char *oid_to_hex(const struct object_id *oid);\t/* same static buffer as sha1_to_hex */\n+char *hash_to_hex_algo_r(char *buffer, const unsigned char *hash, int algo);\n+char *sha1_to_hex_r(char *out, const unsigned char *sha1);\n+char *oid_to_hex_r(char *out, const struct object_id *oid);\n+char *hash_to_hex_algo(const unsigned char *hash, int algo);\t/* static buffer result! */\n+char *sha1_to_hex(const unsigned char *sha1);\t\t\t/* same static buffer */\n+char *hash_to_hex(const unsigned char *hash);\t\t\t/* same static buffer */\n+char *oid_to_hex(const struct object_id *oid);\t\t\t/* same static buffer */\n \n /*\n  * Parse a 40-character hexadecimal object ID starting from hex, updating the\ndiff --git a/hex.c b/hex.c\nindex 10af1a29e8..080597ad3f 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -73,14 +73,15 @@ int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)\n \treturn ret;\n }\n \n-char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)\n+static inline char *hash_to_hex_algop_r(char *buffer, const unsigned char *hash,\n+\t\t\t\t\tconst struct git_hash_algo *algop)\n {\n \tstatic const char hex[] = \"0123456789abcdef\";\n \tchar *buf = buffer;\n \tint i;\n \n-\tfor (i = 0; i < the_hash_algo->rawsz; i++) {\n-\t\tunsigned int val = *sha1++;\n+\tfor (i = 0; i < algop->rawsz; i++) {\n+\t\tunsigned int val = *hash++;\n \t\t*buf++ = hex[val >> 4];\n \t\t*buf++ = hex[val & 0xf];\n \t}\n@@ -89,20 +90,40 @@ char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)\n \treturn buffer;\n }\n \n-char *oid_to_hex_r(char *buffer, const struct object_id *oid)\n+char *hash_to_hex_algo_r(char *buffer, const unsigned char *hash, int algo)\n {\n-\treturn sha1_to_hex_r(buffer, oid->hash);\n+\treturn hash_to_hex_algop_r(buffer, hash, &hash_algos[algo]);\n }\n \n-char *sha1_to_hex(const unsigned char *sha1)\n+char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)\n+{\n+\treturn hash_to_hex_algo_r(buffer, sha1, GIT_HASH_SHA1);\n+}\n+\n+char *oid_to_hex_r(char *buffer, const struct object_id *oid)\n+{\n+\treturn hash_to_hex_algop_r(buffer, oid->hash, the_hash_algo);\n+}\n+\n+char *hash_to_hex_algo(const unsigned char *hash, int algo)\n {\n \tstatic int bufno;\n \tstatic char hexbuffer[4][GIT_MAX_HEXSZ + 1];\n \tbufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);\n-\treturn sha1_to_hex_r(hexbuffer[bufno], sha1);\n+\treturn hash_to_hex_algo_r(hexbuffer[bufno], hash, algo);\n+}\n+\n+char *sha1_to_hex(const unsigned char *sha1)\n+{\n+\treturn hash_to_hex_algo(sha1, GIT_HASH_SHA1);\n+}\n+\n+char *hash_to_hex(const unsigned char *hash)\n+{\n+\treturn hash_to_hex_algo(hash, hash_algo_by_ptr(the_hash_algo));\n }\n \n char *oid_to_hex(const struct object_id *oid)\n {\n-\treturn sha1_to_hex(oid->hash);\n+\treturn hash_to_hex_algo(oid->hash, hash_algo_by_ptr(the_hash_algo));\n }\n"},{"id":"360466","messageId":"20181015021900.1030041-6-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 05/13] t: add basic tests for our SHA-1 implementation","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:52Z","receivedAt":"2018-10-15T02:19:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have in the past had some unfortunate endianness issues with some\nSHA-1 implementations we ship, especially on big-endian machines.  Add\nan explicit test using the test helper to catch these issues and point\nthem out prominently.  This test can also be used as a staging ground\nfor people testing additional algorithms to verify that their\nimplementations are working as expected.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0014-hash.sh | 29 +++++++++++++++++++++++++++++\n 1 file changed, 29 insertions(+)\n create mode 100755 t/t0014-hash.sh\n\ndiff --git a/t/t0014-hash.sh b/t/t0014-hash.sh\nnew file mode 100755\nindex 0000000000..8e763c2c3d\n--- /dev/null\n+++ b/t/t0014-hash.sh\n@@ -0,0 +1,29 @@\n+#!/bin/sh\n+\n+test_description='test basic hash implementation'\n+. ./test-lib.sh\n+\n+\n+test_expect_success 'test basic SHA-1 hash values' '\n+\ttest-tool sha1 </dev/null >actual &&\n+\tgrep da39a3ee5e6b4b0d3255bfef95601890afd80709 actual &&\n+\tprintf \"a\" | test-tool sha1 >actual &&\n+\tgrep 86f7e437faa5a7fce15d1ddcb9eaeaea377667b8 actual &&\n+\tprintf \"abc\" | test-tool sha1 >actual &&\n+\tgrep a9993e364706816aba3e25717850c26c9cd0d89d actual &&\n+\tprintf \"message digest\" | test-tool sha1 >actual &&\n+\tgrep c12252ceda8be8994d5fa0290a47231c1d16aae3 actual &&\n+\tprintf \"abcdefghijklmnopqrstuvwxyz\" | test-tool sha1 >actual &&\n+\tgrep 32d10c7b8cf96570ca04ce37f2a19d84240d3a89 actual &&\n+\tperl -E \"for (1..100000) { print q{aaaaaaaaaa}; }\" | \\\n+\t\ttest-tool sha1 >actual &&\n+\tgrep 34aa973cd4c4daa4f61eeb2bdbad27316534016f actual &&\n+\tprintf \"blob 0\\0\" | test-tool sha1 >actual &&\n+\tgrep e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 actual &&\n+\tprintf \"blob 3\\0abc\" | test-tool sha1 >actual &&\n+\tgrep f2ba8f84ab5c1bce84a7b441cb1959cfc7093b7f actual &&\n+\tprintf \"tree 0\\0\" | test-tool sha1 >actual &&\n+\tgrep 4b825dc642cb6eb9a060e54bf8d69288fbee4904 actual\n+'\n+\n+test_done\n"},{"id":"360469","messageId":"20181015021900.1030041-9-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 08/13] t/helper: add a test helper to compute hash speed","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:55Z","receivedAt":"2018-10-15T02:19:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Add a utility (which is less for the testsuite and more for developers)\nthat can compute hash speeds for whatever hash algorithms are\nimplemented.  This allows developers to test their personal systems to\ndetermine the performance characteristics of various algorithms.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                   |  1 +\n t/helper/test-hash-speed.c | 61 ++++++++++++++++++++++++++++++++++++++\n t/helper/test-tool.c       |  1 +\n t/helper/test-tool.h       |  1 +\n 4 files changed, 64 insertions(+)\n create mode 100644 t/helper/test-hash-speed.c\n\ndiff --git a/Makefile b/Makefile\nindex 324967410d..1c43bf9aa8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -716,6 +716,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o\n TEST_BUILTINS_OBJS += test-genrandom.o\n TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\n+TEST_BUILTINS_OBJS += test-hash-speed.o\n TEST_BUILTINS_OBJS += test-index-version.o\n TEST_BUILTINS_OBJS += test-json-writer.o\n TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\ndiff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c\nnew file mode 100644\nindex 0000000000..432233c7f0\n--- /dev/null\n+++ b/t/helper/test-hash-speed.c\n@@ -0,0 +1,61 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+\n+#define NUM_SECONDS 3\n+\n+static inline void compute_hash(const struct git_hash_algo *algo, git_hash_ctx *ctx, uint8_t *final, const void *p, size_t len)\n+{\n+\talgo->init_fn(ctx);\n+\talgo->update_fn(ctx, p, len);\n+\talgo->final_fn(final, ctx);\n+}\n+\n+int cmd__hash_speed(int ac, const char **av)\n+{\n+\tgit_hash_ctx ctx;\n+\tunsigned char hash[GIT_MAX_RAWSZ];\n+\tclock_t initial, start, end;\n+\tunsigned bufsizes[] = { 64, 256, 1024, 8192, 16384 };\n+\tint i;\n+\tvoid *p;\n+\tconst struct git_hash_algo *algo = NULL;\n+\n+\tif (ac == 2) {\n+\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n+\t\t\tif (!strcmp(av[1], hash_algos[i].name)) {\n+\t\t\t\talgo = &hash_algos[i];\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tif (!algo)\n+\t\tdie(\"usage: test-tool hash-speed algo_name\");\n+\n+\t/* Use this as an offset to make overflow less likely. */\n+\tinitial = clock();\n+\n+\tprintf(\"algo: %s\\n\", algo->name);\n+\n+\tfor (i = 0; i < ARRAY_SIZE(bufsizes); i++) {\n+\t\tunsigned long j, kb;\n+\t\tdouble kb_per_sec;\n+\t\tp = xcalloc(1, bufsizes[i]);\n+\t\tstart = end = clock() - initial;\n+\t\tfor (j = 0; ((end - start) / CLOCKS_PER_SEC) < NUM_SECONDS; j++) {\n+\t\t\tcompute_hash(algo, &ctx, hash, p, bufsizes[i]);\n+\n+\t\t\t/*\n+\t\t\t * Only check elapsed time every 128 iterations to avoid\n+\t\t\t * dominating the runtime with system calls.\n+\t\t\t */\n+\t\t\tif (!(j & 127))\n+\t\t\t\tend = clock() - initial;\n+\t\t}\n+\t\tkb = j * bufsizes[i];\n+\t\tkb_per_sec = kb / (1024 * ((double)end - start) / CLOCKS_PER_SEC);\n+\t\tprintf(\"size %u: %lu iters; %lu KiB; %0.2f KiB/s\\n\", bufsizes[i], j, kb, kb_per_sec);\n+\t\tfree(p);\n+\t}\n+\n+\texit(0);\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 6b5836dc1b..e009c8186d 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -20,6 +20,7 @@ static struct test_cmd cmds[] = {\n \t{ \"example-decorate\", cmd__example_decorate },\n \t{ \"genrandom\", cmd__genrandom },\n \t{ \"hashmap\", cmd__hashmap },\n+\t{ \"hash-speed\", cmd__hash_speed },\n \t{ \"index-version\", cmd__index_version },\n \t{ \"json-writer\", cmd__json_writer },\n \t{ \"lazy-init-name-hash\", cmd__lazy_init_name_hash },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 29ac7b0b0d..19a7e8332a 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -16,6 +16,7 @@ int cmd__dump_untracked_cache(int argc, const char **argv);\n int cmd__example_decorate(int argc, const char **argv);\n int cmd__genrandom(int argc, const char **argv);\n int cmd__hashmap(int argc, const char **argv);\n+int cmd__hash_speed(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\n int cmd__json_writer(int argc, const char **argv);\n int cmd__lazy_init_name_hash(int argc, const char **argv);\n"},{"id":"360468","messageId":"20181015021900.1030041-10-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 09/13] commit-graph: convert to using the_hash_algo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:56Z","receivedAt":"2018-10-15T02:19:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Instead of using hard-coded constants for object sizes, use\nthe_hash_algo to look them up.  In addition, use a function call to look\nup the object ID version and produce the correct value.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n commit-graph.c | 33 +++++++++++++++++----------------\n 1 file changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 7f4519ec3b..7a28fbb03f 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -20,16 +20,11 @@\n #define GRAPH_CHUNKID_DATA 0x43444154 /* \"CDAT\" */\n #define GRAPH_CHUNKID_LARGEEDGES 0x45444745 /* \"EDGE\" */\n \n-#define GRAPH_DATA_WIDTH 36\n+#define GRAPH_DATA_WIDTH (the_hash_algo->rawsz + 16)\n \n #define GRAPH_VERSION_1 0x1\n #define GRAPH_VERSION GRAPH_VERSION_1\n \n-#define GRAPH_OID_VERSION_SHA1 1\n-#define GRAPH_OID_LEN_SHA1 GIT_SHA1_RAWSZ\n-#define GRAPH_OID_VERSION GRAPH_OID_VERSION_SHA1\n-#define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1\n-\n #define GRAPH_OCTOPUS_EDGES_NEEDED 0x80000000\n #define GRAPH_PARENT_MISSING 0x7fffffff\n #define GRAPH_EDGE_LAST_MASK 0x7fffffff\n@@ -41,13 +36,18 @@\n #define GRAPH_FANOUT_SIZE (4 * 256)\n #define GRAPH_CHUNKLOOKUP_WIDTH 12\n #define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \\\n-\t\t\t+ GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)\n+\t\t\t+ GRAPH_FANOUT_SIZE + the_hash_algo->rawsz)\n \n char *get_commit_graph_filename(const char *obj_dir)\n {\n \treturn xstrfmt(\"%s/info/commit-graph\", obj_dir);\n }\n \n+static uint8_t oid_version(void)\n+{\n+\treturn 1;\n+}\n+\n static struct commit_graph *alloc_commit_graph(void)\n {\n \tstruct commit_graph *g = xcalloc(1, sizeof(*g));\n@@ -100,15 +100,15 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)\n \t}\n \n \thash_version = *(unsigned char*)(data + 5);\n-\tif (hash_version != GRAPH_OID_VERSION) {\n+\tif (hash_version != oid_version()) {\n \t\terror(_(\"hash version %X does not match version %X\"),\n-\t\t      hash_version, GRAPH_OID_VERSION);\n+\t\t      hash_version, oid_version());\n \t\tgoto cleanup_fail;\n \t}\n \n \tgraph = alloc_commit_graph();\n \n-\tgraph->hash_len = GRAPH_OID_LEN;\n+\tgraph->hash_len = the_hash_algo->rawsz;\n \tgraph->num_chunks = *(unsigned char*)(data + 6);\n \tgraph->graph_fd = fd;\n \tgraph->data = graph_map;\n@@ -124,7 +124,7 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)\n \n \t\tchunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;\n \n-\t\tif (chunk_offset > graph_size - GIT_MAX_RAWSZ) {\n+\t\tif (chunk_offset > graph_size - the_hash_algo->rawsz) {\n \t\t\terror(_(\"improper chunk offset %08x%08x\"), (uint32_t)(chunk_offset >> 32),\n \t\t\t      (uint32_t)chunk_offset);\n \t\t\tgoto cleanup_fail;\n@@ -711,6 +711,7 @@ void write_commit_graph(const char *obj_dir,\n \tint num_chunks;\n \tint num_extra_edges;\n \tstruct commit_list *parent;\n+\tconst unsigned hashsz = the_hash_algo->rawsz;\n \n \toids.nr = 0;\n \toids.alloc = approximate_object_count() / 4;\n@@ -831,7 +832,7 @@ void write_commit_graph(const char *obj_dir,\n \thashwrite_be32(f, GRAPH_SIGNATURE);\n \n \thashwrite_u8(f, GRAPH_VERSION);\n-\thashwrite_u8(f, GRAPH_OID_VERSION);\n+\thashwrite_u8(f, oid_version());\n \thashwrite_u8(f, num_chunks);\n \thashwrite_u8(f, 0); /* unused padding byte */\n \n@@ -846,8 +847,8 @@ void write_commit_graph(const char *obj_dir,\n \n \tchunk_offsets[0] = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH;\n \tchunk_offsets[1] = chunk_offsets[0] + GRAPH_FANOUT_SIZE;\n-\tchunk_offsets[2] = chunk_offsets[1] + GRAPH_OID_LEN * commits.nr;\n-\tchunk_offsets[3] = chunk_offsets[2] + (GRAPH_OID_LEN + 16) * commits.nr;\n+\tchunk_offsets[2] = chunk_offsets[1] + hashsz * commits.nr;\n+\tchunk_offsets[3] = chunk_offsets[2] + (hashsz + 16) * commits.nr;\n \tchunk_offsets[4] = chunk_offsets[3] + 4 * num_extra_edges;\n \n \tfor (i = 0; i <= num_chunks; i++) {\n@@ -860,8 +861,8 @@ void write_commit_graph(const char *obj_dir,\n \t}\n \n \twrite_graph_chunk_fanout(f, commits.list, commits.nr);\n-\twrite_graph_chunk_oids(f, GRAPH_OID_LEN, commits.list, commits.nr);\n-\twrite_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);\n+\twrite_graph_chunk_oids(f, hashsz, commits.list, commits.nr);\n+\twrite_graph_chunk_data(f, hashsz, commits.list, commits.nr);\n \twrite_graph_chunk_large_edges(f, commits.list, commits.nr);\n \n \tclose_commit_graph();\n"},{"id":"360470","messageId":"20181015021900.1030041-12-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 11/13] sha256: add an SHA-256 implementation using libgcrypt","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:58Z","receivedAt":"2018-10-15T02:19:24Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Generally, one gets better performance out of cryptographic routines\nwritten in assembly than C, and this is also true for SHA-256.  In\naddition, most Linux distributions cannot distribute Git linked against\nOpenSSL for licensing reasons.\n\nMost systems with GnuPG will also have libgcrypt, since it is a\ndependency of GnuPG.  libgcrypt is also faster than the SHA1DC\nimplementation for messages of a few KiB and larger. It is licensed\nunder the LGPL 2.1, which is compatible with the GPL.\n\nAdd an implementation of SHA-256 that uses libgcrypt.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile        | 13 +++++++++++--\n hash.h          |  4 ++++\n sha256/gcrypt.h | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 2 deletions(-)\n create mode 100644 sha256/gcrypt.h\n\ndiff --git a/Makefile b/Makefile\nindex 76d378c7ba..3d91555a81 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -179,6 +179,10 @@ all::\n # in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO\n # wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.\n #\n+# Define BLK_SHA256 to use the built-in SHA-256 routines.\n+#\n+# Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt.\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1634,8 +1638,13 @@ endif\n endif\n endif\n \n-LIB_OBJS += sha256/block/sha256.o\n-BASIC_CFLAGS += -DSHA256_BLK\n+ifdef GCRYPT_SHA256\n+\tBASIC_CFLAGS += -DSHA256_GCRYPT\n+\tEXTLIBS += -lgcrypt\n+else\n+\tLIB_OBJS += sha256/block/sha256.o\n+\tBASIC_CFLAGS += -DSHA256_BLK\n+endif\n \n ifdef SHA1_MAX_BLOCK_SIZE\n \tLIB_OBJS += compat/sha1-chunked.o\ndiff --git a/hash.h b/hash.h\nindex 88d18896d7..9df562f2f6 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -15,7 +15,11 @@\n #include \"block-sha1/sha1.h\"\n #endif\n \n+#if defined(SHA256_GCRYPT)\n+#include \"sha256/gcrypt.h\"\n+#else\n #include \"sha256/block/sha256.h\"\n+#endif\n \n #ifndef platform_SHA_CTX\n /*\ndiff --git a/sha256/gcrypt.h b/sha256/gcrypt.h\nnew file mode 100644\nindex 0000000000..09bd8bb200\n--- /dev/null\n+++ b/sha256/gcrypt.h\n@@ -0,0 +1,30 @@\n+#ifndef SHA256_GCRYPT_H\n+#define SHA256_GCRYPT_H\n+\n+#include <gcrypt.h>\n+\n+#define SHA256_DIGEST_SIZE 32\n+\n+typedef gcry_md_hd_t gcrypt_SHA256_CTX;\n+\n+inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx)\n+{\n+\tgcry_md_open(ctx, GCRY_MD_SHA256, 0);\n+}\n+\n+inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len)\n+{\n+\tgcry_md_write(*ctx, data, len);\n+}\n+\n+inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)\n+{\n+\tmemcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);\n+}\n+\n+#define platform_SHA256_CTX gcrypt_SHA256_CTX\n+#define platform_SHA256_Init gcrypt_SHA256_Init\n+#define platform_SHA256_Update gcrypt_SHA256_Update\n+#define platform_SHA256_Final gcrypt_SHA256_Final\n+\n+#endif\n"},{"id":"360467","messageId":"20181015021900.1030041-13-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 12/13] hash: add an SHA-256 implementation using OpenSSL","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:59Z","receivedAt":"2018-10-15T02:19:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We already have OpenSSL routines available for SHA-1, so add routines\nfor SHA-256 as well.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile | 7 +++++++\n hash.h   | 2 ++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3d91555a81..3164e2aeee 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -183,6 +183,8 @@ all::\n #\n # Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt.\n #\n+# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1638,6 +1640,10 @@ endif\n endif\n endif\n \n+ifdef OPENSSL_SHA256\n+\tEXTLIBS += $(LIB_4_CRYPTO)\n+\tBASIC_CFLAGS += -DSHA256_OPENSSL\n+else\n ifdef GCRYPT_SHA256\n \tBASIC_CFLAGS += -DSHA256_GCRYPT\n \tEXTLIBS += -lgcrypt\n@@ -1645,6 +1651,7 @@ else\n \tLIB_OBJS += sha256/block/sha256.o\n \tBASIC_CFLAGS += -DSHA256_BLK\n endif\n+endif\n \n ifdef SHA1_MAX_BLOCK_SIZE\n \tLIB_OBJS += compat/sha1-chunked.o\ndiff --git a/hash.h b/hash.h\nindex 9df562f2f6..9df06d56b4 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -17,6 +17,8 @@\n \n #if defined(SHA256_GCRYPT)\n #include \"sha256/gcrypt.h\"\n+#elif defined(SHA256_OPENSSL)\n+#include <openssl/sha.h>\n #else\n #include \"sha256/block/sha256.h\"\n #endif\n"},{"id":"360471","messageId":"20181015021900.1030041-14-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:19:00Z","receivedAt":"2018-10-15T02:19:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Since the commit-graph code wants to serialize the hash algorithm into\nthe data store, specify a version number for each supported algorithm.\nNote that we don't use the values of the constants themselves, as they\nare internal and could change in the future.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n commit-graph.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 7a28fbb03f..e587c21bb6 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n \n static uint8_t oid_version(void)\n {\n-\treturn 1;\n+\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n+\t\tcase GIT_HASH_SHA1:\n+\t\t\treturn 1;\n+\t\tcase GIT_HASH_SHA256:\n+\t\t\treturn 2;\n+\t\tdefault:\n+\t\t\tBUG(\"unknown hash algorithm\");\n+\t}\n }\n \n static struct commit_graph *alloc_commit_graph(void)\n"},{"id":"360472","messageId":"20181015021900.1030041-11-sandals@crustytoothpaste.net","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T02:18:57Z","receivedAt":"2018-10-15T02:19:28Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"SHA-1 is weak and we need to transition to a new hash function.  For\nsome time, we have referred to this new function as NewHash.  Recently,\nwe decided to pick SHA-256 as NewHash.\n\nAdd a basic implementation of SHA-256 based off libtomcrypt, which is in\nthe public domain.  Optimize it and restructure it to meet our coding\nstandards.  Place it in a directory called \"sha256\" where it and any\nfuture implementations can live so as to avoid a proliferation of\nimplementation directories.\n\nWire up SHA-256 in the list of hash algorithms, and add a test that the\nalgorithm works correctly.\n\nNote that with this patch, it is still not possible to switch to using\nSHA-256 in Git.  Additional patches are needed to prepare the code to\nhandle a larger hash algorithm and further test fixes are needed.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile               |   4 +\n cache.h                |  12 ++-\n hash.h                 |  19 ++++-\n sha1-file.c            |  45 +++++++++++\n sha256/block/sha256.c  | 180 +++++++++++++++++++++++++++++++++++++++++\n sha256/block/sha256.h  |  26 ++++++\n t/helper/test-sha256.c |   7 ++\n t/helper/test-tool.c   |   1 +\n t/helper/test-tool.h   |   1 +\n t/t0014-hash.sh        |  25 ++++++\n 10 files changed, 316 insertions(+), 4 deletions(-)\n create mode 100644 sha256/block/sha256.c\n create mode 100644 sha256/block/sha256.h\n create mode 100644 t/helper/test-sha256.c\n\ndiff --git a/Makefile b/Makefile\nindex 1c43bf9aa8..76d378c7ba 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -739,6 +739,7 @@ TEST_BUILTINS_OBJS += test-run-command.o\n TEST_BUILTINS_OBJS += test-scrap-cache-tree.o\n TEST_BUILTINS_OBJS += test-sha1.o\n TEST_BUILTINS_OBJS += test-sha1-array.o\n+TEST_BUILTINS_OBJS += test-sha256.o\n TEST_BUILTINS_OBJS += test-sigchain.o\n TEST_BUILTINS_OBJS += test-strcmp-offset.o\n TEST_BUILTINS_OBJS += test-string-list.o\n@@ -1633,6 +1634,9 @@ endif\n endif\n endif\n \n+LIB_OBJS += sha256/block/sha256.o\n+BASIC_CFLAGS += -DSHA256_BLK\n+\n ifdef SHA1_MAX_BLOCK_SIZE\n \tLIB_OBJS += compat/sha1-chunked.o\n \tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\ndiff --git a/cache.h b/cache.h\nindex 48e736b0d5..81ece0360e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -48,11 +48,17 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);\n /* The block size of SHA-1. */\n #define GIT_SHA1_BLKSZ 64\n \n+/* The length in bytes and in hex digits of an object name (SHA-256 value). */\n+#define GIT_SHA256_RAWSZ 32\n+#define GIT_SHA256_HEXSZ (2 * GIT_SHA256_RAWSZ)\n+/* The block size of SHA-256. */\n+#define GIT_SHA256_BLKSZ 64\n+\n /* The length in byte and in hex digits of the largest possible hash value. */\n-#define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ\n-#define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ\n+#define GIT_MAX_RAWSZ GIT_SHA256_RAWSZ\n+#define GIT_MAX_HEXSZ GIT_SHA256_HEXSZ\n /* The largest possible block size for any supported hash. */\n-#define GIT_MAX_BLKSZ GIT_SHA1_BLKSZ\n+#define GIT_MAX_BLKSZ GIT_SHA256_BLKSZ\n \n struct object_id {\n \tunsigned char hash[GIT_MAX_RAWSZ];\ndiff --git a/hash.h b/hash.h\nindex 46dff69eb3..88d18896d7 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -15,6 +15,8 @@\n #include \"block-sha1/sha1.h\"\n #endif\n \n+#include \"sha256/block/sha256.h\"\n+\n #ifndef platform_SHA_CTX\n /*\n  * platform's underlying implementation of SHA-1; could be OpenSSL,\n@@ -34,6 +36,18 @@\n #define git_SHA1_Update\t\tplatform_SHA1_Update\n #define git_SHA1_Final\t\tplatform_SHA1_Final\n \n+#ifndef platform_SHA256_CTX\n+#define platform_SHA256_CTX\tSHA256_CTX\n+#define platform_SHA256_Init\tSHA256_Init\n+#define platform_SHA256_Update\tSHA256_Update\n+#define platform_SHA256_Final\tSHA256_Final\n+#endif\n+\n+#define git_SHA256_CTX\t\tplatform_SHA256_CTX\n+#define git_SHA256_Init\t\tplatform_SHA256_Init\n+#define git_SHA256_Update\tplatform_SHA256_Update\n+#define git_SHA256_Final\tplatform_SHA256_Final\n+\n #ifdef SHA1_MAX_BLOCK_SIZE\n #include \"compat/sha1-chunked.h\"\n #undef git_SHA1_Update\n@@ -52,12 +66,15 @@\n #define GIT_HASH_UNKNOWN 0\n /* SHA-1 */\n #define GIT_HASH_SHA1 1\n+/* SHA-256  */\n+#define GIT_HASH_SHA256 2\n /* Number of algorithms supported (including unknown). */\n-#define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)\n+#define GIT_HASH_NALGOS (GIT_HASH_SHA256 + 1)\n \n /* A suitably aligned type for stack allocations of hash contexts. */\n union git_hash_ctx {\n \tgit_SHA_CTX sha1;\n+\tgit_SHA256_CTX sha256;\n };\n typedef union git_hash_ctx git_hash_ctx;\n \ndiff --git a/sha1-file.c b/sha1-file.c\nindex 9aadaf0c8d..66ba3dadb9 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -40,10 +40,20 @@\n #define EMPTY_TREE_SHA1_BIN_LITERAL \\\n \t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n \t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n+#define EMPTY_TREE_SHA256_BIN_LITERAL \\\n+\t\"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n+\t\"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n+\t\"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n+\t\"\\x53\\x21\"\n \n #define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n \t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n \t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n+#define EMPTY_BLOB_SHA256_BIN_LITERAL \\\n+\t\"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n+\t\"\\x67\\xe3\\xb1\\xe9\\xa7\\xdc\\xda\\x11\\x85\\x43\" \\\n+\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n+\t\"\\x18\\x13\"\n \n const unsigned char null_sha1[GIT_MAX_RAWSZ];\n const struct object_id null_oid;\n@@ -53,6 +63,12 @@ static const struct object_id empty_tree_oid = {\n static const struct object_id empty_blob_oid = {\n \tEMPTY_BLOB_SHA1_BIN_LITERAL\n };\n+static const struct object_id empty_tree_oid_sha256 = {\n+\tEMPTY_TREE_SHA256_BIN_LITERAL\n+};\n+static const struct object_id empty_blob_oid_sha256 = {\n+\tEMPTY_BLOB_SHA256_BIN_LITERAL\n+};\n \n static void git_hash_sha1_init(git_hash_ctx *ctx)\n {\n@@ -69,6 +85,22 @@ static void git_hash_sha1_final(unsigned char *hash, git_hash_ctx *ctx)\n \tgit_SHA1_Final(hash, &ctx->sha1);\n }\n \n+\n+static void git_hash_sha256_init(git_hash_ctx *ctx)\n+{\n+\tgit_SHA256_Init(&ctx->sha256);\n+}\n+\n+static void git_hash_sha256_update(git_hash_ctx *ctx, const void *data, size_t len)\n+{\n+\tgit_SHA256_Update(&ctx->sha256, data, len);\n+}\n+\n+static void git_hash_sha256_final(unsigned char *hash, git_hash_ctx *ctx)\n+{\n+\tgit_SHA256_Final(hash, &ctx->sha256);\n+}\n+\n static void git_hash_unknown_init(git_hash_ctx *ctx)\n {\n \tBUG(\"trying to init unknown hash\");\n@@ -110,6 +142,19 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\t&empty_tree_oid,\n \t\t&empty_blob_oid,\n \t},\n+\t{\n+\t\t\"sha256\",\n+\t\t/* \"s256\", big-endian */\n+\t\t0x73323536,\n+\t\tGIT_SHA256_RAWSZ,\n+\t\tGIT_SHA256_HEXSZ,\n+\t\tGIT_SHA256_BLKSZ,\n+\t\tgit_hash_sha256_init,\n+\t\tgit_hash_sha256_update,\n+\t\tgit_hash_sha256_final,\n+\t\t&empty_tree_oid_sha256,\n+\t\t&empty_blob_oid_sha256,\n+\t}\n };\n \n const char *empty_tree_oid_hex(void)\ndiff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\nnew file mode 100644\nindex 0000000000..18350c161a\n--- /dev/null\n+++ b/sha256/block/sha256.c\n@@ -0,0 +1,180 @@\n+#include \"git-compat-util.h\"\n+#include \"./sha256.h\"\n+\n+#define BLKSIZE blk_SHA256_BLKSIZE\n+\n+void blk_SHA256_Init(blk_SHA256_CTX *ctx)\n+{\n+\tctx->offset = 0;\n+\tctx->length = 0;\n+\tctx->state[0] = 0x6A09E667UL;\n+\tctx->state[1] = 0xBB67AE85UL;\n+\tctx->state[2] = 0x3C6EF372UL;\n+\tctx->state[3] = 0xA54FF53AUL;\n+\tctx->state[4] = 0x510E527FUL;\n+\tctx->state[5] = 0x9B05688CUL;\n+\tctx->state[6] = 0x1F83D9ABUL;\n+\tctx->state[7] = 0x5BE0CD19UL;\n+}\n+\n+static inline uint32_t ror(uint32_t x, unsigned n)\n+{\n+\treturn (x >> n) | (x << (32 - n));\n+}\n+\n+#define Ch(x,y,z)       (z ^ (x & (y ^ z)))\n+#define Maj(x,y,z)      (((x | y) & z) | (x & y))\n+#define S(x, n)         ror((x),(n))\n+#define R(x, n)         ((x)>>(n))\n+#define Sigma0(x)       (S(x, 2) ^ S(x, 13) ^ S(x, 22))\n+#define Sigma1(x)       (S(x, 6) ^ S(x, 11) ^ S(x, 25))\n+#define Gamma0(x)       (S(x, 7) ^ S(x, 18) ^ R(x, 3))\n+#define Gamma1(x)       (S(x, 17) ^ S(x, 19) ^ R(x, 10))\n+\n+static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char *buf)\n+{\n+\n+\tuint32_t S[8], W[64], t0, t1;\n+\tint i;\n+\n+\t/* copy state into S */\n+\tfor (i = 0; i < 8; i++) {\n+\t\tS[i] = ctx->state[i];\n+\t}\n+\n+\t/* copy the state into 512-bits into W[0..15] */\n+\tfor (i = 0; i < 16; i++, buf += sizeof(uint32_t)) {\n+\t\tW[i] = get_be32(buf);\n+\t}\n+\n+\t/* fill W[16..63] */\n+\tfor (i = 16; i < 64; i++) {\n+\t\tW[i] = Gamma1(W[i - 2]) + W[i - 7] + Gamma0(W[i - 15]) + W[i - 16];\n+\t}\n+\n+#define RND(a,b,c,d,e,f,g,h,i,ki)                    \\\n+\tt0 = h + Sigma1(e) + Ch(e, f, g) + ki + W[i];   \\\n+\tt1 = Sigma0(a) + Maj(a, b, c);                  \\\n+\td += t0;                                        \\\n+\th  = t0 + t1;\n+\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],0,0x428a2f98);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],1,0x71374491);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],2,0xb5c0fbcf);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],3,0xe9b5dba5);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],4,0x3956c25b);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],5,0x59f111f1);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],6,0x923f82a4);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],7,0xab1c5ed5);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],8,0xd807aa98);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],9,0x12835b01);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],10,0x243185be);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],11,0x550c7dc3);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],12,0x72be5d74);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],13,0x80deb1fe);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],14,0x9bdc06a7);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],15,0xc19bf174);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],16,0xe49b69c1);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],17,0xefbe4786);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],18,0x0fc19dc6);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],19,0x240ca1cc);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],20,0x2de92c6f);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],21,0x4a7484aa);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],22,0x5cb0a9dc);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],23,0x76f988da);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],24,0x983e5152);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],25,0xa831c66d);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],26,0xb00327c8);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],27,0xbf597fc7);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],28,0xc6e00bf3);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],29,0xd5a79147);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],30,0x06ca6351);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],31,0x14292967);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],32,0x27b70a85);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],33,0x2e1b2138);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],34,0x4d2c6dfc);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],35,0x53380d13);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],36,0x650a7354);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],37,0x766a0abb);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],38,0x81c2c92e);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],39,0x92722c85);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],40,0xa2bfe8a1);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],41,0xa81a664b);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],42,0xc24b8b70);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],43,0xc76c51a3);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],44,0xd192e819);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],45,0xd6990624);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],46,0xf40e3585);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],47,0x106aa070);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],48,0x19a4c116);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],49,0x1e376c08);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],50,0x2748774c);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],51,0x34b0bcb5);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],52,0x391c0cb3);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],53,0x4ed8aa4a);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],54,0x5b9cca4f);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],55,0x682e6ff3);\n+\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],56,0x748f82ee);\n+\tRND(S[7],S[0],S[1],S[2],S[3],S[4],S[5],S[6],57,0x78a5636f);\n+\tRND(S[6],S[7],S[0],S[1],S[2],S[3],S[4],S[5],58,0x84c87814);\n+\tRND(S[5],S[6],S[7],S[0],S[1],S[2],S[3],S[4],59,0x8cc70208);\n+\tRND(S[4],S[5],S[6],S[7],S[0],S[1],S[2],S[3],60,0x90befffa);\n+\tRND(S[3],S[4],S[5],S[6],S[7],S[0],S[1],S[2],61,0xa4506ceb);\n+\tRND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],62,0xbef9a3f7);\n+\tRND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],63,0xc67178f2);\n+\n+#undef RND\n+\n+\tfor (i = 0; i < 8; i++) {\n+\t\tctx->state[i] = ctx->state[i] + S[i];\n+\t}\n+}\n+\n+#define MIN(x, y) ((x) < (y) ? (x) : (y))\n+void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n+{\n+\tconst unsigned char *in = data;\n+\tsize_t n;\n+\tctx->length += len;\n+\twhile (len > 0) {\n+\t\tif (!ctx->offset && len >= BLKSIZE) {\n+\t\t\tblk_SHA256_Transform(ctx, in);\n+\t\t\tin += BLKSIZE;\n+\t\t\tlen -= BLKSIZE;\n+\t\t} else {\n+\t\t\tn = MIN(len, (BLKSIZE - ctx->offset));\n+\t\t\tmemcpy(ctx->buf + ctx->offset, in, n);\n+\t\t\tctx->offset += n;\n+\t\t\tin += n;\n+\t\t\tlen -= n;\n+\t\t\tif (ctx->offset == BLKSIZE) {\n+\t\t\t\tblk_SHA256_Transform(ctx, ctx->buf);\n+\t\t\t\tctx->offset = 0;\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n+void blk_SHA256_Final(unsigned char *digest, blk_SHA256_CTX *ctx)\n+{\n+\tconst unsigned trip = BLKSIZE - sizeof(ctx->length);\n+\tint i;\n+\n+\tctx->length <<= 3;\n+\tctx->buf[ctx->offset++] = 0x80;\n+\n+\tif (ctx->offset > trip) {\n+\t\tmemset(ctx->buf + ctx->offset, 0, BLKSIZE - ctx->offset);\n+\t\tblk_SHA256_Transform(ctx, ctx->buf);\n+\t\tctx->offset = 0;\n+\t}\n+\n+\tmemset(ctx->buf + ctx->offset, 0, BLKSIZE - ctx->offset - sizeof(ctx->length));\n+\n+\tput_be64(ctx->buf + trip, ctx->length);\n+\tblk_SHA256_Transform(ctx, ctx->buf);\n+\n+\t/* copy output */\n+\tfor (i = 0; i < 8; i++, digest += sizeof(uint32_t))\n+\t\tput_be32(digest, ctx->state[i]);\n+}\ndiff --git a/sha256/block/sha256.h b/sha256/block/sha256.h\nnew file mode 100644\nindex 0000000000..ad8b178ad3\n--- /dev/null\n+++ b/sha256/block/sha256.h\n@@ -0,0 +1,26 @@\n+#ifndef SHA256_BLOCK_SHA256_H\n+#define SHA256_BLOCK_SHA256_H\n+\n+#include \"git-compat-util.h\"\n+\n+#define blk_SHA256_BLKSIZE 64\n+\n+struct blk_SHA256_CTX {\n+\tuint32_t state[8];\n+\tuint64_t length;\n+\tuint32_t offset;\n+\tuint8_t buf[blk_SHA256_BLKSIZE];\n+};\n+\n+typedef struct blk_SHA256_CTX blk_SHA256_CTX;\n+\n+void blk_SHA256_Init(blk_SHA256_CTX *ctx);\n+void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len);\n+void blk_SHA256_Final(unsigned char *digest, blk_SHA256_CTX *ctx);\n+\n+#define platform_SHA256_CTX blk_SHA256_CTX\n+#define platform_SHA256_Init blk_SHA256_Init\n+#define platform_SHA256_Update blk_SHA256_Update\n+#define platform_SHA256_Final blk_SHA256_Final\n+\n+#endif\ndiff --git a/t/helper/test-sha256.c b/t/helper/test-sha256.c\nnew file mode 100644\nindex 0000000000..0ac6a99d5f\n--- /dev/null\n+++ b/t/helper/test-sha256.c\n@@ -0,0 +1,7 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+\n+int cmd__sha256(int ac, const char **av)\n+{\n+\treturn cmd_hash_impl(ac, av, GIT_HASH_SHA256);\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex e009c8186d..2a65193514 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -43,6 +43,7 @@ static struct test_cmd cmds[] = {\n \t{ \"scrap-cache-tree\", cmd__scrap_cache_tree },\n \t{ \"sha1\", cmd__sha1 },\n \t{ \"sha1-array\", cmd__sha1_array },\n+\t{ \"sha256\", cmd__sha256 },\n \t{ \"sigchain\", cmd__sigchain },\n \t{ \"strcmp-offset\", cmd__strcmp_offset },\n \t{ \"string-list\", cmd__string_list },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 19a7e8332a..2e66a8e47b 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -39,6 +39,7 @@ int cmd__run_command(int argc, const char **argv);\n int cmd__scrap_cache_tree(int argc, const char **argv);\n int cmd__sha1(int argc, const char **argv);\n int cmd__sha1_array(int argc, const char **argv);\n+int cmd__sha256(int argc, const char **argv);\n int cmd__sigchain(int argc, const char **argv);\n int cmd__strcmp_offset(int argc, const char **argv);\n int cmd__string_list(int argc, const char **argv);\ndiff --git a/t/t0014-hash.sh b/t/t0014-hash.sh\nindex 8e763c2c3d..f8e639743f 100755\n--- a/t/t0014-hash.sh\n+++ b/t/t0014-hash.sh\n@@ -26,4 +26,29 @@ test_expect_success 'test basic SHA-1 hash values' '\n \tgrep 4b825dc642cb6eb9a060e54bf8d69288fbee4904 actual\n '\n \n+test_expect_success 'test basic SHA-256 hash values' '\n+\ttest-tool sha256 </dev/null >actual &&\n+\tgrep e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 actual &&\n+\tprintf \"a\" | test-tool sha256 >actual &&\n+\tgrep ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb actual &&\n+\tprintf \"abc\" | test-tool sha256 >actual &&\n+\tgrep ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad actual &&\n+\tprintf \"message digest\" | test-tool sha256 >actual &&\n+\tgrep f7846f55cf23e14eebeab5b4e1550cad5b509e3348fbc4efa3a1413d393cb650 actual &&\n+\tprintf \"abcdefghijklmnopqrstuvwxyz\" | test-tool sha256 >actual &&\n+\tgrep 71c480df93d6ae2f1efad1447c66c9525e316218cf51fc8d9ed832f2daf18b73 actual &&\n+\tperl -E \"for (1..100000) { print q{aaaaaaaaaa}; }\" | \\\n+\t\ttest-tool sha256 >actual &&\n+\tgrep cdc76e5c9914fb9281a1c7e284d73e67f1809a48a497200e046d39ccc7112cd0 actual &&\n+\tperl -E \"for (1..100000) { print q{abcdefghijklmnopqrstuvwxyz}; }\" | \\\n+\t\ttest-tool sha256 >actual &&\n+\tgrep e406ba321ca712ad35a698bf0af8d61fc4dc40eca6bdcea4697962724ccbde35 actual &&\n+\tprintf \"blob 0\\0\" | test-tool sha256 >actual &&\n+\tgrep 473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813 actual &&\n+\tprintf \"blob 3\\0abc\" | test-tool sha256 >actual &&\n+\tgrep c1cf6e465077930e88dc5136641d402f72a229ddd996f627d60e9639eaba35a6 actual &&\n+\tprintf \"tree 0\\0\" | test-tool sha256 >actual &&\n+\tgrep 6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321 actual\n+'\n+\n test_done\n"},{"id":"360530","messageId":"CACsJy8AOu-SdLu+MQKOzVLEy0SM0r45TjwZYkD_S5W756hWRpw@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-11-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-15T14:59:12Z","receivedAt":"2018-10-15T14:59:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":" On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> SHA-1 is weak and we need to transition to a new hash function.  For\n> some time, we have referred to this new function as NewHash.  Recently,\n> we decided to pick SHA-256 as NewHash.\n>\n> Add a basic implementation of SHA-256 based off libtomcrypt, which is in\n> the public domain.  Optimize it and restructure it to meet our coding\n> standards.  Place it in a directory called \"sha256\" where it and any\n> future implementations can live so as to avoid a proliferation of\n> implementation directories.\n>\n> Wire up SHA-256 in the list of hash algorithms, and add a test that the\n> algorithm works correctly.\n>\n> Note that with this patch, it is still not possible to switch to using\n> SHA-256 in Git.  Additional patches are needed to prepare the code to\n> handle a larger hash algorithm and further test fixes are needed.\n\nAt some point I assume SHA-256 will become functional and be part of a\ngit release without all file formats updated to support multiple\nhashes. Should we somehow discourage the user from using it because it\nwill break when all file formats are finally updated?\n\nThe simplest way is to just not register \"sha256\" in hash_algos unless\nsome developer flag is set. Or rename sha256 to sha256-experimental or\nsomething to make it more obvious (but then we may need to fix up the\ntest suite after renaming it back to sha256, not great)\n-- \nDuy\n"},{"id":"360534","messageId":"9dfd76b3-d706-9844-35e3-3a2318181a7c@gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-10-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 09/13] commit-graph: convert to using the_hash_algo","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-15T15:10:32Z","receivedAt":"2018-10-15T15:10:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2018 10:18 PM, brian m. carlson wrote:\n> Instead of using hard-coded constants for object sizes, use\n> the_hash_algo to look them up.  In addition, use a function call to look\n> up the object ID version and produce the correct value.\nThis looks good and I can see already how this new organization will \nhelp make the hash size more flexible.\n\nThanks!\n-Stolee\n\n"},{"id":"360535","messageId":"a0a4bf24-ce6d-69e1-4485-8742c538304d@gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-14-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-15T15:11:53Z","receivedAt":"2018-10-15T15:11:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2018 10:19 PM, brian m. carlson wrote:\n> Since the commit-graph code wants to serialize the hash algorithm into\n> the data store, specify a version number for each supported algorithm.\n> Note that we don't use the values of the constants themselves, as they\n> are internal and could change in the future.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>   commit-graph.c | 9 ++++++++-\n>   1 file changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 7a28fbb03f..e587c21bb6 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n>   \n>   static uint8_t oid_version(void)\n>   {\n> -\treturn 1;\n> +\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n> +\t\tcase GIT_HASH_SHA1:\n> +\t\t\treturn 1;\n> +\t\tcase GIT_HASH_SHA256:\n> +\t\t\treturn 2;\n> +\t\tdefault:\n> +\t\t\tBUG(\"unknown hash algorithm\");\n> +\t}\n>   }\n>   \n>   static struct commit_graph *alloc_commit_graph(void)\nSimple and easy!\n\nThanks,\n-Stolee\n"},{"id":"360559","messageId":"20181015233035.GB432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"CACsJy8AOu-SdLu+MQKOzVLEy0SM0r45TjwZYkD_S5W756hWRpw@mail.gmail.com","subject":"Re: [PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-15T23:30:36Z","receivedAt":"2018-10-15T23:30:45Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Oct 15, 2018 at 04:59:12PM +0200, Duy Nguyen wrote:\n>  On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > SHA-1 is weak and we need to transition to a new hash function.  For\n> > some time, we have referred to this new function as NewHash.  Recently,\n> > we decided to pick SHA-256 as NewHash.\n> >\n> > Add a basic implementation of SHA-256 based off libtomcrypt, which is in\n> > the public domain.  Optimize it and restructure it to meet our coding\n> > standards.  Place it in a directory called \"sha256\" where it and any\n> > future implementations can live so as to avoid a proliferation of\n> > implementation directories.\n> >\n> > Wire up SHA-256 in the list of hash algorithms, and add a test that the\n> > algorithm works correctly.\n> >\n> > Note that with this patch, it is still not possible to switch to using\n> > SHA-256 in Git.  Additional patches are needed to prepare the code to\n> > handle a larger hash algorithm and further test fixes are needed.\n> \n> At some point I assume SHA-256 will become functional and be part of a\n> git release without all file formats updated to support multiple\n> hashes. Should we somehow discourage the user from using it because it\n> will break when all file formats are finally updated?\n\nIn order to activate SHA-256 in the codebase, currently you need a patch\nto force it on.  Otherwise, the code is simply inert and does nothing\n(other than in the test-tool).  I've included the patch below so you can\nsee what it does (or if you want to play around with it).\n\nWithout this patch, Git remains fully SHA-1 and can't access any of the\nSHA-256 code.  I have some very preliminary patches that do wire up\nextensions.objectFormat (branch object-id-part15 [sic]) but I haven't\npicked them up in a while.  (I need to finish test fixes first.)\n\nMaking the codebase run in SHA-256 mode with a Git that supports both\nalgorithms but defaults to SHA-1 is currently seriously broken, even\nwith the object-id-part15 branch.  When those patches go in, they will\nbe behind a developer flag to prevent wholesale chaos and segfaults.\n\nSo in summary, no, I don't think a developer flag is necessary at this\npoint.\n\n-- >% --\nFrom 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001\nFrom: \"brian m. carlson\" <sandals@crustytoothpaste.net>\nDate: Wed, 25 Jul 2018 23:48:30 +0000\nSubject: [PATCH] Switch default hash algorithm to SHA-256 for testing\n\nSet the default hash algorithm to SHA-256 for testing purposes.\n\nThis is a test commit and should not be used in production.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n repository.c            |  2 +-\n setup.c                 |  2 +-\n t/test-lib-functions.sh |  4 +---\n t/test-lib.sh           | 13 ++++++++-----\n 4 files changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex 5dd1486718..4ce50ea670 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -17,7 +17,7 @@ void initialize_the_repository(void)\n \tthe_repo.objects = raw_object_store_new();\n \tthe_repo.parsed_objects = parsed_object_pool_new();\n \n-\trepo_set_hash_algo(&the_repo, GIT_HASH_SHA1);\n+\trepo_set_hash_algo(&the_repo, GIT_HASH_SHA256);\n }\n \n static void expand_base_dir(char **out, const char *in,\ndiff --git a/setup.c b/setup.c\nindex b24c811c1c..2b4cc4e5a4 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -490,7 +490,7 @@ int read_repository_format(struct repository_format *format, const char *path)\n \tmemset(format, 0, sizeof(*format));\n \tformat->version = -1;\n \tformat->is_bare = -1;\n-\tformat->hash_algo = GIT_HASH_SHA1;\n+\tformat->hash_algo = GIT_HASH_SHA256;\n \tstring_list_init(&format->unknown_extensions, 1);\n \tgit_config_from_file(check_repo_format, path, format);\n \treturn format->version;\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex dfa1bebb46..91cf2b9d40 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1165,9 +1165,7 @@ test_set_hash () {\n \n # Detect the hash algorithm in use.\n test_detect_hash () {\n-\t# Currently we only support SHA-1, but in the future this function will\n-\t# actually detect the algorithm in use.\n-\ttest_hash_algo='sha1'\n+\ttest_hash_algo='sha256'\n }\n \n # Load common hash metadata and common placeholder object IDs for use with\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3f95bfda60..bb507f2d4f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -176,18 +176,21 @@ esac\n \n # Convenience\n #\n-# A regexp to match 5, 35 and 40 hexdigits\n+# A regexp to match 4, 5, 35, 40, and 64 hexdigits\n+_x04='[0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n _x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n _x35=\"$_x05$_x05$_x05$_x05$_x05$_x05$_x05\"\n _x40=\"$_x35$_x05\"\n+_x64=\"$_x40$_x05$_x05$_x05$_x05$_x04\"\n \n # Zero SHA-1\n _z40=0000000000000000000000000000000000000000\n+_z64=0000000000000000000000000000000000000000000000000000000000000000\n \n-OID_REGEX=\"$_x40\"\n-ZERO_OID=$_z40\n-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n-EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\n+OID_REGEX=\"$_x64\"\n+ZERO_OID=$_z64\n+EMPTY_TREE=6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321\n+EMPTY_BLOB=473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813\n \n # Line feed\n LF='\n-- >% --\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360563","messageId":"xmqqo9bu4pww.fsf@gitster-ct.c.googlers.com","threadId":"49566","inReplyTo":"20181015021900.1030041-4-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 03/13] hex: introduce functions to print arbitrary hashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T01:54:23Z","receivedAt":"2018-10-16T01:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> diff --git a/cache.h b/cache.h\n> index d508f3d4f8..a13d14ce0a 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1361,9 +1361,9 @@ extern int get_oid_hex(const char *hex, struct object_id *sha1);\n>  extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n>  \n>  /*\n> - * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant,\n> + * Convert a binary hash to its hex equivalent. The `_r` variant is reentrant,\n>   * and writes the NUL-terminated output to the buffer `out`, which must be at\n> - * least `GIT_SHA1_HEXSZ + 1` bytes, and returns a pointer to out for\n> + * least `GIT_MAX_HEXSZ + 1` bytes, and returns a pointer to out for\n>   * convenience.\n>   *\n>   * The non-`_r` variant returns a static buffer, but uses a ring of 4\n> @@ -1371,10 +1371,13 @@ extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n>   *\n>   *   printf(\"%s -> %s\", sha1_to_hex(one), sha1_to_hex(two));\n>   */\n> -extern char *sha1_to_hex_r(char *out, const unsigned char *sha1);\n> -extern char *oid_to_hex_r(char *out, const struct object_id *oid);\n> -extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n> -extern char *oid_to_hex(const struct object_id *oid);\t/* same static buffer as sha1_to_hex */\n> +char *hash_to_hex_algo_r(char *buffer, const unsigned char *hash, int algo);\n> +char *sha1_to_hex_r(char *out, const unsigned char *sha1);\n> +char *oid_to_hex_r(char *out, const struct object_id *oid);\n> +char *hash_to_hex_algo(const unsigned char *hash, int algo);\t/* static buffer result! */\n> +char *sha1_to_hex(const unsigned char *sha1);\t\t\t/* same static buffer */\n> +char *hash_to_hex(const unsigned char *hash);\t\t\t/* same static buffer */\n> +char *oid_to_hex(const struct object_id *oid);\t\t\t/* same static buffer */\n\nEven though in hex.c I see mixture of *_algo and *_algop helper\nfunctions, I see only \"algo\" variants above.  Is it our official\nstance to use primarily the integer index into the algo array when\nspecifying the hash, and when a caller into 'multi-hash' API happens\nto have other things, it should use functions in 2/13 to convert it\nto the canonical \"int algo\" beforehand?\n\nI am not saying it is bad or good to choose the index into the algo\narray as the primary way to specify the algorithm.  I think it is a\ngood idea to pick one and stick to it, and I wanted to make sure\nthat the choice we made here is clearly communicated to any future\ndeveloper who read this code.\n\n> +char *sha1_to_hex(const unsigned char *sha1)\n> +{\n> +\treturn hash_to_hex_algo(sha1, GIT_HASH_SHA1);\n> +}\n> +\n> +char *hash_to_hex(const unsigned char *hash)\n> +{\n> +\treturn hash_to_hex_algo(hash, hash_algo_by_ptr(the_hash_algo));\n>  }\n>  \n>  char *oid_to_hex(const struct object_id *oid)\n>  {\n> -\treturn sha1_to_hex(oid->hash);\n> +\treturn hash_to_hex_algo(oid->hash, hash_algo_by_ptr(the_hash_algo));\n>  }\n\nHaving said the above, seeing the use of hash_algo_by_ptr() here\nmakes me suspect if it makes more sense to use the algop as the\nprimary way to specify which algorithm the caller wants to use.\nIOW, making the set of helpers in 02/13 to allow quering by name,\nformat-id, or the integer index and have them all return a pointer\nto \"const struct git_hash_algo\".  Two immediate downsides I can see\nis that it exposes the actual structure to the callers (but is it\nreally a problem?  Outside callers learn hash sizes etc. by accessing\nits fields anyway without treating the algo struct as opaque.), and\npassing an 8-byte pointer may be more costly than passing a small\ninteger index that ranges between 0 and 1 at most (assuming that\nwe'd only use SHA-1 and \"the current NewHash\" in the code).\n\n\n"},{"id":"360564","messageId":"xmqqk1mi4pn0.fsf@gitster-ct.c.googlers.com","threadId":"49566","inReplyTo":"20181015021900.1030041-14-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T02:00:19Z","receivedAt":"2018-10-16T02:00:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> Since the commit-graph code wants to serialize the hash algorithm into\n> the data store, specify a version number for each supported algorithm.\n> Note that we don't use the values of the constants themselves, as they\n> are internal and could change in the future.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  commit-graph.c | 9 ++++++++-\n>  1 file changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 7a28fbb03f..e587c21bb6 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n>  \n>  static uint8_t oid_version(void)\n>  {\n> -\treturn 1;\n> +\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n> +\t\tcase GIT_HASH_SHA1:\n> +\t\t\treturn 1;\n> +\t\tcase GIT_HASH_SHA256:\n> +\t\t\treturn 2;\n> +\t\tdefault:\n> +\t\t\tBUG(\"unknown hash algorithm\");\n> +\t}\n\nStyle: align switch/case.\n\n\nShouldn't this be just using GIT_HASH_* constants instead?  Are we\nexpecting that it would be benefitial to allow more than one \"oid\nversion\" even within a same GIT_HASH_*?\n\n>  }\n>  \n>  static struct commit_graph *alloc_commit_graph(void)\n"},{"id":"360565","messageId":"xmqqd0sa4pmg.fsf@gitster-ct.c.googlers.com","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 00/13] Base SHA-256 implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T02:00:39Z","receivedAt":"2018-10-16T02:00:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I realize the increase to GIT_MAX_HEXSZ will result in an increase in\n> memory usage, but we're going to need to do it at some point, and the\n> sooner the code is in the codebase, the sooner people can play around\n> with it and test it.\n\nThanks.\n"},{"id":"360574","messageId":"xmqq7eii35h8.fsf@gitster-ct.c.googlers.com","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 00/13] Base SHA-256 implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T04:01:07Z","receivedAt":"2018-10-16T04:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n>  t/t0014-hash.sh                       |  54 ++++++++\n>  create mode 100755 t/t0014-hash.sh\n\nIf I am not mistaken, 0014 is already used by another topic in\nflight, and will cause test-lint failure on 'pu'.\n\n"},{"id":"360627","messageId":"CACsJy8D_9cwE_Xf7d3rWj_aq8uNUY0wpz8ydLLgvdpV0rQYbrg@mail.gmail.com","threadId":"49566","inReplyTo":"20181015233035.GB432229@genre.crustytoothpaste.net","subject":"Re: [PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T14:59:23Z","receivedAt":"2018-10-16T14:59:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 16, 2018 at 1:31 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On Mon, Oct 15, 2018 at 04:59:12PM +0200, Duy Nguyen wrote:\n> >  On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> > >\n> > > SHA-1 is weak and we need to transition to a new hash function.  For\n> > > some time, we have referred to this new function as NewHash.  Recently,\n> > > we decided to pick SHA-256 as NewHash.\n> > >\n> > > Add a basic implementation of SHA-256 based off libtomcrypt, which is in\n> > > the public domain.  Optimize it and restructure it to meet our coding\n> > > standards.  Place it in a directory called \"sha256\" where it and any\n> > > future implementations can live so as to avoid a proliferation of\n> > > implementation directories.\n> > >\n> > > Wire up SHA-256 in the list of hash algorithms, and add a test that the\n> > > algorithm works correctly.\n> > >\n> > > Note that with this patch, it is still not possible to switch to using\n> > > SHA-256 in Git.  Additional patches are needed to prepare the code to\n> > > handle a larger hash algorithm and further test fixes are needed.\n> >\n> > At some point I assume SHA-256 will become functional and be part of a\n> > git release without all file formats updated to support multiple\n> > hashes. Should we somehow discourage the user from using it because it\n> > will break when all file formats are finally updated?\n>\n> In order to activate SHA-256 in the codebase, currently you need a patch\n> to force it on.  Otherwise, the code is simply inert and does nothing\n> (other than in the test-tool).  I've included the patch below so you can\n> see what it does (or if you want to play around with it).\n>\n> Without this patch, Git remains fully SHA-1 and can't access any of the\n> SHA-256 code.  I have some very preliminary patches that do wire up\n> extensions.objectFormat (branch object-id-part15 [sic]) but I haven't\n> picked them up in a while.  (I need to finish test fixes first.)\n\nAh, I thought that extensions.objectFormat and setup changes already\nlanded (I think I saw that series on this list). Sorry for the noise.\n\n-- \nDuy\n"},{"id":"360631","messageId":"CACsJy8DPBkRyPYF7Vfh0bQHJDvDZcmp54wjzRxG4Ho40SkMTSg@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 01/13] sha1-file: rename algorithm to \"sha1\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T15:17:53Z","receivedAt":"2018-10-16T15:18:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2018 at 4:21 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> The transition plan anticipates us using a syntax such as \"^{sha1}\" for\n> disambiguation.  Since this is a syntax some people will be typing a\n> lot, it makes sense to provide a short, easy-to-type syntax.  Omitting\n> the dash doesn't create any ambiguity, but it does make it shorter and\n\n\"but\" or \"and\"? I think both clauses are on the same side ... or did\nyou mean omitting the dash does create ambiguity?\n\n> easier to type, especially for touch typists.  In addition, the\n> transition plan already uses \"sha1\" in this context.\n>\n> Rename the name of SHA-1 implementation to \"sha1\".\n>\n> Note that this change creates no backwards compatibility concerns, since\n> we haven't yet used this field in any serialized data formats.\n\nBut we're not going to use this _string_ in any data format either\nbecause we'll stick to format_id field anyway, right?\n-- \nDuy\n"},{"id":"360635","messageId":"CACsJy8C6o9PAHsm+t7Rijg6jsz5dXZ5pT79nYGdGbKuK6gGTLQ@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-14-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T15:35:05Z","receivedAt":"2018-10-16T15:35:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> Since the commit-graph code wants to serialize the hash algorithm into\n> the data store, specify a version number for each supported algorithm.\n> Note that we don't use the values of the constants themselves, as they\n> are internal and could change in the future.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  commit-graph.c | 9 ++++++++-\n>  1 file changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 7a28fbb03f..e587c21bb6 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n>\n>  static uint8_t oid_version(void)\n>  {\n> -       return 1;\n> +       switch (hash_algo_by_ptr(the_hash_algo)) {\n> +               case GIT_HASH_SHA1:\n> +                       return 1;\n> +               case GIT_HASH_SHA256:\n> +                       return 2;\n\nShould we just increase this field to uint32_t and store format_id\ninstead? That will keep oid version unique in all data formats.\n\n> +               default:\n> +                       BUG(\"unknown hash algorithm\");\n> +       }\n>  }\n>\n>  static struct commit_graph *alloc_commit_graph(void)\n-- \nDuy\n"},{"id":"360636","messageId":"CACsJy8B3cK7VUfRYBa+Tqnke6gJ6FT9u+AsffAcGJQhQ5wcjvQ@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-13-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 12/13] hash: add an SHA-256 implementation using OpenSSL","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T15:36:40Z","receivedAt":"2018-10-16T15:37:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2018 at 4:22 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> We already have OpenSSL routines available for SHA-1, so add routines\n> for SHA-256 as well.\n\nSince we have \"hash-speed\" tool now, it would be great to keep some\nnumbers of these hash implementations in the commit message (and maybe\nsha1 as well just for comparison).\n\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  Makefile | 7 +++++++\n>  hash.h   | 2 ++\n>  2 files changed, 9 insertions(+)\n>\n> diff --git a/Makefile b/Makefile\n> index 3d91555a81..3164e2aeee 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -183,6 +183,8 @@ all::\n>  #\n>  # Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt.\n>  #\n> +# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.\n> +#\n>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n>  #\n>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n> @@ -1638,6 +1640,10 @@ endif\n>  endif\n>  endif\n>\n> +ifdef OPENSSL_SHA256\n> +       EXTLIBS += $(LIB_4_CRYPTO)\n> +       BASIC_CFLAGS += -DSHA256_OPENSSL\n> +else\n>  ifdef GCRYPT_SHA256\n>         BASIC_CFLAGS += -DSHA256_GCRYPT\n>         EXTLIBS += -lgcrypt\n> @@ -1645,6 +1651,7 @@ else\n>         LIB_OBJS += sha256/block/sha256.o\n>         BASIC_CFLAGS += -DSHA256_BLK\n>  endif\n> +endif\n>\n>  ifdef SHA1_MAX_BLOCK_SIZE\n>         LIB_OBJS += compat/sha1-chunked.o\n> diff --git a/hash.h b/hash.h\n> index 9df562f2f6..9df06d56b4 100644\n> --- a/hash.h\n> +++ b/hash.h\n> @@ -17,6 +17,8 @@\n>\n>  #if defined(SHA256_GCRYPT)\n>  #include \"sha256/gcrypt.h\"\n> +#elif defined(SHA256_OPENSSL)\n> +#include <openssl/sha.h>\n>  #else\n>  #include \"sha256/block/sha256.h\"\n>  #endif\n\n\n\n-- \nDuy\n"},{"id":"360637","messageId":"CACsJy8DV2XKy5hAVz9r7f-kGnwmGPfN2JHRG_9UPmpSopd0rtw@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 00/13] Base SHA-256 implementation","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T15:39:21Z","receivedAt":"2018-10-16T15:39:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2018 at 4:21 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> This series provides an actual SHA-256 implementation and wires it up,\n> along with some housekeeping patches to make it suitable for testing.\n>\n> New in this version is a patch which reverts the change to limit hashcmp\n> to 20 bytes.  I've taken care to permit the compiler to inline as much\n> as possible for efficiency, but it would be helpful to see what the\n> performance impact of these changes is on real-life workflows, or with\n> MSVC and other non-GCC and non-clang compilers.  The resulting change is\n> uglier and more duplicative than I wanted, but oh, well.\n>\n> I didn't attempt to pull in the full complement of code from libtomcrypt\n> to try to show the changes made, since that would have involved\n> importing a significant quantity of code in order to make things work.\n>\n> I realize the increase to GIT_MAX_HEXSZ will result in an increase in\n> memory usage, but we're going to need to do it at some point, and the\n> sooner the code is in the codebase, the sooner people can play around\n> with it and test it.\n>\n> This piece should be the final piece of preparatory work required for a\n> fully functioning SHA-256-only Git.  Additional work should be able to\n> come into the testsuite and codebase without needing to build on work in\n> any series after this one.\n\nThanks again for keeping working on this. I'm still sick and can't\nreally do a deep review. With that in mind, the patches look good.\n-- \nDuy\n"},{"id":"360638","messageId":"CACsJy8Cyep229CH8OfcVRqxC4soRiiCDhE_FMPhiEzVf-Z9v+Q@mail.gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-5-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 04/13] cache: make hashcmp and hasheq work with larger hashes","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T15:44:47Z","receivedAt":"2018-10-16T15:45:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2018 at 4:21 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> diff --git a/cache.h b/cache.h\n> index a13d14ce0a..0b88c3a344 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1024,16 +1024,12 @@ extern const struct object_id null_oid;\n>  static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)\n>  {\n>         /*\n> -        * This is a temporary optimization hack. By asserting the size here,\n> -        * we let the compiler know that it's always going to be 20, which lets\n> -        * it turn this fixed-size memcmp into a few inline instructions.\n> -        *\n> -        * This will need to be extended or ripped out when we learn about\n> -        * hashes of different sizes.\n> +        * Teach the compiler that there are only two possibilities of hash size\n> +        * here, so that it can optimize for this case as much as possible.\n>          */\n> -       if (the_hash_algo->rawsz != 20)\n> -               BUG(\"hash size not yet supported by hashcmp\");\n> -       return memcmp(sha1, sha2, the_hash_algo->rawsz);\n> +       if (the_hash_algo->rawsz == GIT_MAX_RAWSZ)\n\nIt's tangent. But performance is probably another good reason to\ndetach the_hash_algo from the_repository so we have one less\ndereference to do. (the other good reason is these hash operations\nshould work in \"no-repo\" commands as well, where the_repository does\nnot really make sense).\n\n> +               return memcmp(sha1, sha2, GIT_MAX_RAWSZ);\n> +       return memcmp(sha1, sha2, GIT_SHA1_RAWSZ);\n>  }\n>\n>  static inline int oidcmp(const struct object_id *oid1, const struct object_id *oid2)\n> @@ -1043,7 +1039,13 @@ static inline int oidcmp(const struct object_id *oid1, const struct object_id *o\n>\n>  static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2)\n>  {\n> -       return !hashcmp(sha1, sha2);\n> +       /*\n> +        * We write this here instead of deferring to hashcmp so that the\n> +        * compiler can properly inline it and avoid calling memcmp.\n> +        */\n> +       if (the_hash_algo->rawsz == GIT_MAX_RAWSZ)\n> +               return !memcmp(sha1, sha2, GIT_MAX_RAWSZ);\n> +       return !memcmp(sha1, sha2, GIT_SHA1_RAWSZ);\n>  }\n>\n>  static inline int oideq(const struct object_id *oid1, const struct object_id *oid2)\n\n\n\n-- \nDuy\n"},{"id":"360639","messageId":"fbd59390-42ee-8c06-74b7-1e7081680913@gmail.com","threadId":"49566","inReplyTo":"CACsJy8C6o9PAHsm+t7Rijg6jsz5dXZ5pT79nYGdGbKuK6gGTLQ@mail.gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-16T16:01:12Z","receivedAt":"2018-10-16T16:01:17Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/16/2018 11:35 AM, Duy Nguyen wrote:\n> On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>> Since the commit-graph code wants to serialize the hash algorithm into\n>> the data store, specify a version number for each supported algorithm.\n>> Note that we don't use the values of the constants themselves, as they\n>> are internal and could change in the future.\n>>\n>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n>> ---\n>>   commit-graph.c | 9 ++++++++-\n>>   1 file changed, 8 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/commit-graph.c b/commit-graph.c\n>> index 7a28fbb03f..e587c21bb6 100644\n>> --- a/commit-graph.c\n>> +++ b/commit-graph.c\n>> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n>>\n>>   static uint8_t oid_version(void)\n>>   {\n>> -       return 1;\n>> +       switch (hash_algo_by_ptr(the_hash_algo)) {\n>> +               case GIT_HASH_SHA1:\n>> +                       return 1;\n>> +               case GIT_HASH_SHA256:\n>> +                       return 2;\n> Should we just increase this field to uint32_t and store format_id\n> instead? That will keep oid version unique in all data formats.\nBoth the commit-graph and multi-pack-index store a single byte for the \nhash version, so that ship has sailed (without incrementing the full \nfile version number in each format).\n\nIt may be good to make this method accessible to both formats. I'm not \nsure if Brian's branch is built on top of the multi-pack-index code. \nProbably best to see if ds/multi-pack-verify is in the history.\n\nThanks,\n-Stolee\n"},{"id":"360641","messageId":"CACsJy8Bg6BpQp4dDXzs8Y_d6RHW9ti4+FLqnAqGwW8F-SWUnPg@mail.gmail.com","threadId":"49566","inReplyTo":"fbd59390-42ee-8c06-74b7-1e7081680913@gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-16T16:09:41Z","receivedAt":"2018-10-16T16:10:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 16, 2018 at 6:01 PM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 10/16/2018 11:35 AM, Duy Nguyen wrote:\n> > On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> >> Since the commit-graph code wants to serialize the hash algorithm into\n> >> the data store, specify a version number for each supported algorithm.\n> >> Note that we don't use the values of the constants themselves, as they\n> >> are internal and could change in the future.\n> >>\n> >> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> >> ---\n> >>   commit-graph.c | 9 ++++++++-\n> >>   1 file changed, 8 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/commit-graph.c b/commit-graph.c\n> >> index 7a28fbb03f..e587c21bb6 100644\n> >> --- a/commit-graph.c\n> >> +++ b/commit-graph.c\n> >> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n> >>\n> >>   static uint8_t oid_version(void)\n> >>   {\n> >> -       return 1;\n> >> +       switch (hash_algo_by_ptr(the_hash_algo)) {\n> >> +               case GIT_HASH_SHA1:\n> >> +                       return 1;\n> >> +               case GIT_HASH_SHA256:\n> >> +                       return 2;\n> > Should we just increase this field to uint32_t and store format_id\n> > instead? That will keep oid version unique in all data formats.\n> Both the commit-graph and multi-pack-index store a single byte for the\n> hash version, so that ship has sailed (without incrementing the full\n> file version number in each format).\n\nAnd it's probably premature to add the oid version field when multiple\nhash support has not been fully realized. Now we have different ways\nof storing hash id and need separate mappings.\n\nI would go for incrementing file version. Otherwise maybe we just\nupdate format_id to be one byte instead, and the way of storing hash\nversion in commit-graph will be used everywhere.\n\n> It may be good to make this method accessible to both formats. I'm not\n> sure if Brian's branch is built on top of the multi-pack-index code.\n> Probably best to see if ds/multi-pack-verify is in the history.\n>\n> Thanks,\n> -Stolee\n\n\n\n-- \nDuy\n"},{"id":"360687","messageId":"20181016223917.GC432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"xmqqk1mi4pn0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-16T22:39:17Z","receivedAt":"2018-10-16T22:39:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 11:00:19AM +0900, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > Since the commit-graph code wants to serialize the hash algorithm into\n> > the data store, specify a version number for each supported algorithm.\n> > Note that we don't use the values of the constants themselves, as they\n> > are internal and could change in the future.\n> >\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> >  commit-graph.c | 9 ++++++++-\n> >  1 file changed, 8 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/commit-graph.c b/commit-graph.c\n> > index 7a28fbb03f..e587c21bb6 100644\n> > --- a/commit-graph.c\n> > +++ b/commit-graph.c\n> > @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n> >  \n> >  static uint8_t oid_version(void)\n> >  {\n> > -\treturn 1;\n> > +\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n> > +\t\tcase GIT_HASH_SHA1:\n> > +\t\t\treturn 1;\n> > +\t\tcase GIT_HASH_SHA256:\n> > +\t\t\treturn 2;\n> > +\t\tdefault:\n> > +\t\t\tBUG(\"unknown hash algorithm\");\n> > +\t}\n> \n> Style: align switch/case.\n\nWill fix.\n\n> Shouldn't this be just using GIT_HASH_* constants instead?  Are we\n> expecting that it would be benefitial to allow more than one \"oid\n> version\" even within a same GIT_HASH_*?\n\nI really would like to have us rely not rely explicitly on those values.\nWe don't serialize them anywhere else, and they're explicitly documented\nas not being suitable for serialization.  If we were writing this fresh\ntoday, we'd probably use the format_version field, which is designed for\nthis purpose, or simply omit the field altogether.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360689","messageId":"20181016224404.GD432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"CACsJy8Bg6BpQp4dDXzs8Y_d6RHW9ti4+FLqnAqGwW8F-SWUnPg@mail.gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-16T22:44:04Z","receivedAt":"2018-10-16T22:44:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 06:09:41PM +0200, Duy Nguyen wrote:\n> On Tue, Oct 16, 2018 at 6:01 PM Derrick Stolee <stolee@gmail.com> wrote:\n> >\n> > On 10/16/2018 11:35 AM, Duy Nguyen wrote:\n> > > On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson\n> > > <sandals@crustytoothpaste.net> wrote:\n> > >> Since the commit-graph code wants to serialize the hash algorithm into\n> > >> the data store, specify a version number for each supported algorithm.\n> > >> Note that we don't use the values of the constants themselves, as they\n> > >> are internal and could change in the future.\n> > >>\n> > >> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > >> ---\n> > >>   commit-graph.c | 9 ++++++++-\n> > >>   1 file changed, 8 insertions(+), 1 deletion(-)\n> > >>\n> > >> diff --git a/commit-graph.c b/commit-graph.c\n> > >> index 7a28fbb03f..e587c21bb6 100644\n> > >> --- a/commit-graph.c\n> > >> +++ b/commit-graph.c\n> > >> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n> > >>\n> > >>   static uint8_t oid_version(void)\n> > >>   {\n> > >> -       return 1;\n> > >> +       switch (hash_algo_by_ptr(the_hash_algo)) {\n> > >> +               case GIT_HASH_SHA1:\n> > >> +                       return 1;\n> > >> +               case GIT_HASH_SHA256:\n> > >> +                       return 2;\n> > > Should we just increase this field to uint32_t and store format_id\n> > > instead? That will keep oid version unique in all data formats.\n> > Both the commit-graph and multi-pack-index store a single byte for the\n> > hash version, so that ship has sailed (without incrementing the full\n> > file version number in each format).\n> \n> And it's probably premature to add the oid version field when multiple\n> hash support has not been fully realized. Now we have different ways\n> of storing hash id and need separate mappings.\n\nHonestly, anything in the .git directory that is not the v3 pack indexes\nor the loose object file should be in exactly one hash algorithm.  We\ncould simply just leave this value at 1 all the time and ignore the\nfield, since we already know what algorithm it will use.\n\n> I would go for incrementing file version. Otherwise maybe we just\n> update format_id to be one byte instead, and the way of storing hash\n> version in commit-graph will be used everywhere.\n\nIt needs to be four bytes for pack files so that it's four-byte aligned.\nOtherwise accessing pack index fields will cause alignment issues if we\ndon't massively rewrite the pack handling code.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360690","messageId":"20181016224555.GE432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"xmqq7eii35h8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 00/13] Base SHA-256 implementation","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-16T22:45:55Z","receivedAt":"2018-10-16T22:46:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 01:01:07PM +0900, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> >  t/t0014-hash.sh                       |  54 ++++++++\n> >  create mode 100755 t/t0014-hash.sh\n> \n> If I am not mistaken, 0014 is already used by another topic in\n> flight, and will cause test-lint failure on 'pu'.\n\nThis series was written a while back.  I'll rename it.  Feel free to\nwait for v3 before picking it up.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360741","messageId":"d7412414-61a0-2da5-257f-f3d9764e15dd@gmail.com","threadId":"49566","inReplyTo":"20181015021900.1030041-14-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-17T12:21:42Z","receivedAt":"2018-10-17T12:21:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2018 10:19 PM, brian m. carlson wrote:\n> Since the commit-graph code wants to serialize the hash algorithm into\n> the data store, specify a version number for each supported algorithm.\n> Note that we don't use the values of the constants themselves, as they\n> are internal and could change in the future.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>   commit-graph.c | 9 ++++++++-\n>   1 file changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 7a28fbb03f..e587c21bb6 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n>   \n>   static uint8_t oid_version(void)\n>   {\n> -\treturn 1;\n> +\tswitch (hash_algo_by_ptr(the_hash_algo)) {\nhash_algo_by_ptr is specified as an inline function in hash.h, so this \nleads to a linking error in jch and pu right now.\n\nI think the fix is simply to add '#include \"hash.h\"' to the list of \nincludes.\n\nThanks,\n-Stolee\n"},{"id":"360748","messageId":"20181017133220.GQ19800@szeder.dev","threadId":"49566","inReplyTo":"20181015021900.1030041-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 02/13] sha1-file: provide functions to look up hash algorithms","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-17T13:32:20Z","receivedAt":"2018-10-17T13:32:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 15, 2018 at 02:18:49AM +0000, brian m. carlson wrote:\n> There are several ways we might refer to a hash algorithm: by name, such\n> as in the config file; by format ID, such as in a pack; or internally,\n> by a pointer to the hash_algos array.  Provide functions to look up hash\n> algorithms based on these various forms and return the internal constant\n> used for them.  If conversion to another form is necessary, this\n> internal constant can be used to look up the proper data in the\n> hash_algos array.\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  hash.h      | 13 +++++++++++++\n>  sha1-file.c | 21 +++++++++++++++++++++\n>  2 files changed, 34 insertions(+)\n> \n> diff --git a/hash.h b/hash.h\n> index 7c8238bc2e..90f4344619 100644\n> --- a/hash.h\n> +++ b/hash.h\n> @@ -98,4 +98,17 @@ struct git_hash_algo {\n>  };\n>  extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n>  \n> +/*\n> + * Return a GIT_HASH_* constant based on the name.  Returns GIT_HASH_UNKNOWN if\n> + * the name doesn't match a known algorithm.\n> + */\n> +int hash_algo_by_name(const char *name);\n> +/* Identical, except based on the format ID. */\n> +int hash_algo_by_id(uint32_t format_id);\n> +/* Identical, except for a pointer to struct git_hash_algo. */\n> +inline int hash_algo_by_ptr(const struct git_hash_algo *p)\n\nThis has to be declared as static, otherwise the linker will error out\nwhen building without optimization:\n\n    LINK git\nlibgit.a(commit-graph.o): In function `oid_version':\n/home/szeder/src/git/commit-graph.c:48: undefined reference to `hash_algo_by_ptr'\nlibgit.a(hex.o): In function `hash_to_hex':\n/home/szeder/src/git/hex.c:123: undefined reference to `hash_algo_by_ptr'\nlibgit.a(hex.o): In function `oid_to_hex':\n/home/szeder/src/git/hex.c:128: undefined reference to `hash_algo_by_ptr'\ncollect2: error: ld returned 1 exit status\nMakefile:2055: recipe for target 'git' failed\nmake: *** [git] Error 1\n\n\n> +{\n> +\treturn p - hash_algos;\n> +}\n> +\n>  #endif\n> diff --git a/sha1-file.c b/sha1-file.c\n> index e29825f259..3a75d515eb 100644\n> --- a/sha1-file.c\n> +++ b/sha1-file.c\n> @@ -122,6 +122,27 @@ const char *empty_blob_oid_hex(void)\n>  \treturn oid_to_hex_r(buf, the_hash_algo->empty_blob);\n>  }\n>  \n> +int hash_algo_by_name(const char *name)\n> +{\n> +\tint i;\n> +\tif (!name)\n> +\t\treturn GIT_HASH_UNKNOWN;\n> +\tfor (i = 1; i < GIT_HASH_NALGOS; i++)\n> +\t\tif (!strcmp(name, hash_algos[i].name))\n> +\t\t\treturn i;\n> +\treturn GIT_HASH_UNKNOWN;\n> +}\n> +\n> +int hash_algo_by_id(uint32_t format_id)\n> +{\n> +\tint i;\n> +\tfor (i = 1; i < GIT_HASH_NALGOS; i++)\n> +\t\tif (format_id == hash_algos[i].format_id)\n> +\t\t\treturn i;\n> +\treturn GIT_HASH_UNKNOWN;\n> +}\n> +\n> +\n>  /*\n>   * This is meant to hold a *small* number of objects that you would\n>   * want read_sha1_file() to be able to return, but yet you do not want\n"},{"id":"360754","messageId":"CACsJy8C9nySzG0Uqs6_5V2rXHkAKr-ShXU7N9sPp4N5z=D0zQA@mail.gmail.com","threadId":"49566","inReplyTo":"20181016224404.GD432229@genre.crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-17T14:31:19Z","receivedAt":"2018-10-17T14:31:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Oct 17, 2018 at 12:44 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> > > >>   static uint8_t oid_version(void)\n> > > >>   {\n> > > >> -       return 1;\n> > > >> +       switch (hash_algo_by_ptr(the_hash_algo)) {\n> > > >> +               case GIT_HASH_SHA1:\n> > > >> +                       return 1;\n> > > >> +               case GIT_HASH_SHA256:\n> > > >> +                       return 2;\n> > > > Should we just increase this field to uint32_t and store format_id\n> > > > instead? That will keep oid version unique in all data formats.\n> > > Both the commit-graph and multi-pack-index store a single byte for the\n> > > hash version, so that ship has sailed (without incrementing the full\n> > > file version number in each format).\n> >\n> > And it's probably premature to add the oid version field when multiple\n> > hash support has not been fully realized. Now we have different ways\n> > of storing hash id and need separate mappings.\n>\n> Honestly, anything in the .git directory that is not the v3 pack indexes\n> or the loose object file should be in exactly one hash algorithm.  We\n> could simply just leave this value at 1 all the time and ignore the\n> field, since we already know what algorithm it will use.\n\nIn this particular case, I agree, but not as a general principle. It's\nnice to have independence for fsck-like tools. I don't know if we have\na tool that simply validates commit-graph file format (and not trying\nto access any real object). But for such a tool, I guess we can just\npass the hash algorithm from command line. The user would have to\nguess a bit.\n-- \nDuy\n"},{"id":"360757","messageId":"20181017161241.GS19800@szeder.dev","threadId":"49566","inReplyTo":"20181015021900.1030041-11-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-17T16:12:41Z","receivedAt":"2018-10-17T16:12:49Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 15, 2018 at 02:18:57AM +0000, brian m. carlson wrote:\n> diff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\n> new file mode 100644\n> index 0000000000..18350c161a\n> --- /dev/null\n> +++ b/sha256/block/sha256.c\n> @@ -0,0 +1,180 @@\n> +#include \"git-compat-util.h\"\n> +#include \"./sha256.h\"\n> +\n> +#define BLKSIZE blk_SHA256_BLKSIZE\n> +\n> +void blk_SHA256_Init(blk_SHA256_CTX *ctx)\n> +{\n> +\tctx->offset = 0;\n> +\tctx->length = 0;\n> +\tctx->state[0] = 0x6A09E667UL;\n> +\tctx->state[1] = 0xBB67AE85UL;\n> +\tctx->state[2] = 0x3C6EF372UL;\n> +\tctx->state[3] = 0xA54FF53AUL;\n> +\tctx->state[4] = 0x510E527FUL;\n> +\tctx->state[5] = 0x9B05688CUL;\n> +\tctx->state[6] = 0x1F83D9ABUL;\n> +\tctx->state[7] = 0x5BE0CD19UL;\n> +}\n> +\n> +static inline uint32_t ror(uint32_t x, unsigned n)\n> +{\n> +\treturn (x >> n) | (x << (32 - n));\n> +}\n> +\n> +#define Ch(x,y,z)       (z ^ (x & (y ^ z)))\n> +#define Maj(x,y,z)      (((x | y) & z) | (x & y))\n> +#define S(x, n)         ror((x),(n))\n> +#define R(x, n)         ((x)>>(n))\n> +#define Sigma0(x)       (S(x, 2) ^ S(x, 13) ^ S(x, 22))\n> +#define Sigma1(x)       (S(x, 6) ^ S(x, 11) ^ S(x, 25))\n> +#define Gamma0(x)       (S(x, 7) ^ S(x, 18) ^ R(x, 3))\n> +#define Gamma1(x)       (S(x, 17) ^ S(x, 19) ^ R(x, 10))\n\n[...]\n\n> +#define RND(a,b,c,d,e,f,g,h,i,ki)                    \\\n> +\tt0 = h + Sigma1(e) + Ch(e, f, g) + ki + W[i];   \\\n> +\tt1 = Sigma0(a) + Maj(a, b, c);                  \\\n> +\td += t0;                                        \\\n> +\th  = t0 + t1;\n> +\n> +\tRND(S[0],S[1],S[2],S[3],S[4],S[5],S[6],S[7],0,0x428a2f98);\n\n[...]\n\n> +#undef RND\n> +\n> +\tfor (i = 0; i < 8; i++) {\n> +\t\tctx->state[i] = ctx->state[i] + S[i];\n> +\t}\n> +}\n> +\n> +#define MIN(x, y) ((x) < (y) ? (x) : (y))\n\nOn macOS there is a MIN macro already defined in the system headers,\nresulting in the following error:\n\n      CC sha256/block/sha256.o\n  sha256/block/sha256.c:133:9: error: 'MIN' macro redefined [-Werror,-Wmacro-redefined]\n  #define MIN(x, y) ((x) < (y) ? (x) : (y))\n          ^\n  /usr/include/sys/param.h:215:9: note: previous definition is here\n  #define MIN(a,b) (((a)<(b))?(a):(b))\n          ^\n  1 error generated.\n  make: *** [sha256/block/sha256.o] Error 1\n\nA simple \"#undef MIN\" solves this issue.  However, I wonder whether we\nshould #undef the other #define directives as well, just to be sure\n(and perhaps overly cautious).\n\n> +void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n> +{\n> +\tconst unsigned char *in = data;\n> +\tsize_t n;\n> +\tctx->length += len;\n> +\twhile (len > 0) {\n> +\t\tif (!ctx->offset && len >= BLKSIZE) {\n> +\t\t\tblk_SHA256_Transform(ctx, in);\n> +\t\t\tin += BLKSIZE;\n> +\t\t\tlen -= BLKSIZE;\n> +\t\t} else {\n> +\t\t\tn = MIN(len, (BLKSIZE - ctx->offset));\n> +\t\t\tmemcpy(ctx->buf + ctx->offset, in, n);\n> +\t\t\tctx->offset += n;\n> +\t\t\tin += n;\n> +\t\t\tlen -= n;\n> +\t\t\tif (ctx->offset == BLKSIZE) {\n> +\t\t\t\tblk_SHA256_Transform(ctx, ctx->buf);\n> +\t\t\t\tctx->offset = 0;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void blk_SHA256_Final(unsigned char *digest, blk_SHA256_CTX *ctx)\n> +{\n> +\tconst unsigned trip = BLKSIZE - sizeof(ctx->length);\n> +\tint i;\n> +\n> +\tctx->length <<= 3;\n> +\tctx->buf[ctx->offset++] = 0x80;\n> +\n> +\tif (ctx->offset > trip) {\n> +\t\tmemset(ctx->buf + ctx->offset, 0, BLKSIZE - ctx->offset);\n> +\t\tblk_SHA256_Transform(ctx, ctx->buf);\n> +\t\tctx->offset = 0;\n> +\t}\n> +\n> +\tmemset(ctx->buf + ctx->offset, 0, BLKSIZE - ctx->offset - sizeof(ctx->length));\n> +\n> +\tput_be64(ctx->buf + trip, ctx->length);\n\nSome GCC versions (e.g. gcc-4.8 with -O2 -Wall -Werror) complain about\nthe above line:\n\n      CC sha256/block/sha256.o\n  sha256/block/sha256.c: In function ‘blk_SHA256_Final’:\n  sha256/block/sha256.c:174:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n    put_be64(ctx->buf + trip, ctx->length);\n    ^\n  cc1: all warnings being treated as errors\n  make: *** [sha256/block/sha256.o] Error 1\n\nSomething like this makes it compile:\n\n  void *ptr = ctx->buf + trip;\n  put_be64(ptr, ctx->length);\n\nHowever, it's not immediately obvious to me why the compiler\ncomplains, or why that intermediate void* variable makes any\ndifference, but now it's not the time to put on my language lawyer\nhat.\n\nPerhaps an old compiler bug?  Clang in general, newer GCC versions, or\ngcc-4.8 with -Wall -Werror but without -O2 don't seem to be affected.\n\n\n> +\tblk_SHA256_Transform(ctx, ctx->buf);\n> +\n> +\t/* copy output */\n> +\tfor (i = 0; i < 8; i++, digest += sizeof(uint32_t))\n> +\t\tput_be32(digest, ctx->state[i]);\n> +}\n"},{"id":"360790","messageId":"20181017223855.GI432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"d7412414-61a0-2da5-257f-f3d9764e15dd@gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-17T22:38:55Z","receivedAt":"2018-10-17T22:39:04Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 17, 2018 at 08:21:42AM -0400, Derrick Stolee wrote:\n> On 10/14/2018 10:19 PM, brian m. carlson wrote:\n> > Since the commit-graph code wants to serialize the hash algorithm into\n> > the data store, specify a version number for each supported algorithm.\n> > Note that we don't use the values of the constants themselves, as they\n> > are internal and could change in the future.\n> > \n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> >   commit-graph.c | 9 ++++++++-\n> >   1 file changed, 8 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/commit-graph.c b/commit-graph.c\n> > index 7a28fbb03f..e587c21bb6 100644\n> > --- a/commit-graph.c\n> > +++ b/commit-graph.c\n> > @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)\n> >   static uint8_t oid_version(void)\n> >   {\n> > -\treturn 1;\n> > +\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n> hash_algo_by_ptr is specified as an inline function in hash.h, so this leads\n> to a linking error in jch and pu right now.\n> \n> I think the fix is simply to add '#include \"hash.h\"' to the list of\n> includes.\n\nhash.h is already included by cache.h, so it should be present.  We\nprobably need to make it static.  I'll make that change; can you see if\nit fixes the problem for you?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360791","messageId":"20181017225306.GJ432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"CACsJy8DPBkRyPYF7Vfh0bQHJDvDZcmp54wjzRxG4Ho40SkMTSg@mail.gmail.com","subject":"Re: [PATCH v2 01/13] sha1-file: rename algorithm to \"sha1\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-17T22:53:06Z","receivedAt":"2018-10-17T22:53:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 05:17:53PM +0200, Duy Nguyen wrote:\n> On Mon, Oct 15, 2018 at 4:21 AM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > The transition plan anticipates us using a syntax such as \"^{sha1}\" for\n> > disambiguation.  Since this is a syntax some people will be typing a\n> > lot, it makes sense to provide a short, easy-to-type syntax.  Omitting\n> > the dash doesn't create any ambiguity, but it does make it shorter and\n> \n> \"but\" or \"and\"? I think both clauses are on the same side ... or did\n> you mean omitting the dash does create ambiguity?\n\nI think \"but\" is correct here.  This is a standard \"This doesn't make it\nworse, but it does make it better\" phrase.  The \"but\" creates a contrast\nbetween what it doesn't do and what it does.\n\nI'm trying to come up with a different way to say this that may be\neasier to understand, but I'm failing to do so in a natural-sounding\nway.  Does the following sound better?\n\n  Omitting the dash doesn't introduce any ambiguity; however, it does\n  make the text shorter and easier to type, especially for touch\n  typists.\n\n> > easier to type, especially for touch typists.  In addition, the\n> > transition plan already uses \"sha1\" in this context.\n> >\n> > Rename the name of SHA-1 implementation to \"sha1\".\n> >\n> > Note that this change creates no backwards compatibility concerns, since\n> > we haven't yet used this field in any serialized data formats.\n> \n> But we're not going to use this _string_ in any data format either\n> because we'll stick to format_id field anyway, right?\n\nWe'll use it in extensions.objectFormat and other config files.  But in\nanything binary, we'll use format_id.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360793","messageId":"20181017230409.GK432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"20181017161241.GS19800@szeder.dev","subject":"Re: [PATCH v2 10/13] Add a base implementation of SHA-256 support","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-17T23:04:09Z","receivedAt":"2018-10-17T23:04:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 17, 2018 at 06:12:41PM +0200, SZEDER Gábor wrote:\n> On macOS there is a MIN macro already defined in the system headers,\n> resulting in the following error:\n> \n>       CC sha256/block/sha256.o\n>   sha256/block/sha256.c:133:9: error: 'MIN' macro redefined [-Werror,-Wmacro-redefined]\n>   #define MIN(x, y) ((x) < (y) ? (x) : (y))\n>           ^\n>   /usr/include/sys/param.h:215:9: note: previous definition is here\n>   #define MIN(a,b) (((a)<(b))?(a):(b))\n>           ^\n>   1 error generated.\n>   make: *** [sha256/block/sha256.o] Error 1\n> \n> A simple \"#undef MIN\" solves this issue.  However, I wonder whether we\n> should #undef the other #define directives as well, just to be sure\n> (and perhaps overly cautious).\n\nI'll undefine the macros at the top.\n\nFor MIN, I'm going to go with the simple approach and pull pieces from\nthe block-sha1 implementation, which doesn't require this code path...\n\n> Some GCC versions (e.g. gcc-4.8 with -O2 -Wall -Werror) complain about\n> the above line:\n> \n>       CC sha256/block/sha256.o\n>   sha256/block/sha256.c: In function ‘blk_SHA256_Final’:\n>   sha256/block/sha256.c:174:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n>     put_be64(ctx->buf + trip, ctx->length);\n>     ^\n>   cc1: all warnings being treated as errors\n>   make: *** [sha256/block/sha256.o] Error 1\n> \n> Something like this makes it compile:\n> \n>   void *ptr = ctx->buf + trip;\n>   put_be64(ptr, ctx->length);\n> \n> However, it's not immediately obvious to me why the compiler\n> complains, or why that intermediate void* variable makes any\n> difference, but now it's not the time to put on my language lawyer\n> hat.\n> \n> Perhaps an old compiler bug?  Clang in general, newer GCC versions, or\n> gcc-4.8 with -Wall -Werror but without -O2 don't seem to be affected.\n\n...or this code path.  Presumably GCC 4.8 and macOS are happy with the\ncode that we already have in block-sha1.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360795","messageId":"20181017234904.GL432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"xmqqo9bu4pww.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 03/13] hex: introduce functions to print arbitrary hashes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-17T23:49:04Z","receivedAt":"2018-10-17T23:49:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 10:54:23AM +0900, Junio C Hamano wrote:\n> Even though in hex.c I see mixture of *_algo and *_algop helper\n> functions, I see only \"algo\" variants above.  Is it our official\n> stance to use primarily the integer index into the algo array when\n> specifying the hash, and when a caller into 'multi-hash' API happens\n> to have other things, it should use functions in 2/13 to convert it\n> to the canonical \"int algo\" beforehand?\n\nThat was my intention, yes.\n\n> I am not saying it is bad or good to choose the index into the algo\n> array as the primary way to specify the algorithm.  I think it is a\n> good idea to pick one and stick to it, and I wanted to make sure\n> that the choice we made here is clearly communicated to any future\n> developer who read this code.\n\nYeah, that was my feeling as well.  I wanted to pick something fixed and\nstick to it.\n\n> Having said the above, seeing the use of hash_algo_by_ptr() here\n> makes me suspect if it makes more sense to use the algop as the\n> primary way to specify which algorithm the caller wants to use.\n> IOW, making the set of helpers in 02/13 to allow quering by name,\n> format-id, or the integer index and have them all return a pointer\n> to \"const struct git_hash_algo\".  Two immediate downsides I can see\n> is that it exposes the actual structure to the callers (but is it\n> really a problem?  Outside callers learn hash sizes etc. by accessing\n> its fields anyway without treating the algo struct as opaque.), and\n> passing an 8-byte pointer may be more costly than passing a small\n> integer index that ranges between 0 and 1 at most (assuming that\n> we'd only use SHA-1 and \"the current NewHash\" in the code).\n\nI thought about this.  The one downside to this is that we can't use\nthose values anywhere we need an integer constant expression, like a\nswitch.  I suppose that just means we need hash_algo_by_ptr in those\ncases, and not everywhere else, which would make the code cleaner.\n\nLet me reroll with that change, and we'll see if we like it better.  If\nwe don't, I can pull the old version out of history.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360796","messageId":"20181018000618.GM432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"CACsJy8C9nySzG0Uqs6_5V2rXHkAKr-ShXU7N9sPp4N5z=D0zQA@mail.gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-18T00:06:19Z","receivedAt":"2018-10-18T00:06:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 17, 2018 at 04:31:19PM +0200, Duy Nguyen wrote:\n> On Wed, Oct 17, 2018 at 12:44 AM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > Honestly, anything in the .git directory that is not the v3 pack indexes\n> > or the loose object file should be in exactly one hash algorithm.  We\n> > could simply just leave this value at 1 all the time and ignore the\n> > field, since we already know what algorithm it will use.\n> \n> In this particular case, I agree, but not as a general principle. It's\n> nice to have independence for fsck-like tools. I don't know if we have\n> a tool that simply validates commit-graph file format (and not trying\n> to access any real object). But for such a tool, I guess we can just\n> pass the hash algorithm from command line. The user would have to\n> guess a bit.\n\nI'm going to drop this patch for now.  I'll send a follow-up series\nlater which bumps the format version for this and the multi-pack index\nand serializes them with the four-byte value.  I probably should have\ncaught this earlier, but unfortunately I don't always have the time to\nlook at every series that hits the list.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360847","messageId":"734fb14a-b9c8-a27b-5cda-862486c77052@gmail.com","threadId":"49566","inReplyTo":"20181018000618.GM432229@genre.crustytoothpaste.net","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-18T13:03:22Z","receivedAt":"2018-10-18T13:03:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/17/2018 8:06 PM, brian m. carlson wrote:\n> On Wed, Oct 17, 2018 at 04:31:19PM +0200, Duy Nguyen wrote:\n>> On Wed, Oct 17, 2018 at 12:44 AM brian m. carlson\n>> <sandals@crustytoothpaste.net> wrote:\n>>> Honestly, anything in the .git directory that is not the v3 pack indexes\n>>> or the loose object file should be in exactly one hash algorithm.  We\n>>> could simply just leave this value at 1 all the time and ignore the\n>>> field, since we already know what algorithm it will use.\n>> In this particular case, I agree, but not as a general principle. It's\n>> nice to have independence for fsck-like tools. I don't know if we have\n>> a tool that simply validates commit-graph file format (and not trying\n>> to access any real object). But for such a tool, I guess we can just\n>> pass the hash algorithm from command line. The user would have to\n>> guess a bit.\n> I'm going to drop this patch for now.  I'll send a follow-up series\n> later which bumps the format version for this and the multi-pack index\n> and serializes them with the four-byte value.  I probably should have\n> caught this earlier, but unfortunately I don't always have the time to\n> look at every series that hits the list.\nWe should coordinate before incrementing the version number. I was \nworking on making the file formats incremental (think split-index) but \ncouldn't come up with a way to do it without incrementing the file \nformat. It would be best to combine these features into v2 of each format.\n\nThanks,\n-Stolee\n"},{"id":"360985","messageId":"20181019222103.GN432229@genre.crustytoothpaste.net","threadId":"49566","inReplyTo":"734fb14a-b9c8-a27b-5cda-862486c77052@gmail.com","subject":"Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-19T22:21:04Z","receivedAt":"2018-10-19T22:21:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Oct 18, 2018 at 09:03:22AM -0400, Derrick Stolee wrote:\n> We should coordinate before incrementing the version number. I was working\n> on making the file formats incremental (think split-index) but couldn't come\n> up with a way to do it without incrementing the file format. It would be\n> best to combine these features into v2 of each format.\n\nFor the moment, I'm happy to leave things as they are, and I'll\ninterpret version 1 of the hash version field as whatever hash is in use\nelsewhere in .git.  If you're going to bump the version in the future,\nfeel free to CC me on the patches, and we'll make sure that we serialize\nthe format_version field in the file then.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"}]}