{"thread":{"id":"54045","subject":"[PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats","startedAt":"2020-08-14T18:07:25Z","lastAt":"2020-08-17T23:12:30Z","messageCount":22,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Taylor Blau","brian m. carlson","Derrick Stolee","Eric Sunshine","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"403693","messageId":"pull.703.git.1597428440.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":null,"subject":"[PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-14T18:07:17Z","receivedAt":"2020-08-14T18:07:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"As discussed [1], there is some concern around binary file formats requiring\nthe context of the repository config in order to infer hash lengths. Two\nformats that were designed with the hash transition in mind (commit-graph\nand multi-pack-index) have bytes available to indicate the hash algorithm\nused. Let's actually update these formats to be more self-contained with the\ntwo hash algorithms being available.\n\n[1] \nhttps://lore.kernel.org/git/CAN0heSp024=Kyy7gdQ2VSetk_5iVhj_qdT8CMVPcry_AwWrhHQ@mail.gmail.com/\n\nThis merges cleanly with tb/bloom-improvements, but both that branch and\nthis patch series have merge conflicts with the corrected commit date patch\nseries [2].\n\n[2] \nhttps://lore.kernel.org/git/pull.676.v2.git.1596941624.gitgitgadget@gmail.com/\n\nIn particular, the following conflict can be resolved in the \"obvioius\" way:\n\n<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< HEAD\n header: 43475048 1 $OID_VERSION 3 $NUM_BASE\n================================\n header: 43475048 1 1 4 $NUM_BASE\n>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> abhishek/corrected_commit_date\n\nInstead use:\n\n header: 43475048 1 $OID_VERSION 4 $NUM_BASE\n\nBut, it also needs the following fix to actually work with this series:\n\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 211ec625d2..09f133792c 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -464,7 +464,7 @@ test_expect_success 'setup repo for mixed generation commit-graph-chain' '\n GIT_TEST_COMMIT_GRAPH_NO_GDAT=1 git commit-graph write --reachable --split=no-merge &&\n test-tool read-graph >output &&\n cat >expect <<-EOF &&\n- header: 43475048 1 1 4 1\n+ header: 43475048 1 $OID_VERSION 4 1\n num_commits: 2\n chunks: oid_fanout oid_lookup commit_metadata\n EOF\n@@ -482,7 +482,7 @@ test_expect_success 'does not write generation data chunk if not present on exis\n git commit-graph write --reachable --split=no-merge &&\n test-tool read-graph >output &&\n cat >expect <<-EOF &&\n- header: 43475048 1 1 4 2\n+ header: 43475048 1 $OID_VERSION 4 2\n num_commits: 3\n chunks: oid_fanout oid_lookup commit_metadata\n EOF\n\nIf this is the way we want to go with the formats, then I'll assist\ncoordinating these textual and semantic merge conflicts.\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n t/README: document GIT_TEST_DEFAULT_HASH\n commit-graph: use the hash version byte\n multi-pack-index: use hash version byte\n\n .../technical/commit-graph-format.txt | 9 +++-\n Documentation/technical/pack-format.txt | 7 ++-\n commit-graph.c | 6 ++-\n midx.c | 32 +++++++++++---\n t/README | 3 ++\n t/helper/test-read-midx.c | 8 +++-\n t/t4216-log-bloom.sh | 8 +++-\n t/t5318-commit-graph.sh | 37 +++++++++++++++-\n t/t5319-multi-pack-index.sh | 43 +++++++++++++++++--\n t/t5324-split-commit-graph.sh | 8 +++-\n 10 files changed, 142 insertions(+), 19 deletions(-)\n\n\nbase-commit: 878e727637ec5815ccb3301eb994a54df95b21b8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-703%2Fderrickstolee%2Fcommit-graph-256-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-703/derrickstolee/commit-graph-256-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/703\n-- \ngitgitgadget\n"},{"id":"403694","messageId":"242a44b63c8fc0ab7e8d8a6a913fde71444f931d.1597428440.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.git.1597428440.gitgitgadget@gmail.com","subject":"[PATCH 1/3] t/README: document GIT_TEST_DEFAULT_HASH","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-14T18:07:18Z","receivedAt":"2020-08-14T18:07:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n t/README | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex 70ec61cf88..ecf8c7291d 100644\n--- a/t/README\n+++ b/t/README\n@@ -421,6 +421,9 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true (which is\n the default when running tests), errors out when an abbreviated option\n is used.\n \n+GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to use\n+in the test scripts.\n+\n Naming Tests\n ------------\n \n-- \ngitgitgadget\n\n"},{"id":"403695","messageId":"4bbfd345d16da4604dd20decda8ecb12372e4223.1597428440.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.git.1597428440.gitgitgadget@gmail.com","subject":"[PATCH 2/3] commit-graph: use the hash version byte","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-14T18:07:19Z","receivedAt":"2020-08-14T18:07:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe commit-graph format reserved a byte among the header of the file to\nstore a \"hash version\". During the SHA-256 work, this was not modified\nbecause file formats are not necessarily intended to work across hash\nversions. If a repository has SHA-256 as its hash algorithm, it\nautomatically up-shifts the lengths of object names in all necessary\nformats.\n\nHowever, since we have this byte available for adjusting the version, we\ncan make the file formats more obviously incompatible instead of relying\non other context from the repository.\n\nUpdate the oid_version() method in commit-graph.c to add a new value, 2,\nfor sha-256. This automatically writes the new value in a SHA-256\nrepository _and_ verifies the value is correct. This is a breaking\nchange relative to the current 'master' branch since 092b677 (Merge\nbranch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking\nrelative to any released version of Git.\n\nThe test impact is relatively minor: the output of 'test-tool\nread-graph' lists the header information, so those instances of '1' need\nto be replaced with a variable determined by GIT_TEST_DEFAULT_HASH. A\nmore careful test is added that specifically creates a repository of\neach type then swaps the commit-graph files. The important value here is\nthat the \"git log\" command succeeds while writing a message to stderr.\n\nSigned-off-by: Derrick Stolee \n---\n .../technical/commit-graph-format.txt | 9 ++++-\n commit-graph.c | 6 ++-\n t/t4216-log-bloom.sh | 8 +++-\n t/t5318-commit-graph.sh | 37 ++++++++++++++++++-\n t/t5324-split-commit-graph.sh | 8 +++-\n 5 files changed, 62 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt\nindex 440541045d..6ddbceba15 100644\n--- a/Documentation/technical/commit-graph-format.txt\n+++ b/Documentation/technical/commit-graph-format.txt\n@@ -42,8 +42,13 @@ HEADER:\n 1-byte version number:\n Currently, the only valid version is 1.\n \n- 1-byte Hash Version (1 = SHA-1)\n- We infer the hash length (H) from this value.\n+ 1-byte Hash Version\n+ We infer the hash length (H) from this value:\n+\t1 => SHA-1\n+\t2 => SHA-256\n+ If the hash type does not match the repository's hash algorithm, the\n+ commit-graph file should be ignored with a warning presented to the\n+ user.\n \n 1-byte number (C) of \"chunks\"\n \ndiff --git a/commit-graph.c b/commit-graph.c\nindex e51c91dd5b..d03328d64c 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -179,7 +179,11 @@ static char *get_chain_filename(struct object_directory *odb)\n \n static uint8_t oid_version(void)\n {\n-\treturn 1;\n+\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n+\t\treturn 1;\n+\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n+\t\treturn 2;\n+\tdie(_(\"invalid hash version\"));\n }\n \n static struct commit_graph *alloc_commit_graph(void)\ndiff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\nindex c21cc160f3..906af2799d 100755\n--- a/t/t4216-log-bloom.sh\n+++ b/t/t4216-log-bloom.sh\n@@ -6,6 +6,12 @@ test_description='git log for a path with Bloom filters'\n GIT_TEST_COMMIT_GRAPH=0\n GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n \n+OID_VERSION=1\n+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n+then\n+\tOID_VERSION=2\n+fi\n+\n test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n \tgit init &&\n \tmkdir A A/B A/B/C &&\n@@ -35,7 +41,7 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n graph_read_expect () {\n \tNUM_CHUNKS=5\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 $NUM_CHUNKS 0\n+\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data\n \tEOF\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 044cf8a3de..5b65017676 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -5,6 +5,12 @@ test_description='commit graph'\n \n GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n \n+OID_VERSION=1\n+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n+then\n+\tOID_VERSION=2\n+fi\n+\n test_expect_success 'setup full repo' '\n \tmkdir full &&\n \tcd \"$TRASH_DIRECTORY/full\" &&\n@@ -77,7 +83,7 @@ graph_read_expect() {\n \t\tNUM_CHUNKS=$((3 + $(echo \"$2\" | wc -w)))\n \tfi\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 $NUM_CHUNKS 0\n+\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \tEOF\n@@ -412,6 +418,35 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n \t)\n '\n \n+test_expect_success 'warn on improper hash version' '\n+\tgit init --object-format=sha1 sha1 &&\n+\t(\n+\t\tcd sha1 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha1\n+\t) &&\n+\tgit init --object-format=sha256 sha256 &&\n+\t(\n+\t\tcd sha256 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha256\n+\t) &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n+\t) &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n+\t)\n+'\n+\n # the verify tests below expect the commit-graph to contain\n # exactly the commits reachable from the commits/8 branch.\n # If the file changes the set of commits in the list, then the\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex ea28d522b8..6f1a324f4f 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -6,6 +6,12 @@ test_description='split commit graph'\n GIT_TEST_COMMIT_GRAPH=0\n GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n \n+OID_VERSION=1\n+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n+then\n+\tOID_VERSION=2\n+fi\n+\n test_expect_success 'setup repo' '\n \tgit init &&\n \tgit config core.commitGraph true &&\n@@ -28,7 +34,7 @@ graph_read_expect() {\n \t\tNUM_BASE=$2\n \tfi\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 3 $NUM_BASE\n+\theader: 43475048 1 $OID_VERSION 3 $NUM_BASE\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata\n \tEOF\n-- \ngitgitgadget\n\n"},{"id":"403696","messageId":"b4645789adf26e46cea721896867135941fb7654.1597428440.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.git.1597428440.gitgitgadget@gmail.com","subject":"[PATCH 3/3] multi-pack-index: use hash version byte","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-14T18:07:20Z","receivedAt":"2020-08-14T18:07:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSimilar to the commit-graph format, the multi-pack-index format has a\nbyte in the header intended to track the hash version used to write the\nfile. This allows one to interpret the hash length without having the\ncontext of the repository config specifying the hash length. This was\nnot modified as part of the SHA-256 work because the hash length was\nautomatically up-shifted due to that config.\n\nSince we have this byte available, we can make the file formats more\nobviously incompatible instead of relying on other context from the\nrepository.\n\nAdd a new oid_version() method in midx.c similar to the one in\ncommit-graph.c. This is specifically made separate from that\nimplementation to avoid artificially linking the formats.\n\nThe test impact requires a few more things than the corresponding change\nin the commit-graph format. Specifically, 'test-tool read-midx' was not\nwriting anything about this header value to output. Since the value\navailable in 'struct multi_pack_index' is hash_len instead of a version\nvalue, we output \"20\" or \"32\" instead of \"1\" or \"2\".\n\nSince we want a user to not have their Git commands fail if their\nmulti-pack-index has the incorrect hash version compared to the\nrepository's hash version, we relax the die() to an error() in\nload_multi_pack_index(). This has some effect on 'git multi-pack-index\nverify' as we need to check that a failed parse of a file that exists is\nactually a verify error. For that test that checks the hash version\nmatches, we change the corrupted byte from \"2\" to \"3\" to ensure the test\nfails for both hash algorithms.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/technical/pack-format.txt | 7 +++-\n midx.c | 32 ++++++++++++++----\n t/helper/test-read-midx.c | 8 +++--\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++---\n 4 files changed, 77 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/technical/pack-format.txt b/Documentation/technical/pack-format.txt\nindex d3a142c652..16cf7e83aa 100644\n--- a/Documentation/technical/pack-format.txt\n+++ b/Documentation/technical/pack-format.txt\n@@ -273,7 +273,12 @@ HEADER:\n \t Git only writes or recognizes version 1.\n \n \t1-byte Object Id Version\n-\t Git only writes or recognizes version 1 (SHA1).\n+\t We infer the length of object IDs (OIDs) from this value:\n+\t\t1 => SHA-1\n+\t\t2 => SHA-256\n+\t If the hash type does not match the repository's hash algorithm,\n+\t the multi-pack-index file should be ignored with a warning\n+\t presented to the user.\n \n \t1-byte number of \"chunks\"\n \ndiff --git a/midx.c b/midx.c\nindex a5fb797ede..0c165a40f5 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -17,7 +17,6 @@\n #define MIDX_BYTE_HASH_VERSION 5\n #define MIDX_BYTE_NUM_CHUNKS 6\n #define MIDX_BYTE_NUM_PACKS 8\n-#define MIDX_HASH_VERSION 1\n #define MIDX_HEADER_SIZE 12\n #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz)\n \n@@ -36,6 +35,15 @@\n \n #define PACK_EXPIRED UINT_MAX\n \n+static uint8_t oid_version(void)\n+{\n+\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n+\t\treturn 1;\n+\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n+\t\treturn 2;\n+\tdie(_(\"invalid hash version\"));\n+}\n+\n static char *get_midx_filename(const char *object_dir)\n {\n \treturn xstrfmt(\"%s/pack/multi-pack-index\", object_dir);\n@@ -90,8 +98,11 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \t\t m->version);\n \n \thash_version = m->data[MIDX_BYTE_HASH_VERSION];\n-\tif (hash_version != MIDX_HASH_VERSION)\n-\t\tdie(_(\"hash version %u does not match\"), hash_version);\n+\tif (hash_version != oid_version()) {\n+\t\terror(_(\"multi-pack-index hash version %u does not match version %u\"),\n+\t\t hash_version, oid_version());\n+\t\tgoto cleanup_fail;\n+\t}\n \tm->hash_len = the_hash_algo->rawsz;\n \n \tm->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];\n@@ -418,7 +429,7 @@ static size_t write_midx_header(struct hashfile *f,\n \n \thashwrite_be32(f, MIDX_SIGNATURE);\n \tbyte_values[0] = MIDX_VERSION;\n-\tbyte_values[1] = MIDX_HASH_VERSION;\n+\tbyte_values[1] = oid_version();\n \tbyte_values[2] = num_chunks;\n \tbyte_values[3] = 0; /* unused */\n \thashwrite(f, byte_values, sizeof(byte_values));\n@@ -1105,8 +1116,17 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n-\tif (!m)\n-\t\treturn 0;\n+\tif (!m) {\n+\t\tint result = 0;\n+\t\tstruct stat sb;\n+\t\tchar *filename = get_midx_filename(object_dir);\n+\t\tif (!stat(filename, &sb)) {\n+\t\t\terror(_(\"multi-pack-index file exists, but failed to parse\"));\n+\t\t\tresult = 1;\n+\t\t}\n+\t\tfree(filename);\n+\t\treturn result;\n+\t}\n \n \tif (flags & MIDX_PROGRESS)\n \t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 831b586d02..2430880f78 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -7,14 +7,18 @@\n static int read_midx_file(const char *object_dir)\n {\n \tuint32_t i;\n-\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tstruct multi_pack_index *m;\n+\n+\tsetup_git_directory();\n+\tm = load_multi_pack_index(object_dir, 1);\n \n \tif (!m)\n \t\treturn 1;\n \n-\tprintf(\"header: %08x %d %d %d\\n\",\n+\tprintf(\"header: %08x %d %d %d %d\\n\",\n \t m->signature,\n \t m->version,\n+\t m->hash_len,\n \t m->num_chunks,\n \t m->num_packs);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 7dfff0f8f4..09cbca4949 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -5,6 +5,8 @@ test_description='multi-pack-indexes'\n \n objdir=.git/objects\n \n+HASH_LEN=$(test_oid rawsz)\n+\n midx_read_expect () {\n \tNUM_PACKS=$1\n \tNUM_OBJECTS=$2\n@@ -13,7 +15,7 @@ midx_read_expect () {\n \tEXTRA_CHUNKS=\"$5\"\n \t{\n \t\tcat <<-EOF &&\n-\t\theader: 4d494458 1 $NUM_CHUNKS $NUM_PACKS\n+\t\theader: 4d494458 1 $HASH_LEN $NUM_CHUNKS $NUM_PACKS\n \t\tchunks: pack-names oid-fanout oid-lookup object-offsets$EXTRA_CHUNKS\n \t\tnum_objects: $NUM_OBJECTS\n \t\tpacks:\n@@ -46,7 +48,7 @@ test_expect_success \"don't write midx with no packs\" '\n \ttest_path_is_missing pack/multi-pack-index\n '\n \n-test_expect_success \"Warn if a midx contains no oid\" '\n+test_expect_success SHA1 'warn if a midx contains no oid' '\n \tcp \"$TEST_DIRECTORY\"/t5319/no-objects.midx $objdir/pack/multi-pack-index &&\n \ttest_must_fail git multi-pack-index verify &&\n \trm $objdir/pack/multi-pack-index\n@@ -198,6 +200,40 @@ test_expect_success 'write midx with twelve packs' '\n \n compare_results_with_midx \"twelve packs\"\n \n+test_expect_success 'warn on improper hash version' '\n+\tgit init --object-format=sha1 sha1 &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tgit config core.multiPackIndex true &&\n+\t\ttest_commit 1 &&\n+\t\tgit repack -a &&\n+\t\tgit multi-pack-index write &&\n+\t\tmv .git/objects/pack/multi-pack-index ../mpi-sha1\n+\t) &&\n+\tgit init --object-format=sha256 sha256 &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tgit config core.multiPackIndex true &&\n+\t\ttest_commit 1 &&\n+\t\tgit repack -a &&\n+\t\tgit multi-pack-index write &&\n+\t\tmv .git/objects/pack/multi-pack-index ../mpi-sha256\n+\t) &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tmv ../mpi-sha256 .git/objects/pack/multi-pack-index &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"multi-pack-index hash version 2 does not match version 1\" err\n+\t) &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tmv ../mpi-sha1 .git/objects/pack/multi-pack-index &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"multi-pack-index hash version 1 does not match version 2\" err\n+\t)\n+'\n+\n+\n test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n@@ -243,7 +279,6 @@ test_expect_success 'verify bad signature' '\n \t\t\"multi-pack-index signature\"\n '\n \n-HASH_LEN=$(test_oid rawsz)\n NUM_OBJECTS=74\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n@@ -272,7 +307,7 @@ test_expect_success 'verify bad version' '\n '\n \n test_expect_success 'verify bad OID version' '\n-\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\02\" $objdir \\\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\03\" $objdir \\\n \t\t\"hash version\"\n '\n \n-- \ngitgitgadget\n"},{"id":"403698","messageId":"xmqqtux5vyn4.fsf@gitster.c.googlers.com","threadId":"54045","inReplyTo":"242a44b63c8fc0ab7e8d8a6a913fde71444f931d.1597428440.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] t/README: document GIT_TEST_DEFAULT_HASH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T19:02:55Z","receivedAt":"2020-08-14T19:03:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> Signed-off-by: Derrick Stolee \n> ---\n> t/README | 3 +++\n> 1 file changed, 3 insertions(+)\n\nVery helpful. Will queue.\n\n>\n> diff --git a/t/README b/t/README\n> index 70ec61cf88..ecf8c7291d 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -421,6 +421,9 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true (which is\n> the default when running tests), errors out when an abbreviated option\n> is used.\n> \n> +GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to use\n> +in the test scripts.\n> +\n> Naming Tests\n> ------------\n"},{"id":"403699","messageId":"xmqqo8ndvyje.fsf@gitster.c.googlers.com","threadId":"54045","inReplyTo":"4bbfd345d16da4604dd20decda8ecb12372e4223.1597428440.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T19:05:09Z","receivedAt":"2020-08-14T19:05:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The commit-graph format reserved a byte among the header of the file to\n> store a \"hash version\". During the SHA-256 work, this was not modified\n> because file formats are not necessarily intended to work across hash\n> versions. If a repository has SHA-256 as its hash algorithm, it\n> automatically up-shifts the lengths of object names in all necessary\n> formats.\n>\n> However, since we have this byte available for adjusting the version, we\n> can make the file formats more obviously incompatible instead of relying\n> on other context from the repository.\n\nVery good idea.\n\n> Update the oid_version() method in commit-graph.c to add a new value, 2,\n> for sha-256. This automatically writes the new value in a SHA-256\n> repository _and_ verifies the value is correct. This is a breaking\n> change relative to the current 'master' branch since 092b677 (Merge\n> branch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking\n> relative to any released version of Git.\n\nThat is perfectly fine. I think any file and on-wire protocol that\nuses anything but SHA-1 without identifying what it uses is a bug.\n\nWill queue. Thanks.\n\n> +OID_VERSION=1\n> +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> +then\n> +\tOID_VERSION=2\n> +fi\n> +\n> test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n> \tgit init &&\n> \tmkdir A A/B A/B/C &&\n> @@ -35,7 +41,7 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n> graph_read_expect () {\n> \tNUM_CHUNKS=5\n> \tcat >expect <<- EOF\n> -\theader: 43475048 1 1 $NUM_CHUNKS 0\n> +\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n> \tnum_commits: $1\n> \tchunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data\n> \tEOF\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index 044cf8a3de..5b65017676 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -5,6 +5,12 @@ test_description='commit graph'\n> \n> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n> \n> +OID_VERSION=1\n> +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> +then\n> +\tOID_VERSION=2\n> +fi\n> +\n> test_expect_success 'setup full repo' '\n> \tmkdir full &&\n> \tcd \"$TRASH_DIRECTORY/full\" &&\n> @@ -77,7 +83,7 @@ graph_read_expect() {\n> \t\tNUM_CHUNKS=$((3 + $(echo \"$2\" | wc -w)))\n> \tfi\n> \tcat >expect <<- EOF\n> -\theader: 43475048 1 1 $NUM_CHUNKS 0\n> +\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n> \tnum_commits: $1\n> \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n> \tEOF\n> @@ -412,6 +418,35 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n> \t)\n> '\n> \n> +test_expect_success 'warn on improper hash version' '\n> +\tgit init --object-format=sha1 sha1 &&\n> +\t(\n> +\t\tcd sha1 &&\n> +\t\ttest_commit 1 &&\n> +\t\tgit commit-graph write --reachable &&\n> +\t\tmv .git/objects/info/commit-graph ../cg-sha1\n> +\t) &&\n> +\tgit init --object-format=sha256 sha256 &&\n> +\t(\n> +\t\tcd sha256 &&\n> +\t\ttest_commit 1 &&\n> +\t\tgit commit-graph write --reachable &&\n> +\t\tmv .git/objects/info/commit-graph ../cg-sha256\n> +\t) &&\n> +\t(\n> +\t\tcd sha1 &&\n> +\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n> +\t\tgit log -1 2>err &&\n> +\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n> +\t) &&\n> +\t(\n> +\t\tcd sha256 &&\n> +\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n> +\t\tgit log -1 2>err &&\n> +\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n> +\t)\n> +'\n> +\n> # the verify tests below expect the commit-graph to contain\n> # exactly the commits reachable from the commits/8 branch.\n> # If the file changes the set of commits in the list, then the\n> diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\n> index ea28d522b8..6f1a324f4f 100755\n> --- a/t/t5324-split-commit-graph.sh\n> +++ b/t/t5324-split-commit-graph.sh\n> @@ -6,6 +6,12 @@ test_description='split commit graph'\n> GIT_TEST_COMMIT_GRAPH=0\n> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n> \n> +OID_VERSION=1\n> +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> +then\n> +\tOID_VERSION=2\n> +fi\n> +\n> test_expect_success 'setup repo' '\n> \tgit init &&\n> \tgit config core.commitGraph true &&\n> @@ -28,7 +34,7 @@ graph_read_expect() {\n> \t\tNUM_BASE=$2\n> \tfi\n> \tcat >expect <<- EOF\n> -\theader: 43475048 1 1 3 $NUM_BASE\n> +\theader: 43475048 1 $OID_VERSION 3 $NUM_BASE\n> \tnum_commits: $1\n> \tchunks: oid_fanout oid_lookup commit_metadata\n> \tEOF\n"},{"id":"403700","messageId":"xmqqeeo9vxl0.fsf@gitster.c.googlers.com","threadId":"54045","inReplyTo":"pull.703.git.1597428440.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T19:25:47Z","receivedAt":"2020-08-14T19:25:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> As discussed [1], there is some concern around binary file formats requiring\n> the context of the repository config in order to infer hash lengths. Two\n> formats that were designed with the hash transition in mind (commit-graph\n> and multi-pack-index) have bytes available to indicate the hash algorithm\n> used. Let's actually update these formats to be more self-contained with the\n> two hash algorithms being available.\n> ...\n> If this is the way we want to go with the formats, then I'll assist\n> coordinating these textual and semantic merge conflicts.\n\nI agree that the files should be self-identifying, but have these\nchanges tested without sha256 hash?\n"},{"id":"403703","messageId":"20200814200552.GA30103@syl.lan","threadId":"54045","inReplyTo":"xmqqo8ndvyje.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-08-14T20:05:52Z","receivedAt":"2020-08-14T20:05:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Aug 14, 2020 at 12:05:09PM -0700, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n>\n> > From: Derrick Stolee \n> >\n> > The commit-graph format reserved a byte among the header of the file to\n> > store a \"hash version\". During the SHA-256 work, this was not modified\n> > because file formats are not necessarily intended to work across hash\n> > versions. If a repository has SHA-256 as its hash algorithm, it\n> > automatically up-shifts the lengths of object names in all necessary\n> > formats.\n> >\n> > However, since we have this byte available for adjusting the version, we\n> > can make the file formats more obviously incompatible instead of relying\n> > on other context from the repository.\n>\n> Very good idea.\n>\n> > Update the oid_version() method in commit-graph.c to add a new value, 2,\n> > for sha-256. This automatically writes the new value in a SHA-256\n> > repository _and_ verifies the value is correct. This is a breaking\n> > change relative to the current 'master' branch since 092b677 (Merge\n> > branch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking\n> > relative to any released version of Git.\n>\n> That is perfectly fine. I think any file and on-wire protocol that\n> uses anything but SHA-1 without identifying what it uses is a bug.\n>\n> Will queue. Thanks.\n\nGreat. I have nothing to add other than my 'ack' that this is a good\nidea.\n\n> > +OID_VERSION=1\n> > +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> > +then\n> > +\tOID_VERSION=2\n> > +fi\n> > +\n> > test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n> > \tgit init &&\n> > \tmkdir A A/B A/B/C &&\n> > @@ -35,7 +41,7 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n> > graph_read_expect () {\n> > \tNUM_CHUNKS=5\n> > \tcat >expect <<- EOF\n> > -\theader: 43475048 1 1 $NUM_CHUNKS 0\n> > +\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n> > \tnum_commits: $1\n> > \tchunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data\n> > \tEOF\n> > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> > index 044cf8a3de..5b65017676 100755\n> > --- a/t/t5318-commit-graph.sh\n> > +++ b/t/t5318-commit-graph.sh\n> > @@ -5,6 +5,12 @@ test_description='commit graph'\n> >\n> > GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n> >\n> > +OID_VERSION=1\n> > +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> > +then\n> > +\tOID_VERSION=2\n> > +fi\n> > +\n> > test_expect_success 'setup full repo' '\n> > \tmkdir full &&\n> > \tcd \"$TRASH_DIRECTORY/full\" &&\n> > @@ -77,7 +83,7 @@ graph_read_expect() {\n> > \t\tNUM_CHUNKS=$((3 + $(echo \"$2\" | wc -w)))\n> > \tfi\n> > \tcat >expect <<- EOF\n> > -\theader: 43475048 1 1 $NUM_CHUNKS 0\n> > +\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n> > \tnum_commits: $1\n> > \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n> > \tEOF\n> > @@ -412,6 +418,35 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n> > \t)\n> > '\n> >\n> > +test_expect_success 'warn on improper hash version' '\n> > +\tgit init --object-format=sha1 sha1 &&\n> > +\t(\n> > +\t\tcd sha1 &&\n> > +\t\ttest_commit 1 &&\n> > +\t\tgit commit-graph write --reachable &&\n> > +\t\tmv .git/objects/info/commit-graph ../cg-sha1\n> > +\t) &&\n> > +\tgit init --object-format=sha256 sha256 &&\n> > +\t(\n> > +\t\tcd sha256 &&\n> > +\t\ttest_commit 1 &&\n> > +\t\tgit commit-graph write --reachable &&\n> > +\t\tmv .git/objects/info/commit-graph ../cg-sha256\n> > +\t) &&\n> > +\t(\n> > +\t\tcd sha1 &&\n> > +\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n> > +\t\tgit log -1 2>err &&\n> > +\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n> > +\t) &&\n> > +\t(\n> > +\t\tcd sha256 &&\n> > +\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n> > +\t\tgit log -1 2>err &&\n> > +\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n> > +\t)\n> > +'\n> > +\n> > # the verify tests below expect the commit-graph to contain\n> > # exactly the commits reachable from the commits/8 branch.\n> > # If the file changes the set of commits in the list, then the\n> > diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\n> > index ea28d522b8..6f1a324f4f 100755\n> > --- a/t/t5324-split-commit-graph.sh\n> > +++ b/t/t5324-split-commit-graph.sh\n> > @@ -6,6 +6,12 @@ test_description='split commit graph'\n> > GIT_TEST_COMMIT_GRAPH=0\n> > GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n> >\n> > +OID_VERSION=1\n> > +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> > +then\n> > +\tOID_VERSION=2\n> > +fi\n> > +\n> > test_expect_success 'setup repo' '\n> > \tgit init &&\n> > \tgit config core.commitGraph true &&\n> > @@ -28,7 +34,7 @@ graph_read_expect() {\n> > \t\tNUM_BASE=$2\n> > \tfi\n> > \tcat >expect <<- EOF\n> > -\theader: 43475048 1 1 3 $NUM_BASE\n> > +\theader: 43475048 1 $OID_VERSION 3 $NUM_BASE\n> > \tnum_commits: $1\n> > \tchunks: oid_fanout oid_lookup commit_metadata\n> > \tEOF\nThanks,\nTaylor\n"},{"id":"403706","messageId":"20200814201153.GK8085@camp.crustytoothpaste.net","threadId":"54045","inReplyTo":"4bbfd345d16da4604dd20decda8ecb12372e4223.1597428440.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-08-14T20:11:53Z","receivedAt":"2020-08-14T20:12:02Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-08-14 at 18:07:19, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/commit-graph.c b/commit-graph.c\n> index e51c91dd5b..d03328d64c 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -179,7 +179,11 @@ static char *get_chain_filename(struct object_directory *odb)\n> \n> static uint8_t oid_version(void)\n> {\n> -\treturn 1;\n> +\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n> +\t\treturn 1;\n> +\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n> +\t\treturn 2;\n> +\tdie(_(\"invalid hash version\"));\n> }\n\nCan we maybe say something like this?\n\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 default:\n die(_(\"invalid hash version\"));\n }\n\nThat way if SHA-256 gets broken and we switch to another 256-bit hash\n(SHA-3-256?), we'll be forced to update this properly.\n\n> static struct commit_graph *alloc_commit_graph(void)\n> diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\n> index c21cc160f3..906af2799d 100755\n> --- a/t/t4216-log-bloom.sh\n> +++ b/t/t4216-log-bloom.sh\n> @@ -6,6 +6,12 @@ test_description='git log for a path with Bloom filters'\n> GIT_TEST_COMMIT_GRAPH=0\n> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n> \n> +OID_VERSION=1\n> +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n> +then\n> +\tOID_VERSION=2\n> +fi\n\nCan we do something like this in the setup test instead?\n\n test_oid_cache <<-EOF\n oid_version sha1:1\n oid_version sha256:2\n EOF\n\n OID_VERSION=$(test_oid oid_version)\n # or, inline\n $(test_oid oid_version)\n\nThat uses the existing test framework to ensure we produce the right\nresults if someone adds another hash algorithm and that we produce a BUG\nif the value is missing. It will also make it easier to write tests if\nwe end up creating SHA-1- or SHA-256-specific tests, since we can look\nup those values directly with test_oid.\n\nSince you're using this across several tests, you could even just add\nthis to t/oid-info/hash-info like so:\n\n commit_graph_oid_version sha1:1\n commit_graph_oid_version sha256:2\n\nand then it's set in one place and can be used without any required\ntest_oid_cache invocations at all. I think three tests is sufficient\nbasis for us to add an entry there.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"403707","messageId":"20200814201448.GL8085@camp.crustytoothpaste.net","threadId":"54045","inReplyTo":"b4645789adf26e46cea721896867135941fb7654.1597428440.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] multi-pack-index: use hash version byte","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-08-14T20:14:48Z","receivedAt":"2020-08-14T20:15:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-08-14 at 18:07:20, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/midx.c b/midx.c\n> index a5fb797ede..0c165a40f5 100644\n> --- a/midx.c\n> +++ b/midx.c\n> @@ -17,7 +17,6 @@\n> #define MIDX_BYTE_HASH_VERSION 5\n> #define MIDX_BYTE_NUM_CHUNKS 6\n> #define MIDX_BYTE_NUM_PACKS 8\n> -#define MIDX_HASH_VERSION 1\n> #define MIDX_HEADER_SIZE 12\n> #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz)\n> \n> @@ -36,6 +35,15 @@\n> \n> #define PACK_EXPIRED UINT_MAX\n> \n> +static uint8_t oid_version(void)\n> +{\n> +\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n> +\t\treturn 1;\n> +\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n> +\t\treturn 2;\n> +\tdie(_(\"invalid hash version\"));\n> +}\n\nI think my same response to the last patch applies here.\n\nBeyond the things I mentioned in the last patch and here, I think this\nseries looks fine.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"403713","messageId":"xmqqy2mhuge2.fsf@gitster.c.googlers.com","threadId":"54045","inReplyTo":"20200814201153.GK8085@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T20:22:29Z","receivedAt":"2020-08-14T20:22:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> Can we maybe say something like this?\n>\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> default:\n> die(_(\"invalid hash version\"));\n> }\n>\n> That way if SHA-256 gets broken and we switch to another 256-bit hash\n> (SHA-3-256?), we'll be forced to update this properly.\n\nExcellent.\n"},{"id":"403718","messageId":"04ebec72-07de-4494-688f-a9f48442906e@gmail.com","threadId":"54045","inReplyTo":"xmqqeeo9vxl0.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-14T20:34:56Z","receivedAt":"2020-08-14T20:35:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/14/2020 3:25 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> As discussed [1], there is some concern around binary file formats requiring\n>> the context of the repository config in order to infer hash lengths. Two\n>> formats that were designed with the hash transition in mind (commit-graph\n>> and multi-pack-index) have bytes available to indicate the hash algorithm\n>> used. Let's actually update these formats to be more self-contained with the\n>> two hash algorithms being available.\n>> ...\n>> If this is the way we want to go with the formats, then I'll assist\n>> coordinating these textual and semantic merge conflicts.\n> \n> I agree that the files should be self-identifying, but have these\n> changes tested without sha256 hash?\n\nAll of the test scripts pass with and without GIT_TEST_DEFAULT_HASH=sha256,\nand this test in t5318 (and a similar one in t5319) are explicit about\ntesting both options:\n\n+test_expect_success 'warn on improper hash version' '\n+\tgit init --object-format=sha1 sha1 &&\n+\t(\n+\t\tcd sha1 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha1\n+\t) &&\n+\tgit init --object-format=sha256 sha256 &&\n+\t(\n+\t\tcd sha256 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha256\n+\t) &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n+\t) &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n+\t)\n+'\n+\n\nSince this tests exactly that the \"hash version\" byte is the same in\na SHA-1 repo, this checks that the new version of Git writes backwards-\ncompatible data in SHA-1 repos.\n\nOr are you hinting at a more subtle test scenario that I missed?\n\nThanks,\n-Stolee\n"},{"id":"403720","messageId":"7082f3c9-652e-36d9-26ae-d1f86e872480@gmail.com","threadId":"54045","inReplyTo":"20200814201153.GK8085@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-14T20:36:42Z","receivedAt":"2020-08-14T20:36:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/14/2020 4:11 PM, brian m. carlson wrote:\n> On 2020-08-14 at 18:07:19, Derrick Stolee via GitGitGadget wrote:\n>> diff --git a/commit-graph.c b/commit-graph.c\n>> index e51c91dd5b..d03328d64c 100644\n>> --- a/commit-graph.c\n>> +++ b/commit-graph.c\n>> @@ -179,7 +179,11 @@ static char *get_chain_filename(struct object_directory *odb)\n>> \n>> static uint8_t oid_version(void)\n>> {\n>> -\treturn 1;\n>> +\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n>> +\t\treturn 1;\n>> +\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n>> +\t\treturn 2;\n>> +\tdie(_(\"invalid hash version\"));\n>> }\n> \n> Can we maybe say something like this?\n> \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> default:\n> die(_(\"invalid hash version\"));\n> }\n> \n> That way if SHA-256 gets broken and we switch to another 256-bit hash\n> (SHA-3-256?), we'll be forced to update this properly.\n> \n>> static struct commit_graph *alloc_commit_graph(void)\n>> diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\n>> index c21cc160f3..906af2799d 100755\n>> --- a/t/t4216-log-bloom.sh\n>> +++ b/t/t4216-log-bloom.sh\n>> @@ -6,6 +6,12 @@ test_description='git log for a path with Bloom filters'\n>> GIT_TEST_COMMIT_GRAPH=0\n>> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n>> \n>> +OID_VERSION=1\n>> +if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n>> +then\n>> +\tOID_VERSION=2\n>> +fi\n> \n> Can we do something like this in the setup test instead?\n> \n> test_oid_cache <<-EOF\n> oid_version sha1:1\n> oid_version sha256:2\n> EOF\n> \n> OID_VERSION=$(test_oid oid_version)\n> # or, inline\n> $(test_oid oid_version)\n> \n> That uses the existing test framework to ensure we produce the right\n> results if someone adds another hash algorithm and that we produce a BUG\n> if the value is missing. It will also make it easier to write tests if\n> we end up creating SHA-1- or SHA-256-specific tests, since we can look\n> up those values directly with test_oid.\n> \n> Since you're using this across several tests, you could even just add\n> this to t/oid-info/hash-info like so:\n> \n> commit_graph_oid_version sha1:1\n> commit_graph_oid_version sha256:2\n> \n> and then it's set in one place and can be used without any required\n> test_oid_cache invocations at all. I think three tests is sufficient\n> basis for us to add an entry there.\n\nI appreciate the suggested improvements here. I'm happy to do\nsomething more similar to other places in the code.\n\nThese will be part of my v2 to be re-rolled early next week.\n\nThanks,\n-Stolee\n"},{"id":"403722","messageId":"CAPig+cQLR1=6W-Y1y1XiWg1pC9FKikpqVsMrDKeK=exLJFDgLg@mail.gmail.com","threadId":"54045","inReplyTo":"242a44b63c8fc0ab7e8d8a6a913fde71444f931d.1597428440.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] t/README: document GIT_TEST_DEFAULT_HASH","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-14T20:39:43Z","receivedAt":"2020-08-14T20:39:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 14, 2020 at 2:07 PM Derrick Stolee via GitGitGadget\n wrote:\n> diff --git a/t/README b/t/README\n> @@ -421,6 +421,9 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true> +GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to use\n> +in the test scripts.\n\nNit: The documentation for all the other environment variables has the\nform `GIT_VAR=` rather than placing the\npossible literal values within the angle brackets. So, perhaps this\ncan be written as:\n\n GIT_TEST_DEFAULT_HASH= specifies which hash algorithm\n to use in the test scripts. Recognized values for are\n \"sha1\" and \"sha256\".\n"},{"id":"403723","messageId":"8555fa4c-1eba-63da-e59c-edba6a39eac0@gmail.com","threadId":"54045","inReplyTo":"CAPig+cQLR1=6W-Y1y1XiWg1pC9FKikpqVsMrDKeK=exLJFDgLg@mail.gmail.com","subject":"Re: [PATCH 1/3] t/README: document GIT_TEST_DEFAULT_HASH","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-14T20:41:46Z","receivedAt":"2020-08-14T20:41:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/14/2020 4:39 PM, Eric Sunshine wrote:\n> On Fri, Aug 14, 2020 at 2:07 PM Derrick Stolee via GitGitGadget\n> wrote:\n>> diff --git a/t/README b/t/README\n>> @@ -421,6 +421,9 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true> +GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to use\n>> +in the test scripts.\n> \n> Nit: The documentation for all the other environment variables has the\n> form `GIT_VAR=` rather than placing the\n> possible literal values within the angle brackets. So, perhaps this\n> can be written as:\n> \n> GIT_TEST_DEFAULT_HASH= specifies which hash algorithm\n> to use in the test scripts. Recognized values for are\n> \"sha1\" and \"sha256\".\n\nSounds good!\n\nThanks,\n-Stolee\n"},{"id":"403727","messageId":"xmqqbljcvrbj.fsf@gitster.c.googlers.com","threadId":"54045","inReplyTo":"04ebec72-07de-4494-688f-a9f48442906e@gmail.com","subject":"Re: [PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T21:41:04Z","receivedAt":"2020-08-14T21:41:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 8/14/2020 3:25 PM, Junio C Hamano wrote:\n>> \"Derrick Stolee via GitGitGadget\" writes:\n>> \n>>> As discussed [1], there is some concern around binary file formats requiring\n>>> the context of the repository config in order to infer hash lengths. Two\n>>> formats that were designed with the hash transition in mind (commit-graph\n>>> and multi-pack-index) have bytes available to indicate the hash algorithm\n>>> used. Let's actually update these formats to be more self-contained with the\n>>> two hash algorithms being available.\n>>> ...\n>>> If this is the way we want to go with the formats, then I'll assist\n>>> coordinating these textual and semantic merge conflicts.\n>> \n>> I agree that the files should be self-identifying, but have these\n>> changes tested without sha256 hash?\n>\n> All of the test scripts pass with and without GIT_TEST_DEFAULT_HASH=sha256,\n> and this test in t5318 (and a similar one in t5319) are explicit about\n> testing both options:\n>\n> +test_expect_success 'warn on improper hash version' '\n> +\tgit init --object-format=sha1 sha1 &&\n> +\t(\n> +\t\tcd sha1 &&\n> +\t\ttest_commit 1 &&\n> +\t\tgit commit-graph write --reachable &&\n> +\t\tmv .git/objects/info/commit-graph ../cg-sha1\n> +\t) &&\n> +\tgit init --object-format=sha256 sha256 &&\n> +\t(\n> +\t\tcd sha256 &&\n> +\t\ttest_commit 1 &&\n> +\t\tgit commit-graph write --reachable &&\n> +\t\tmv .git/objects/info/commit-graph ../cg-sha256\n> +\t) &&\n> +\t(\n> +\t\tcd sha1 &&\n> +\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n> +\t\tgit log -1 2>err &&\n> +\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n> +\t) &&\n> +\t(\n> +\t\tcd sha256 &&\n> +\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n> +\t\tgit log -1 2>err &&\n> +\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n> +\t)\n> +'\n> +\n>\n> Since this tests exactly that the \"hash version\" byte is the same in\n> a SHA-1 repo, this checks that the new version of Git writes backwards-\n> compatible data in SHA-1 repos.\n>\n> Or are you hinting at a more subtle test scenario that I missed?\n\nNo, I was just wondering how ready we are, as the four tests looked\ntoo easy ;-)\n"},{"id":"403756","messageId":"CAN0heSq3voEdae_qiFOx1D1X2qD-2QiOTb5f0Y750pC1UPdH-A@mail.gmail.com","threadId":"54045","inReplyTo":"7082f3c9-652e-36d9-26ae-d1f86e872480@gmail.com","subject":"Re: [PATCH 2/3] commit-graph: use the hash version byte","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-08-15T13:46:41Z","receivedAt":"2020-08-15T21:58:03Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 14 Aug 2020 at 22:36, Derrick Stolee wrote:\n>\n> I appreciate the suggested improvements here. I'm happy to do\n> something more similar to other places in the code.\n>\n> These will be part of my v2 to be re-rolled early next week.\n\nI have nothing to add to brian's great suggestions. I'll be very happy\nto see this patch instead of my patch 5/5 'commit-graph-format.txt: fix\n\"Hash Version\" description'.\n\nMartin\n"},{"id":"403821","messageId":"pull.703.v2.git.1597673089.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.git.1597428440.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-17T14:04:45Z","receivedAt":"2020-08-17T14:04:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"As discussed [1], there is some concern around binary file formats requiring\nthe context of the repository config in order to infer hash lengths. Two\nformats that were designed with the hash transition in mind (commit-graph\nand multi-pack-index) have bytes available to indicate the hash algorithm\nused. Let's actually update these formats to be more self-contained with the\ntwo hash algorithms being available.\n\n[1] \nhttps://lore.kernel.org/git/CAN0heSp024=Kyy7gdQ2VSetk_5iVhj_qdT8CMVPcry_AwWrhHQ@mail.gmail.com/\n\nThis merges cleanly with tb/bloom-improvements, but both that branch and\nthis patch series have merge conflicts with the corrected commit date patch\nseries [2].\n\n[2] \nhttps://lore.kernel.org/git/pull.676.v2.git.1596941624.gitgitgadget@gmail.com/\n\nIn particular, the following conflict can be resolved in the \"obvioius\" way:\n\n<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< HEAD\n header: 43475048 1 $(test_oid oid_version) 3 $NUM_BASE\n================================\n header: 43475048 1 1 4 $NUM_BASE\n>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> abhishek/corrected_commit_date\n\nInstead use:\n\n header: 43475048 1 $(test_oid oid_version) 4 $NUM_BASE\n\nBut, it also needs the following fix to actually work with this series:\n\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 211ec625d2..09f133792c 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -464,7 +464,7 @@ test_expect_success 'setup repo for mixed generation commit-graph-chain' '\n GIT_TEST_COMMIT_GRAPH_NO_GDAT=1 git commit-graph write --reachable --split=no-merge &&\n test-tool read-graph >output &&\n cat >expect <<-EOF &&\n- header: 43475048 1 1 4 1\n+ header: 43475048 1 $(test_oid oid_version) 4 1\n num_commits: 2\n chunks: oid_fanout oid_lookup commit_metadata\n EOF\n@@ -482,7 +482,7 @@ test_expect_success 'does not write generation data chunk if not present on exis\n git commit-graph write --reachable --split=no-merge &&\n test-tool read-graph >output &&\n cat >expect <<-EOF &&\n- header: 43475048 1 1 4 2\n+ header: 43475048 1 $(test_oid oid_version) 4 2\n num_commits: 3\n chunks: oid_fanout oid_lookup commit_metadata\n EOF\n\nIf this is the way we want to go with the formats, then I'll assist\ncoordinating these textual and semantic merge conflicts.\n\nUPDATES IN V2\n=============\n\n * Documentation is updated, thanks to Eric's suggestion.\n * The implementation of oid_version() and the way we access it in the test\n scripts is improved, thanks to Brian's suggestion.\n * I use \"mv\" instead of \"cp\" in the cross-version tests because of a\n subtlety on macOS when overwriting these files.\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n t/README: document GIT_TEST_DEFAULT_HASH\n commit-graph: use the \"hash version\" byte\n multi-pack-index: use hash version byte\n\n .../technical/commit-graph-format.txt | 9 +++-\n Documentation/technical/pack-format.txt | 7 ++-\n commit-graph.c | 9 +++-\n midx.c | 35 ++++++++++++---\n t/README | 4 ++\n t/helper/test-read-midx.c | 8 +++-\n t/t4216-log-bloom.sh | 9 +++-\n t/t5318-commit-graph.sh | 38 +++++++++++++++-\n t/t5319-multi-pack-index.sh | 43 +++++++++++++++++--\n t/t5324-split-commit-graph.sh | 5 ++-\n 10 files changed, 146 insertions(+), 21 deletions(-)\n\n\nbase-commit: 878e727637ec5815ccb3301eb994a54df95b21b8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-703%2Fderrickstolee%2Fcommit-graph-256-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-703/derrickstolee/commit-graph-256-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/703\n\nRange-diff vs v1:\n\n 1: 242a44b63c ! 1: 62e7247bad t/README: document GIT_TEST_DEFAULT_HASH\n @@ Metadata\n ## Commit message ##\n t/README: document GIT_TEST_DEFAULT_HASH\n \n + Helped-by: Eric Sunshine \n Signed-off-by: Derrick Stolee \n \n ## t/README ##\n @@ t/README: GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true (which is\n the default when running tests), errors out when an abbreviated option\n is used.\n \n -+GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to use\n -+in the test scripts.\n ++GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to\n ++use in the test scripts. Recognized values for are \"sha1\"\n ++and \"sha256\".\n +\n Naming Tests\n ------------\n 2: 4bbfd345d1 ! 2: 8d481f3b22 commit-graph: use the hash version byte\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - commit-graph: use the hash version byte\n + commit-graph: use the \"hash version\" byte\n \n The commit-graph format reserved a byte among the header of the file to\n store a \"hash version\". During the SHA-256 work, this was not modified\n @@ Commit message\n each type then swaps the commit-graph files. The important value here is\n that the \"git log\" command succeeds while writing a message to stderr.\n \n + Helped-by: brian m. carlson \n Signed-off-by: Derrick Stolee \n \n ## Documentation/technical/commit-graph-format.txt ##\n @@ commit-graph.c: static char *get_chain_filename(struct object_directory *odb)\n static uint8_t oid_version(void)\n {\n -\treturn 1;\n -+\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n ++\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n ++\tcase GIT_HASH_SHA1:\n +\t\treturn 1;\n -+\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n ++\tcase GIT_HASH_SHA256:\n +\t\treturn 2;\n -+\tdie(_(\"invalid hash version\"));\n ++\tdefault:\n ++\t\tdie(_(\"invalid hash version\"));\n ++\t}\n }\n \n static struct commit_graph *alloc_commit_graph(void)\n \n ## t/t4216-log-bloom.sh ##\n -@@ t/t4216-log-bloom.sh: test_description='git log for a path with Bloom filters'\n - GIT_TEST_COMMIT_GRAPH=0\n - GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n - \n -+OID_VERSION=1\n -+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n -+then\n -+\tOID_VERSION=2\n -+fi\n -+\n - test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n - \tgit init &&\n - \tmkdir A A/B A/B/C &&\n @@ t/t4216-log-bloom.sh: test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n + \trm file_to_be_deleted &&\n + \tgit add . &&\n + \tgit commit -m \"file removed\" &&\n +-\tgit commit-graph write --reachable --changed-paths\n ++\tgit commit-graph write --reachable --changed-paths &&\n ++\n ++\ttest_oid_cache <<-EOF\n ++\toid_version sha1:1\n ++\toid_version sha256:2\n ++\tEOF\n + '\n graph_read_expect () {\n \tNUM_CHUNKS=5\n \tcat >expect <<- EOF\n -\theader: 43475048 1 1 $NUM_CHUNKS 0\n -+\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n ++\theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data\n \tEOF\n \n ## t/t5318-commit-graph.sh ##\n -@@ t/t5318-commit-graph.sh: test_description='commit graph'\n - \n - GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n - \n -+OID_VERSION=1\n -+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n -+then\n -+\tOID_VERSION=2\n -+fi\n -+\n - test_expect_success 'setup full repo' '\n - \tmkdir full &&\n +@@ t/t5318-commit-graph.sh: test_expect_success 'setup full repo' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n + \tgit init &&\n + \tgit config core.commitGraph true &&\n +-\tobjdir=\".git/objects\"\n ++\tobjdir=\".git/objects\" &&\n ++\n ++\ttest_oid_cache <<-EOF\n ++\toid_version sha1:1\n ++\toid_version sha256:2\n ++\tEOF\n + '\n + \n + test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n @@ t/t5318-commit-graph.sh: graph_read_expect() {\n \t\tNUM_CHUNKS=$((3 + $(echo \"$2\" | wc -w)))\n \tfi\n \tcat >expect <<- EOF\n -\theader: 43475048 1 1 $NUM_CHUNKS 0\n -+\theader: 43475048 1 $OID_VERSION $NUM_CHUNKS 0\n ++\theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \tEOF\n @@ t/t5318-commit-graph.sh: test_expect_success 'replace-objects invalidates commit\n # If the file changes the set of commits in the list, then the\n \n ## t/t5324-split-commit-graph.sh ##\n -@@ t/t5324-split-commit-graph.sh: test_description='split commit graph'\n - GIT_TEST_COMMIT_GRAPH=0\n - GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0\n +@@ t/t5324-split-commit-graph.sh: test_expect_success 'setup repo' '\n \n -+OID_VERSION=1\n -+if [ \"$GIT_DEFAULT_HASH\" = \"sha256\" ]\n -+then\n -+\tOID_VERSION=2\n -+fi\n + \tbase sha1:1376\n + \tbase sha256:1496\n +\n - test_expect_success 'setup repo' '\n - \tgit init &&\n - \tgit config core.commitGraph true &&\n ++\toid_version sha1:1\n ++\toid_version sha256:2\n + \tEOM\n + '\n + \n @@ t/t5324-split-commit-graph.sh: graph_read_expect() {\n \t\tNUM_BASE=$2\n \tfi\n \tcat >expect <<- EOF\n -\theader: 43475048 1 1 3 $NUM_BASE\n -+\theader: 43475048 1 $OID_VERSION 3 $NUM_BASE\n ++\theader: 43475048 1 $(test_oid oid_version) 3 $NUM_BASE\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata\n \tEOF\n 3: b4645789ad ! 3: 822e46868f multi-pack-index: use hash version byte\n @@ Commit message\n matches, we change the corrupted byte from \"2\" to \"3\" to ensure the test\n fails for both hash algorithms.\n \n + Helped-by: brian m. carlson \n Signed-off-by: Derrick Stolee \n \n ## Documentation/technical/pack-format.txt ##\n @@ midx.c\n \n +static uint8_t oid_version(void)\n +{\n -+\tif (the_hash_algo->rawsz == GIT_SHA1_RAWSZ)\n ++\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n ++\tcase GIT_HASH_SHA1:\n +\t\treturn 1;\n -+\tif (the_hash_algo->rawsz == GIT_SHA256_RAWSZ)\n ++\tcase GIT_HASH_SHA256:\n +\t\treturn 2;\n -+\tdie(_(\"invalid hash version\"));\n ++\tdefault:\n ++\t\tdie(_(\"invalid hash version\"));\n ++\t}\n +}\n +\n static char *get_midx_filename(const char *object_dir)\n\n-- \ngitgitgadget\n"},{"id":"403822","messageId":"62e7247badffe64ac5cfa69e02057cf704c02583.1597673089.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.v2.git.1597673089.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] t/README: document GIT_TEST_DEFAULT_HASH","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-17T14:04:46Z","receivedAt":"2020-08-17T14:04:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nHelped-by: Eric Sunshine \nSigned-off-by: Derrick Stolee \n---\n t/README | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex 70ec61cf88..2adaf7c2d2 100644\n--- a/t/README\n+++ b/t/README\n@@ -421,6 +421,10 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=, when true (which is\n the default when running tests), errors out when an abbreviated option\n is used.\n \n+GIT_TEST_DEFAULT_HASH= specifies which hash algorithm to\n+use in the test scripts. Recognized values for are \"sha1\"\n+and \"sha256\".\n+\n Naming Tests\n ------------\n \n-- \ngitgitgadget\n\n"},{"id":"403823","messageId":"8d481f3b223960f2193678929b0a1705222386b7.1597673089.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.v2.git.1597673089.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] commit-graph: use the \"hash version\" byte","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-17T14:04:47Z","receivedAt":"2020-08-17T14:05:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe commit-graph format reserved a byte among the header of the file to\nstore a \"hash version\". During the SHA-256 work, this was not modified\nbecause file formats are not necessarily intended to work across hash\nversions. If a repository has SHA-256 as its hash algorithm, it\nautomatically up-shifts the lengths of object names in all necessary\nformats.\n\nHowever, since we have this byte available for adjusting the version, we\ncan make the file formats more obviously incompatible instead of relying\non other context from the repository.\n\nUpdate the oid_version() method in commit-graph.c to add a new value, 2,\nfor sha-256. This automatically writes the new value in a SHA-256\nrepository _and_ verifies the value is correct. This is a breaking\nchange relative to the current 'master' branch since 092b677 (Merge\nbranch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking\nrelative to any released version of Git.\n\nThe test impact is relatively minor: the output of 'test-tool\nread-graph' lists the header information, so those instances of '1' need\nto be replaced with a variable determined by GIT_TEST_DEFAULT_HASH. A\nmore careful test is added that specifically creates a repository of\neach type then swaps the commit-graph files. The important value here is\nthat the \"git log\" command succeeds while writing a message to stderr.\n\nHelped-by: brian m. carlson \nSigned-off-by: Derrick Stolee \n---\n .../technical/commit-graph-format.txt | 9 ++++-\n commit-graph.c | 9 ++++-\n t/t4216-log-bloom.sh | 9 ++++-\n t/t5318-commit-graph.sh | 38 ++++++++++++++++++-\n t/t5324-split-commit-graph.sh | 5 ++-\n 5 files changed, 62 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt\nindex 440541045d..6ddbceba15 100644\n--- a/Documentation/technical/commit-graph-format.txt\n+++ b/Documentation/technical/commit-graph-format.txt\n@@ -42,8 +42,13 @@ HEADER:\n 1-byte version number:\n Currently, the only valid version is 1.\n \n- 1-byte Hash Version (1 = SHA-1)\n- We infer the hash length (H) from this value.\n+ 1-byte Hash Version\n+ We infer the hash length (H) from this value:\n+\t1 => SHA-1\n+\t2 => SHA-256\n+ If the hash type does not match the repository's hash algorithm, the\n+ commit-graph file should be ignored with a warning presented to the\n+ user.\n \n 1-byte number (C) of \"chunks\"\n \ndiff --git a/commit-graph.c b/commit-graph.c\nindex e51c91dd5b..0ed003e218 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -179,7 +179,14 @@ static char *get_chain_filename(struct object_directory *odb)\n \n static uint8_t oid_version(void)\n {\n-\treturn 1;\n+\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n+\tcase GIT_HASH_SHA1:\n+\t\treturn 1;\n+\tcase GIT_HASH_SHA256:\n+\t\treturn 2;\n+\tdefault:\n+\t\tdie(_(\"invalid hash version\"));\n+\t}\n }\n \n static struct commit_graph *alloc_commit_graph(void)\ndiff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\nindex c21cc160f3..4bb9e9dbe2 100755\n--- a/t/t4216-log-bloom.sh\n+++ b/t/t4216-log-bloom.sh\n@@ -30,12 +30,17 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '\n \trm file_to_be_deleted &&\n \tgit add . &&\n \tgit commit -m \"file removed\" &&\n-\tgit commit-graph write --reachable --changed-paths\n+\tgit commit-graph write --reachable --changed-paths &&\n+\n+\ttest_oid_cache <<-EOF\n+\toid_version sha1:1\n+\toid_version sha256:2\n+\tEOF\n '\n graph_read_expect () {\n \tNUM_CHUNKS=5\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 $NUM_CHUNKS 0\n+\theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data\n \tEOF\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 044cf8a3de..2ed0c1544d 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -10,7 +10,12 @@ test_expect_success 'setup full repo' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n \tgit init &&\n \tgit config core.commitGraph true &&\n-\tobjdir=\".git/objects\"\n+\tobjdir=\".git/objects\" &&\n+\n+\ttest_oid_cache <<-EOF\n+\toid_version sha1:1\n+\toid_version sha256:2\n+\tEOF\n '\n \n test_expect_success POSIXPERM 'tweak umask for modebit tests' '\n@@ -77,7 +82,7 @@ graph_read_expect() {\n \t\tNUM_CHUNKS=$((3 + $(echo \"$2\" | wc -w)))\n \tfi\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 $NUM_CHUNKS 0\n+\theader: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata$OPTIONAL\n \tEOF\n@@ -412,6 +417,35 @@ test_expect_success 'replace-objects invalidates commit-graph' '\n \t)\n '\n \n+test_expect_success 'warn on improper hash version' '\n+\tgit init --object-format=sha1 sha1 &&\n+\t(\n+\t\tcd sha1 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha1\n+\t) &&\n+\tgit init --object-format=sha256 sha256 &&\n+\t(\n+\t\tcd sha256 &&\n+\t\ttest_commit 1 &&\n+\t\tgit commit-graph write --reachable &&\n+\t\tmv .git/objects/info/commit-graph ../cg-sha256\n+\t) &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tmv ../cg-sha256 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 2 does not match version 1\" err\n+\t) &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tmv ../cg-sha1 .git/objects/info/commit-graph &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"commit-graph hash version 1 does not match version 2\" err\n+\t)\n+'\n+\n # the verify tests below expect the commit-graph to contain\n # exactly the commits reachable from the commits/8 branch.\n # If the file changes the set of commits in the list, then the\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex ea28d522b8..18216463c7 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -18,6 +18,9 @@ test_expect_success 'setup repo' '\n \n \tbase sha1:1376\n \tbase sha256:1496\n+\n+\toid_version sha1:1\n+\toid_version sha256:2\n \tEOM\n '\n \n@@ -28,7 +31,7 @@ graph_read_expect() {\n \t\tNUM_BASE=$2\n \tfi\n \tcat >expect <<- EOF\n-\theader: 43475048 1 1 3 $NUM_BASE\n+\theader: 43475048 1 $(test_oid oid_version) 3 $NUM_BASE\n \tnum_commits: $1\n \tchunks: oid_fanout oid_lookup commit_metadata\n \tEOF\n-- \ngitgitgadget\n\n"},{"id":"403824","messageId":"822e46868f52e5518681b6fe433eac0b2072b130.1597673089.git.gitgitgadget@gmail.com","threadId":"54045","inReplyTo":"pull.703.v2.git.1597673089.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] multi-pack-index: use hash version byte","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-08-17T14:04:48Z","receivedAt":"2020-08-17T14:05:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSimilar to the commit-graph format, the multi-pack-index format has a\nbyte in the header intended to track the hash version used to write the\nfile. This allows one to interpret the hash length without having the\ncontext of the repository config specifying the hash length. This was\nnot modified as part of the SHA-256 work because the hash length was\nautomatically up-shifted due to that config.\n\nSince we have this byte available, we can make the file formats more\nobviously incompatible instead of relying on other context from the\nrepository.\n\nAdd a new oid_version() method in midx.c similar to the one in\ncommit-graph.c. This is specifically made separate from that\nimplementation to avoid artificially linking the formats.\n\nThe test impact requires a few more things than the corresponding change\nin the commit-graph format. Specifically, 'test-tool read-midx' was not\nwriting anything about this header value to output. Since the value\navailable in 'struct multi_pack_index' is hash_len instead of a version\nvalue, we output \"20\" or \"32\" instead of \"1\" or \"2\".\n\nSince we want a user to not have their Git commands fail if their\nmulti-pack-index has the incorrect hash version compared to the\nrepository's hash version, we relax the die() to an error() in\nload_multi_pack_index(). This has some effect on 'git multi-pack-index\nverify' as we need to check that a failed parse of a file that exists is\nactually a verify error. For that test that checks the hash version\nmatches, we change the corrupted byte from \"2\" to \"3\" to ensure the test\nfails for both hash algorithms.\n\nHelped-by: brian m. carlson \nSigned-off-by: Derrick Stolee \n---\n Documentation/technical/pack-format.txt | 7 +++-\n midx.c | 35 ++++++++++++++++----\n t/helper/test-read-midx.c | 8 +++--\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++---\n 4 files changed, 80 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/technical/pack-format.txt b/Documentation/technical/pack-format.txt\nindex d3a142c652..16cf7e83aa 100644\n--- a/Documentation/technical/pack-format.txt\n+++ b/Documentation/technical/pack-format.txt\n@@ -273,7 +273,12 @@ HEADER:\n \t Git only writes or recognizes version 1.\n \n \t1-byte Object Id Version\n-\t Git only writes or recognizes version 1 (SHA1).\n+\t We infer the length of object IDs (OIDs) from this value:\n+\t\t1 => SHA-1\n+\t\t2 => SHA-256\n+\t If the hash type does not match the repository's hash algorithm,\n+\t the multi-pack-index file should be ignored with a warning\n+\t presented to the user.\n \n \t1-byte number of \"chunks\"\n \ndiff --git a/midx.c b/midx.c\nindex a5fb797ede..551a30b907 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -17,7 +17,6 @@\n #define MIDX_BYTE_HASH_VERSION 5\n #define MIDX_BYTE_NUM_CHUNKS 6\n #define MIDX_BYTE_NUM_PACKS 8\n-#define MIDX_HASH_VERSION 1\n #define MIDX_HEADER_SIZE 12\n #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz)\n \n@@ -36,6 +35,18 @@\n \n #define PACK_EXPIRED UINT_MAX\n \n+static uint8_t oid_version(void)\n+{\n+\tswitch (hash_algo_by_ptr(the_hash_algo)) {\n+\tcase GIT_HASH_SHA1:\n+\t\treturn 1;\n+\tcase GIT_HASH_SHA256:\n+\t\treturn 2;\n+\tdefault:\n+\t\tdie(_(\"invalid hash version\"));\n+\t}\n+}\n+\n static char *get_midx_filename(const char *object_dir)\n {\n \treturn xstrfmt(\"%s/pack/multi-pack-index\", object_dir);\n@@ -90,8 +101,11 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \t\t m->version);\n \n \thash_version = m->data[MIDX_BYTE_HASH_VERSION];\n-\tif (hash_version != MIDX_HASH_VERSION)\n-\t\tdie(_(\"hash version %u does not match\"), hash_version);\n+\tif (hash_version != oid_version()) {\n+\t\terror(_(\"multi-pack-index hash version %u does not match version %u\"),\n+\t\t hash_version, oid_version());\n+\t\tgoto cleanup_fail;\n+\t}\n \tm->hash_len = the_hash_algo->rawsz;\n \n \tm->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];\n@@ -418,7 +432,7 @@ static size_t write_midx_header(struct hashfile *f,\n \n \thashwrite_be32(f, MIDX_SIGNATURE);\n \tbyte_values[0] = MIDX_VERSION;\n-\tbyte_values[1] = MIDX_HASH_VERSION;\n+\tbyte_values[1] = oid_version();\n \tbyte_values[2] = num_chunks;\n \tbyte_values[3] = 0; /* unused */\n \thashwrite(f, byte_values, sizeof(byte_values));\n@@ -1105,8 +1119,17 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n-\tif (!m)\n-\t\treturn 0;\n+\tif (!m) {\n+\t\tint result = 0;\n+\t\tstruct stat sb;\n+\t\tchar *filename = get_midx_filename(object_dir);\n+\t\tif (!stat(filename, &sb)) {\n+\t\t\terror(_(\"multi-pack-index file exists, but failed to parse\"));\n+\t\t\tresult = 1;\n+\t\t}\n+\t\tfree(filename);\n+\t\treturn result;\n+\t}\n \n \tif (flags & MIDX_PROGRESS)\n \t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 831b586d02..2430880f78 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -7,14 +7,18 @@\n static int read_midx_file(const char *object_dir)\n {\n \tuint32_t i;\n-\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tstruct multi_pack_index *m;\n+\n+\tsetup_git_directory();\n+\tm = load_multi_pack_index(object_dir, 1);\n \n \tif (!m)\n \t\treturn 1;\n \n-\tprintf(\"header: %08x %d %d %d\\n\",\n+\tprintf(\"header: %08x %d %d %d %d\\n\",\n \t m->signature,\n \t m->version,\n+\t m->hash_len,\n \t m->num_chunks,\n \t m->num_packs);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 7dfff0f8f4..09cbca4949 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -5,6 +5,8 @@ test_description='multi-pack-indexes'\n \n objdir=.git/objects\n \n+HASH_LEN=$(test_oid rawsz)\n+\n midx_read_expect () {\n \tNUM_PACKS=$1\n \tNUM_OBJECTS=$2\n@@ -13,7 +15,7 @@ midx_read_expect () {\n \tEXTRA_CHUNKS=\"$5\"\n \t{\n \t\tcat <<-EOF &&\n-\t\theader: 4d494458 1 $NUM_CHUNKS $NUM_PACKS\n+\t\theader: 4d494458 1 $HASH_LEN $NUM_CHUNKS $NUM_PACKS\n \t\tchunks: pack-names oid-fanout oid-lookup object-offsets$EXTRA_CHUNKS\n \t\tnum_objects: $NUM_OBJECTS\n \t\tpacks:\n@@ -46,7 +48,7 @@ test_expect_success \"don't write midx with no packs\" '\n \ttest_path_is_missing pack/multi-pack-index\n '\n \n-test_expect_success \"Warn if a midx contains no oid\" '\n+test_expect_success SHA1 'warn if a midx contains no oid' '\n \tcp \"$TEST_DIRECTORY\"/t5319/no-objects.midx $objdir/pack/multi-pack-index &&\n \ttest_must_fail git multi-pack-index verify &&\n \trm $objdir/pack/multi-pack-index\n@@ -198,6 +200,40 @@ test_expect_success 'write midx with twelve packs' '\n \n compare_results_with_midx \"twelve packs\"\n \n+test_expect_success 'warn on improper hash version' '\n+\tgit init --object-format=sha1 sha1 &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tgit config core.multiPackIndex true &&\n+\t\ttest_commit 1 &&\n+\t\tgit repack -a &&\n+\t\tgit multi-pack-index write &&\n+\t\tmv .git/objects/pack/multi-pack-index ../mpi-sha1\n+\t) &&\n+\tgit init --object-format=sha256 sha256 &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tgit config core.multiPackIndex true &&\n+\t\ttest_commit 1 &&\n+\t\tgit repack -a &&\n+\t\tgit multi-pack-index write &&\n+\t\tmv .git/objects/pack/multi-pack-index ../mpi-sha256\n+\t) &&\n+\t(\n+\t\tcd sha1 &&\n+\t\tmv ../mpi-sha256 .git/objects/pack/multi-pack-index &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"multi-pack-index hash version 2 does not match version 1\" err\n+\t) &&\n+\t(\n+\t\tcd sha256 &&\n+\t\tmv ../mpi-sha1 .git/objects/pack/multi-pack-index &&\n+\t\tgit log -1 2>err &&\n+\t\ttest_i18ngrep \"multi-pack-index hash version 1 does not match version 2\" err\n+\t)\n+'\n+\n+\n test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n@@ -243,7 +279,6 @@ test_expect_success 'verify bad signature' '\n \t\t\"multi-pack-index signature\"\n '\n \n-HASH_LEN=$(test_oid rawsz)\n NUM_OBJECTS=74\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n@@ -272,7 +307,7 @@ test_expect_success 'verify bad version' '\n '\n \n test_expect_success 'verify bad OID version' '\n-\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\02\" $objdir \\\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\03\" $objdir \\\n \t\t\"hash version\"\n '\n \n-- \ngitgitgadget\n"},{"id":"403908","messageId":"20200817231215.GT8085@camp.crustytoothpaste.net","threadId":"54045","inReplyTo":"pull.703.v2.git.1597673089.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] SHA-256: Update commit-graph and multi-pack-index formats","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-08-17T23:12:15Z","receivedAt":"2020-08-17T23:12:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-08-17 at 14:04:45, Derrick Stolee via GitGitGadget wrote:\n> As discussed [1], there is some concern around binary file formats requiring\n> the context of the repository config in order to infer hash lengths. Two\n> formats that were designed with the hash transition in mind (commit-graph\n> and multi-pack-index) have bytes available to indicate the hash algorithm\n> used. Let's actually update these formats to be more self-contained with the\n> two hash algorithms being available.\n> \n> [1] \n> https://lore.kernel.org/git/CAN0heSp024=Kyy7gdQ2VSetk_5iVhj_qdT8CMVPcry_AwWrhHQ@mail.gmail.com/\n> \n> This merges cleanly with tb/bloom-improvements, but both that branch and\n> this patch series have merge conflicts with the corrected commit date patch\n> series [2].\n> \n> [2] \n> https://lore.kernel.org/git/pull.676.v2.git.1596941624.gitgitgadget@gmail.com/\n\nThis series looked sane to me. Thanks for the patches, and feel free to\nhave my\n\nReviewed-by: brian m. carlson \n-- \nbrian m. carlson: Houston, Texas, US\n"}]}