{"thread":{"id":"52869","subject":"[PATCH v2 00/24] SHA-256 stage 4 implementation, part 1/3","startedAt":"2020-02-22T20:18:03Z","lastAt":"2020-06-05T22:53:58Z","messageCount":51,"participants":["brian m. carlson","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":24},"messages":[{"id":"392310","messageId":"20200222201749.937983-1-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":null,"subject":"[PATCH v2 00/24] SHA-256 stage 4 implementation, part 1/3","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:25Z","receivedAt":"2020-02-22T20:18:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This is a series for part 1 of 3 of a stage 4 SHA-256 implementation.\nIt is mostly the same as v1[0], which was RFC. The interested reader is\nreferred there for the in-depth explanations.\n\nA few interesting changes have taken place since v1. First, I\ndiscovered a regression in a recent series which introduced a hard-coded\nconstant, so patch 1 addresses this.\n\nSecond, I discovered a mistaken assumption that we were making about our\nhash implementations: that copying the struct was sufficient to copy the\ncontext. This is not true for libgcrypt, where our context is a pointer\ninstead, so patch 2 addresses this with a helper function.\n\nFinally, I've added a check to prevent non-developers from creating\nSHA-256 repositories, since this series is not sufficient to implement\nfull SHA-256 support. Even as a developer, creating a SHA-256\nrepository immediately leads to a broken state, since we don't recognize\nthe extension (yet) and therefore promptly refuse to operate on it in\nany way. Preventing this experience seemed prudent.\n\nThere are a few other minor changes indicated in the range-diff below.\n\nIf folks have opinions about things like option names (or really,\nanything else), I'd love to hear them.\n\nThere are two more parts for this implementation and one more set of\ntest fixes. They will be coming in due course.\n\n[0] https://lore.kernel.org/git/20200113124729.3684846-1-sandals@crustytoothpaste.net/\n\nbrian m. carlson (24):\n builtin/pack-objects: make hash agnostic\n hash: implement and use a context cloning function\n hex: introduce parsing variants taking hash algorithms\n hex: add functions to parse hex object IDs in any algorithm\n repository: require a build flag to use SHA-256\n t: use hash-specific lookup tables to define test constants\n t6300: abstract away SHA-1-specific constants\n t6300: make hash algorithm independent\n t/helper/test-dump-split-index: initialize git repository\n t/helper: initialize repository if necessary\n t/helper: make repository tests hash independent\n setup: allow check_repository_format to read repository format\n builtin/init-db: allow specifying hash algorithm on command line\n builtin/init-db: add environment variable for new repo hash\n init-db: move writing repo version into a function\n worktree: allow repository version 1\n commit: use expected signature header for SHA-256\n gpg-interface: improve interface for parsing tags\n tag: store SHA-256 signatures in a header\n fast-import: permit reading multiple marks files\n fast-import: add helper function for inserting mark object entries\n fast-import: make find_marks work on any mark set\n fast-import: add a generic function to iterate over marks\n fast-import: add options for rewriting submodules\n\n Documentation/git-fast-import.txt | 20 +++\n Documentation/git-init.txt | 7 +-\n Documentation/git.txt | 6 +\n builtin/clone.c | 2 +-\n builtin/commit.c | 2 +-\n builtin/fmt-merge-msg.c | 26 +++-\n builtin/init-db.c | 75 +++++++--\n builtin/mktag.c | 14 ++\n builtin/pack-objects.c | 2 +-\n builtin/receive-pack.c | 4 +-\n builtin/tag.c | 20 ++-\n cache.h | 25 ++-\n commit.c | 58 +++++--\n commit.h | 8 +\n config.mak.dev | 2 +\n csum-file.c | 2 +-\n fast-import.c | 246 ++++++++++++++++++++++--------\n gpg-interface.c | 17 ++-\n gpg-interface.h | 9 +-\n hash.h | 21 +++\n hex.c | 57 ++++++-\n log-tree.c | 14 +-\n path.c | 2 +-\n ref-filter.c | 23 ++-\n repository.c | 4 +\n sequencer.c | 2 +-\n setup.c | 6 +-\n sha1-file.c | 18 +++\n sha256/gcrypt.h | 6 +\n t/helper/test-dump-split-index.c | 2 +\n t/helper/test-repository.c | 14 +-\n t/t1450-fsck.sh | 24 +++\n t/t5801-remote-helpers.sh | 4 +-\n t/t6300-for-each-ref.sh | 61 +++++---\n t/t7004-tag.sh | 8 +-\n t/t7030-verify-tag.sh | 17 +++\n t/t7510-signed-commit.sh | 16 +-\n t/t9300-fast-import.sh | 109 +++++++++++++\n t/test-lib.sh | 29 ++--\n tag.c | 15 +-\n worktree.c | 10 +-\n 41 files changed, 810 insertions(+), 197 deletions(-)\n\nRange-diff against v1:\n -: ---------- > 1: 0037d0f950 builtin/pack-objects: make hash agnostic\n -: ---------- > 2: e6f632eb38 hash: implement and use a context cloning function\n 1: 17e86d7e8a = 3: 205c67144b hex: introduce parsing variants taking hash algorithms\n 2: 8a4e05f03f = 4: 09e3f4ca8e hex: add functions to parse hex object IDs in any algorithm\n 3: 4efa071dd6 = 5: 3df541b2f0 repository: require a build flag to use SHA-256\n 4: 0c26727e8c = 6: 1161b70d24 t: use hash-specific lookup tables to define test constants\n 5: 3aff50578d = 7: fd3a85f59f t6300: abstract away SHA-1-specific constants\n 6: 2736da7f60 = 8: fb93b0900b t6300: make hash algorithm independent\n 7: 343cd6c9b2 = 9: 1d873fc05a t/helper/test-dump-split-index: initialize git repository\n 8: 1c8e31fd41 = 10: 8e918b28d5 t/helper: initialize repository if necessary\n 9: 9dade56ee5 = 11: 7ae0d820eb t/helper: make repository tests hash independent\n10: b08bec90e7 = 12: f13b1465cf setup: allow check_repository_format to read repository format\n11: acfdc2b0c8 ! 13: 273a2c06ce builtin/init-db: allow specifying hash algorithm on command line\n @@ Commit message\n algorithm. Ensure that if we are writing a non-SHA-1 repository that we\n set the repository version to 1 and write the objectFormat extension.\n \n + Restrict this option to work only when ENABLE_SHA256 is set until the\n + codebase is in a situation to fully support this.\n +\n Signed-off-by: brian m. carlson \n \n ## Documentation/git-init.txt ##\n @@ builtin/init-db.c: static int create_default_files(const char *template_path,\n \t\t\texit(1);\n \t}\n \n ++#ifndef ENABLE_SHA256\n ++\tif (fmt->hash_algo != GIT_HASH_SHA1)\n ++\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n ++#endif\n ++\n +\tif (fmt->hash_algo != GIT_HASH_SHA1)\n +\t\trepo_version = GIT_REPO_VERSION_READ;\n +\n12: adbd01c070 = 14: 6f3c736259 builtin/init-db: add environment variable for new repo hash\n13: 21cdc16e15 ! 15: 3966b560b5 init-db: move writing repo version into a function\n @@ builtin/init-db.c: static int needs_work_tree_config(const char *git_dir, const\n +\tchar repo_version_string[10];\n +\tint repo_version = GIT_REPO_VERSION;\n +\n ++#ifndef ENABLE_SHA256\n ++\tif (hash_algo != GIT_HASH_SHA1)\n ++\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[hash_algo].name);\n ++#endif\n ++\n +\tif (hash_algo != GIT_HASH_SHA1)\n +\t\trepo_version = GIT_REPO_VERSION_READ;\n +\n @@ builtin/init-db.c: static int create_default_files(const char *template_path,\n \t\t\texit(1);\n \t}\n \n +-#ifndef ENABLE_SHA256\n +-\tif (fmt->hash_algo != GIT_HASH_SHA1)\n +-\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n +-#endif\n +-\n -\tif (fmt->hash_algo != GIT_HASH_SHA1)\n -\t\trepo_version = GIT_REPO_VERSION_READ;\n -\n14: 45423efc1c = 16: 5af56f4f6a worktree: allow repository version 1\n15: 7b99132acd ! 17: fb51a683f1 commit: use expected signature header for SHA-256\n @@ commit.c: int remove_signature(struct strbuf *buf)\n \n ## sequencer.c ##\n @@ sequencer.c: static int try_to_commit(struct repository *r,\n - \tif (parse_head(r, ¤t_head))\n \t\treturn -1;\n + \n \tif (flags & AMEND_MSG) {\n -\t\tconst char *exclude_gpgsig[] = { \"gpgsig\", NULL };\n +\t\tconst char *exclude_gpgsig[] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n16: b4eecdbc28 ! 18: 3afac82c18 gpg-interface: improve interface for parsing tags\n @@ log-tree.c: static int show_one_mergetag(struct commit *commit,\n +\tstruct strbuf payload = STRBUF_INIT;\n +\tstruct strbuf signature = STRBUF_INIT;\n \n - \thash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);\n - \ttag = lookup_tag(the_repository, &oid);\n + \thash_object_file(the_hash_algo, extra->value, extra->len,\n + \t\t\t type_name(OBJ_TAG), &oid);\n @@ log-tree.c: static int show_one_mergetag(struct commit *commit,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \tgpg_message_offset = verify_message.len;\n17: 5d9c8753b1 = 19: 5e6d1ccbb2 tag: store SHA-256 signatures in a header\n18: 4a279f679d = 20: fb15c66ba1 fast-import: permit reading multiple marks files\n19: 01ef63b04d = 21: 273468b0ac fast-import: add helper function for inserting mark object entries\n20: 2469dc1324 = 22: bba3cf497d fast-import: make find_marks work on any mark set\n21: 0ad1e4d3a8 = 23: 714ae09a8c fast-import: add a generic function to iterate over marks\n22: bc53f8aaec = 24: 5c50871f30 fast-import: add options for rewriting submodules\n"},{"id":"392311","messageId":"20200222201749.937983-2-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 01/24] builtin/pack-objects: make hash agnostic","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:26Z","receivedAt":"2020-02-22T20:18:06Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Avoid hard-coding a hash size, instead preferring to use the_hash_algo.\n\nSigned-off-by: brian m. carlson \n---\n builtin/pack-objects.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 940fbcb7b3..fceac7b462 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -879,7 +879,7 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out,\n \t\t\tlen = encode_in_pack_object_header(header, sizeof(header),\n \t\t\t\t\t\t\t OBJ_REF_DELTA, size);\n \t\t\thashwrite(out, header, len);\n-\t\t\thashwrite(out, base_sha1, 20);\n+\t\t\thashwrite(out, base_sha1, the_hash_algo->rawsz);\n \t\t\tcopy_pack_data(out, reuse_packfile, w_curs, cur, next - cur);\n \t\t\treturn;\n \t\t}\n"},{"id":"392312","messageId":"20200222201749.937983-4-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 03/24] hex: introduce parsing variants taking hash algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:28Z","receivedAt":"2020-02-22T20:18:07Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Introduce variants of get_oid_hex and parse_oid_hex that parse an\narbitrary hash algorithm, implementing internal functions to avoid\nduplication. These functions can be used in the transport code to parse\nrefs properly.\n\nSigned-off-by: brian m. carlson \n---\n cache.h | 7 +++++++\n hex.c | 35 +++++++++++++++++++++++++++--------\n 2 files changed, 34 insertions(+), 8 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 37c899b53f..4c9c21a11d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1481,6 +1481,9 @@ int set_disambiguate_hint_config(const char *var, const char *value);\n int get_sha1_hex(const char *hex, unsigned char *sha1);\n int get_oid_hex(const char *hex, struct object_id *sha1);\n \n+/* Like get_oid_hex, but for an arbitrary hash algorithm. */\n+int get_oid_hex_algop(const char *hex, struct object_id *oid, const struct git_hash_algo *algop);\n+\n /*\n * Read `len` pairs of hexadecimal digits from `hex` and write the\n * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n@@ -1516,6 +1519,10 @@ char *oid_to_hex(const struct object_id *oid);\t\t\t\t\t\t/* same static buffer */\n */\n int parse_oid_hex(const char *hex, struct object_id *oid, const char **end);\n \n+/* Like parse_oid_hex, but for an arbitrary hash algorithm. */\n+int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end,\n+\t\t\tconst struct git_hash_algo *algo);\n+\n /*\n * This reads short-hand syntax that not only evaluates to a commit\n * object name, but also can act as if the end user spelled the name\ndiff --git a/hex.c b/hex.c\nindex fd7f00c43f..10e24dc2e4 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -47,30 +47,49 @@ int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)\n \treturn 0;\n }\n \n-int get_sha1_hex(const char *hex, unsigned char *sha1)\n+static int get_hash_hex_algop(const char *hex, unsigned char *hash,\n+\t\t\t const struct git_hash_algo *algop)\n {\n \tint i;\n-\tfor (i = 0; i < the_hash_algo->rawsz; i++) {\n+\tfor (i = 0; i < algop->rawsz; i++) {\n \t\tint val = hex2chr(hex);\n \t\tif (val < 0)\n \t\t\treturn -1;\n-\t\t*sha1++ = val;\n+\t\t*hash++ = val;\n \t\thex += 2;\n \t}\n \treturn 0;\n }\n \n+int get_sha1_hex(const char *hex, unsigned char *sha1)\n+{\n+\treturn get_hash_hex_algop(hex, sha1, the_hash_algo);\n+}\n+\n+int get_oid_hex_algop(const char *hex, struct object_id *oid,\n+\t\t const struct git_hash_algo *algop)\n+{\n+\treturn get_hash_hex_algop(hex, oid->hash, algop);\n+}\n+\n int get_oid_hex(const char *hex, struct object_id *oid)\n {\n-\treturn get_sha1_hex(hex, oid->hash);\n+\treturn get_oid_hex_algop(hex, oid, the_hash_algo);\n+}\n+\n+int parse_oid_hex_algop(const char *hex, struct object_id *oid,\n+\t\t\tconst char **end,\n+\t\t\tconst struct git_hash_algo *algop)\n+{\n+\tint ret = get_hash_hex_algop(hex, oid->hash, algop);\n+\tif (!ret)\n+\t\t*end = hex + algop->hexsz;\n+\treturn ret;\n }\n \n int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)\n {\n-\tint ret = get_oid_hex(hex, oid);\n-\tif (!ret)\n-\t\t*end = hex + the_hash_algo->hexsz;\n-\treturn ret;\n+\treturn parse_oid_hex_algop(hex, oid, end, the_hash_algo);\n }\n \n char *hash_to_hex_algop_r(char *buffer, const unsigned char *hash,\n"},{"id":"392313","messageId":"20200222201749.937983-8-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 07/24] t6300: abstract away SHA-1-specific constants","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:32Z","receivedAt":"2020-02-22T20:18:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Adjust the test so that it computes variables for object IDs instead of\nusing hard-coded hashes.\n\nSigned-off-by: brian m. carlson \n---\n t/t6300-for-each-ref.sh | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 9c910ce746..2406b93d35 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -20,6 +20,10 @@ setdate_and_increment () {\n }\n \n test_expect_success setup '\n+\ttest_oid_cache <<-EOF &&\n+\tdisklen sha1:138\n+\tdisklen sha256:154\n+\tEOF\n \tsetdate_and_increment &&\n \techo \"Using $datestamp\" > one &&\n \tgit add one &&\n@@ -50,6 +54,9 @@ test_atom() {\n \t\"\n }\n \n+hexlen=$(test_oid hexsz)\n+disklen=$(test_oid disklen)\n+\n test_atom head refname refs/heads/master\n test_atom head refname: refs/heads/master\n test_atom head refname:short master\n@@ -82,9 +89,9 @@ test_atom head push:rstrip=-1 refs\n test_atom head push:strip=1 remotes/myfork/master\n test_atom head push:strip=-1 master\n test_atom head objecttype commit\n-test_atom head objectsize 171\n-test_atom head objectsize:disk 138\n-test_atom head deltabase 0000000000000000000000000000000000000000\n+test_atom head objectsize $((131 + hexlen))\n+test_atom head objectsize:disk $disklen\n+test_atom head deltabase $ZERO_OID\n test_atom head objectname $(git rev-parse refs/heads/master)\n test_atom head objectname:short $(git rev-parse --short refs/heads/master)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n@@ -125,11 +132,11 @@ test_atom tag refname:short testtag\n test_atom tag upstream ''\n test_atom tag push ''\n test_atom tag objecttype tag\n-test_atom tag objectsize 154\n-test_atom tag objectsize:disk 138\n-test_atom tag '*objectsize:disk' 138\n-test_atom tag deltabase 0000000000000000000000000000000000000000\n-test_atom tag '*deltabase' 0000000000000000000000000000000000000000\n+test_atom tag objectsize $((114 + hexlen))\n+test_atom tag objectsize:disk $disklen\n+test_atom tag '*objectsize:disk' $disklen\n+test_atom tag deltabase $ZERO_OID\n+test_atom tag '*deltabase' $ZERO_OID\n test_atom tag objectname $(git rev-parse refs/tags/testtag)\n test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n@@ -139,7 +146,7 @@ test_atom tag parent ''\n test_atom tag numparent ''\n test_atom tag object $(git rev-parse refs/tags/testtag^0)\n test_atom tag type 'commit'\n-test_atom tag '*objectname' 'ea122842f48be4afb2d1fc6a4b96c05885ab7463'\n+test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})\n test_atom tag '*objecttype' 'commit'\n test_atom tag author ''\n test_atom tag authorname ''\n"},{"id":"392314","messageId":"20200222201749.937983-13-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 12/24] setup: allow check_repository_format to read repository format","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:37Z","receivedAt":"2020-02-22T20:18:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In some cases, we will want to not only check the repository format, but\nextract the information that we've gained. To do so, allow\ncheck_repository_format to take a pointer to struct repository_format.\nAllow passing NULL for this argument if we're not interested in the\ninformation, and pass NULL for all existing callers. A future patch\nwill make use of this information.\n\nSigned-off-by: brian m. carlson \n---\n builtin/init-db.c | 2 +-\n cache.h | 4 +++-\n path.c | 2 +-\n setup.c | 6 ++++--\n 4 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 944ec77fe1..b11f07064d 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -378,7 +378,7 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t * config file, so this will not fail. What we are catching\n \t * is an attempt to reinitialize new repository with an old tool.\n \t */\n-\tcheck_repository_format();\n+\tcheck_repository_format(NULL);\n \n \treinit = create_default_files(template_dir, original_git_dir);\n \ndiff --git a/cache.h b/cache.h\nindex 158d7ccfd8..29ee02a8d4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1086,8 +1086,10 @@ int verify_repository_format(const struct repository_format *format,\n * and die if it is a version we don't understand. Generally one would\n * set_git_dir() before calling this, and use it only for \"are we in a valid\n * repo?\".\n+ *\n+ * If successful and fmt is not NULL, fill fmt with data.\n */\n-void check_repository_format(void);\n+void check_repository_format(struct repository_format *fmt);\n \n #define MTIME_CHANGED\t0x0001\n #define CTIME_CHANGED\t0x0002\ndiff --git a/path.c b/path.c\nindex 88cf593007..a10b62c0c4 100644\n--- a/path.c\n+++ b/path.c\n@@ -851,7 +851,7 @@ const char *enter_repo(const char *path, int strict)\n \n \tif (is_git_directory(\".\")) {\n \t\tset_git_dir(\".\");\n-\t\tcheck_repository_format();\n+\t\tcheck_repository_format(NULL);\n \t\treturn path;\n \t}\n \ndiff --git a/setup.c b/setup.c\nindex 4ea7a0b081..af20c3d7c0 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1253,10 +1253,12 @@ int git_config_perm(const char *var, const char *value)\n \treturn -(i & 0666);\n }\n \n-void check_repository_format(void)\n+void check_repository_format(struct repository_format *fmt)\n {\n \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n-\tcheck_repository_format_gently(get_git_dir(), &repo_fmt, NULL);\n+\tif (!fmt)\n+\t\tfmt = &repo_fmt;\n+\tcheck_repository_format_gently(get_git_dir(), fmt, NULL);\n \tstartup_info->have_repository = 1;\n \tclear_repository_format(&repo_fmt);\n }\n"},{"id":"392315","messageId":"20200222201749.937983-12-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 11/24] t/helper: make repository tests hash independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:36Z","receivedAt":"2020-02-22T20:18:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This test currently hard-codes the hash algorithm as SHA-1 when calling\nrepo_set_hash_algo so that the_hash_algo is properly initialized.\nHowever, this does not work with SHA-256 repositories. Read the\nrepository value that repo_init has read into the local repository\nvariable and set the algorithm based on that value.\n\nSigned-off-by: brian m. carlson \n---\n t/helper/test-repository.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex ecc768e4cb..56f0e3c1be 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -19,12 +19,11 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,\n \n \tmemset(the_repository, 0, sizeof(*the_repository));\n \n-\t/* TODO: Needed for temporary hack in hashcmp, see 183a638b7da. */\n-\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n-\n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n \n+\trepo_set_hash_algo(the_repository, hash_algo_by_ptr(r.hash_algo));\n+\n \tc = lookup_commit(&r, commit_oid);\n \n \tif (!parse_commit_in_graph(&r, c))\n@@ -50,12 +49,11 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n \n \tmemset(the_repository, 0, sizeof(*the_repository));\n \n-\t/* TODO: Needed for temporary hack in hashcmp, see 183a638b7da. */\n-\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n-\n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n \n+\trepo_set_hash_algo(the_repository, hash_algo_by_ptr(r.hash_algo));\n+\n \tc = lookup_commit(&r, commit_oid);\n \n \t/*\n"},{"id":"392316","messageId":"20200222201749.937983-15-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 14/24] builtin/init-db: add environment variable for new repo hash","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:39Z","receivedAt":"2020-02-22T20:18:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"For the foreseeable future, SHA-1 will be the default algorithm for Git.\nHowever, when running the testsuite, we want to be able to test an\narbitrary algorithm. It would be quite burdensome and very untidy to\nhave to specify the algorithm we'd like to test every time we\ninitialized a new repository somewhere in the testsuite, so add an\nenvironment variable to allow us to specify the default hash algorithm\nfor Git.\n\nThis has the benefit that we can set it once for the entire testsuite\nand not have to think about it. In the future, users can also use it to\nset the default for their repositories if they would like to do so.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git.txt | 6 ++++++\n builtin/init-db.c | 9 +++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex b0672bd806..9d6769e95a 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -493,6 +493,12 @@ double-quotes and respecting backslash escapes. E.g., the value\n \tdetails. This variable has lower precedence than other path\n \tvariables such as GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY...\n \n+`GIT_DEFAULT_HASH_ALGORITHM`::\n+\tIf this variable is set, the default hash algorithm for new\n+\trepositories will be set to this value. This value is currently\n+\tignored when cloning; the setting of the remote repository\n+\tis used instead. The default is \"sha1\".\n+\n Git Commits\n ~~~~~~~~~~~\n `GIT_AUTHOR_NAME`::\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex d05552f0ae..ab4fd682ab 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -20,6 +20,8 @@\n #define TEST_FILEMODE 1\n #endif\n \n+#define GIT_DEFAULT_HASH_ENVIRONMENT \"GIT_DEFAULT_HASH\"\n+\n static int init_is_bare_repository = 0;\n static int init_shared_repository = -1;\n static const char *init_db_template_dir;\n@@ -356,6 +358,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link)\n \n static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash)\n {\n+\tconst char *env = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);\n \t/*\n \t * If we already have an initialized repo, don't allow the user to\n \t * specify a different algorithm, as that could cause corruption.\n@@ -365,6 +368,12 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash\n \t\tdie(_(\"attempt to reinitialize repository with different hash\"));\n \telse if (hash != GIT_HASH_UNKNOWN)\n \t\trepo_fmt->hash_algo = hash;\n+\telse if (env) {\n+\t\tint env_algo = hash_algo_by_name(env);\n+\t\tif (env_algo == GIT_HASH_UNKNOWN)\n+\t\t\tdie(_(\"unknown hash algorithm '%s'\"), env);\n+\t\trepo_fmt->hash_algo = env_algo;\n+\t}\n }\n \n int init_db(const char *git_dir, const char *real_git_dir,\n"},{"id":"392317","messageId":"20200222201749.937983-7-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 06/24] t: use hash-specific lookup tables to define test constants","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:31Z","receivedAt":"2020-02-22T20:18:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll allow developers to run the testsuite with a hash\nalgorithm of their choice. To make this easier, compute the fixed\nconstants using test_oid. Move the constant initialization down below\nthe point where test-lib-functions.sh is loaded so the functions are\ndefined.\n\nNote that we don't provide a value for the OID_REGEX value directly\nbecause writing a large number of instances of \"[0-9a-f]\" in the\noid-info files is unwieldy and there isn't a way to compute it based on\nthose values. Instead, compute it based on ZERO_OID.\n\nSigned-off-by: brian m. carlson \n---\n t/test-lib.sh | 29 ++++++++++++++---------------\n 1 file changed, 14 insertions(+), 15 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0ea1e5a05e..9fe390bd5a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -494,21 +494,6 @@ case $(echo $GIT_TRACE |tr \"[A-Z]\" \"[a-z]\") in\n \t;;\n esac\n \n-# Convenience\n-#\n-# A regexp to match 5, 35 and 40 hexdigits\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-\n-# Zero SHA-1\n-_z40=0000000000000000000000000000000000000000\n-\n-OID_REGEX=\"$_x40\"\n-ZERO_OID=$_z40\n-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n-EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\n-\n # Line feed\n LF='\n '\n@@ -1383,6 +1368,20 @@ then\n \tfi\n fi\n \n+# Convenience\n+# A regexp to match 5, 35 and 40 hexdigits\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+\n+test_oid_init\n+\n+ZERO_OID=$(test_oid zero)\n+OID_REGEX=$(echo $ZERO_OID | sed -e 's/0/[0-9a-f]/g')\n+EMPTY_TREE=$(test_oid empty_tree)\n+EMPTY_BLOB=$(test_oid empty_blob)\n+_z40=$ZERO_OID\n+\n # Provide an implementation of the 'yes' utility; the upper bound\n # limit is there to help Windows that cannot stop this loop from\n # wasting cycles when the downstream stops reading, so do not be\n"},{"id":"392318","messageId":"20200222201749.937983-11-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 10/24] t/helper: initialize repository if necessary","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:35Z","receivedAt":"2020-02-22T20:18:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The repository helper is used in t5318 to read commit graphs whether\nwe're in a repository or not. However, without a repository, we have no\nway to properly initialize the hash algorithm, meaning that the file is\nmisread.\n\nFix this by calling setup_git_directory_gently, which will read the\nenvironment variable the testsuite sets to ensure that the correct hash\nalgorithm is set.\n\nSigned-off-by: brian m. carlson \n---\n t/helper/test-repository.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex f7f8618445..ecc768e4cb 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -75,6 +75,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n \n int cmd__repository(int argc, const char **argv)\n {\n+\tint nongit_ok = 0;\n+\n+\tsetup_git_directory_gently(&nongit_ok);\n+\n \tif (argc < 2)\n \t\tdie(\"must have at least 2 arguments\");\n \tif (!strcmp(argv[1], \"parse_commit_in_graph\")) {\n"},{"id":"392319","messageId":"20200222201749.937983-23-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 22/24] fast-import: make find_marks work on any mark set","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:47Z","receivedAt":"2020-02-22T20:18:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll use multiple different mark sets with this\nfunction, so make it take an argument that points to the mark set to\noperate on.\n\nSigned-off-by: brian m. carlson \n---\n fast-import.c | 21 ++++++++++-----------\n 1 file changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 3ce4a04473..8aaa7f6289 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -517,10 +517,9 @@ static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry\n \ts->data.marked[idnum] = oe;\n }\n \n-static struct object_entry *find_mark(uintmax_t idnum)\n+static void *find_mark(struct mark_set *s, uintmax_t idnum)\n {\n \tuintmax_t orig_idnum = idnum;\n-\tstruct mark_set *s = marks;\n \tstruct object_entry *oe = NULL;\n \tif ((idnum >> s->shift) < 1024) {\n \t\twhile (s && s->shift) {\n@@ -2225,7 +2224,7 @@ static void file_change_m(const char *p, struct branch *b)\n \t}\n \n \tif (*p == ':') {\n-\t\toe = find_mark(parse_mark_ref_space(&p));\n+\t\toe = find_mark(marks, parse_mark_ref_space(&p));\n \t\toidcpy(&oid, &oe->idx.oid);\n \t} else if (skip_prefix(p, \"inline \", &p)) {\n \t\tinline_data = 1;\n@@ -2399,7 +2398,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa\n \t/* Now parse the notemodify command. */\n \t/* or 'inline' */\n \tif (*p == ':') {\n-\t\toe = find_mark(parse_mark_ref_space(&p));\n+\t\toe = find_mark(marks, parse_mark_ref_space(&p));\n \t\toidcpy(&oid, &oe->idx.oid);\n \t} else if (skip_prefix(p, \"inline \", &p)) {\n \t\tinline_data = 1;\n@@ -2420,7 +2419,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa\n \t\toidcpy(&commit_oid, &s->oid);\n \t} else if (*p == ':') {\n \t\tuintmax_t commit_mark = parse_mark_ref_eol(p);\n-\t\tstruct object_entry *commit_oe = find_mark(commit_mark);\n+\t\tstruct object_entry *commit_oe = find_mark(marks, commit_mark);\n \t\tif (commit_oe->type != OBJ_COMMIT)\n \t\t\tdie(\"Mark :%\" PRIuMAX \" not a commit\", commit_mark);\n \t\toidcpy(&commit_oid, &commit_oe->idx.oid);\n@@ -2524,7 +2523,7 @@ static int parse_objectish(struct branch *b, const char *objectish)\n \t\toidcpy(&b->branch_tree.versions[1].oid, t);\n \t} else if (*objectish == ':') {\n \t\tuintmax_t idnum = parse_mark_ref_eol(objectish);\n-\t\tstruct object_entry *oe = find_mark(idnum);\n+\t\tstruct object_entry *oe = find_mark(marks, idnum);\n \t\tif (oe->type != OBJ_COMMIT)\n \t\t\tdie(\"Mark :%\" PRIuMAX \" not a commit\", idnum);\n \t\tif (!oideq(&b->oid, &oe->idx.oid)) {\n@@ -2588,7 +2587,7 @@ static struct hash_list *parse_merge(unsigned int *count)\n \t\t\toidcpy(&n->oid, &s->oid);\n \t\telse if (*from == ':') {\n \t\t\tuintmax_t idnum = parse_mark_ref_eol(from);\n-\t\t\tstruct object_entry *oe = find_mark(idnum);\n+\t\t\tstruct object_entry *oe = find_mark(marks, idnum);\n \t\t\tif (oe->type != OBJ_COMMIT)\n \t\t\t\tdie(\"Mark :%\" PRIuMAX \" not a commit\", idnum);\n \t\t\toidcpy(&n->oid, &oe->idx.oid);\n@@ -2762,7 +2761,7 @@ static void parse_new_tag(const char *arg)\n \t} else if (*from == ':') {\n \t\tstruct object_entry *oe;\n \t\tfrom_mark = parse_mark_ref_eol(from);\n-\t\toe = find_mark(from_mark);\n+\t\toe = find_mark(marks, from_mark);\n \t\ttype = oe->type;\n \t\toidcpy(&oid, &oe->idx.oid);\n \t} else if (!get_oid(from, &oid)) {\n@@ -2920,7 +2919,7 @@ static void parse_get_mark(const char *p)\n \tif (*p != ':')\n \t\tdie(\"Not a mark: %s\", p);\n \n-\toe = find_mark(parse_mark_ref_eol(p));\n+\toe = find_mark(marks, parse_mark_ref_eol(p));\n \tif (!oe)\n \t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \n@@ -2935,7 +2934,7 @@ static void parse_cat_blob(const char *p)\n \n \t/* cat-blob SP LF */\n \tif (*p == ':') {\n-\t\toe = find_mark(parse_mark_ref_eol(p));\n+\t\toe = find_mark(marks, parse_mark_ref_eol(p));\n \t\tif (!oe)\n \t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \t\toidcpy(&oid, &oe->idx.oid);\n@@ -3010,7 +3009,7 @@ static struct object_entry *parse_treeish_dataref(const char **p)\n \tstruct object_entry *e;\n \n \tif (**p == ':') {\t/* */\n-\t\te = find_mark(parse_mark_ref_space(p));\n+\t\te = find_mark(marks, parse_mark_ref_space(p));\n \t\tif (!e)\n \t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \t\toidcpy(&oid, &e->idx.oid);\n"},{"id":"392320","messageId":"20200222201749.937983-20-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 19/24] tag: store SHA-256 signatures in a header","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:44Z","receivedAt":"2020-02-22T20:18:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll want to allow a user to sign both the SHA-1 version\nof a tag and the SHA-256 version at the same time. Since for SHA-1 the\nsignature is appended to the tag message, we must use a different way to\nallow multiple signature.\n\nThe transition plan envisions this using a gpgsig-sha256 header, much as\nfor commits. Refactor the commit code that performs parsing of this\nheader and use it for tags that use SHA-256. Check that we get tags in\nthe correct format depending on what algorithm we're using.\n\nNote that currently we have no way to rewrite an object into another\nhash algorithm, and therefore we don't have a way to verify the\nsignatures of the other hash algorithm. Because of the way the\nsignatures are stored, we'll reject commits signed with both algorithms,\nwhich is essential for security. If we allowed both signatures despite\nnot being able to verify them and one signature were invalid, we'd end\nup with a security problem.\n\nThere are, however, a few things to note.\n\nIn the ref-filter code, we hoist the signature parsing above the blank\nline delimiting headers and body so we can find the signature when using\nSHA-256. For similar reasons, t6300 no longer emits the signature as\npart of the body since it's no longer part of the body.\n\nWe mark a test for exporting signatures with remote helpers as SHA-1\nonly. Since we no longer have signatures in the body (those are for\nSHA-1 only), they cannot be exported this way.\n\nSigned-off-by: brian m. carlson \n---\n builtin/mktag.c | 14 ++++++++++++++\n builtin/tag.c | 4 +++-\n commit.c | 19 +++++++++++++++----\n commit.h | 8 ++++++++\n gpg-interface.c | 16 ++++++++++------\n ref-filter.c | 7 +++----\n t/t5801-remote-helpers.sh | 4 +++-\n t/t6300-for-each-ref.sh | 34 +++++++++++++++++++++++++---------\n t/t7004-tag.sh | 8 +++++++-\n t/t7030-verify-tag.sh | 17 +++++++++++++++++\n 10 files changed, 105 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/mktag.c b/builtin/mktag.c\nindex 4982d3a93e..ce1665c4e4 100644\n--- a/builtin/mktag.c\n+++ b/builtin/mktag.c\n@@ -144,6 +144,20 @@ static int verify_tag(char *buffer, unsigned long size)\n \t\t\t(uintmax_t) (tagger_line - buffer));\n \ttagger_line += 6;\n \n+\tif (hash_algo_by_ptr(the_hash_algo) == GIT_HASH_SHA256 &&\n+\t\t!memcmp(tagger_line, \"gpgsig-sha256 \", 14)) {\n+\t\tchar *p = strpbrk(tagger_line + 1, \"\\n\");\n+\t\tif (!p)\n+\t\t\treturn error(\"char%\"PRIuMAX\": could not find end of line\",\n+\t\t\t\t(uintmax_t) (tagger_line - buffer));\n+\t\ttagger_line = p + 1;\n+\t\twhile (*tagger_line == ' ' && (p = strpbrk(tagger_line, \"\\n\")))\n+\t\t\ttagger_line = p + 1;\n+\t\tif (!p)\n+\t\t\treturn error(\"char%\"PRIuMAX\": could not find end of line\",\n+\t\t\t\t(uintmax_t) (tagger_line - buffer));\n+\t}\n+\n \t/* Verify the blank line separating the header from the body */\n \tif (*tagger_line != '\\n')\n \t\treturn error(\"char%\"PRIuMAX\": trailing garbage in tag header\",\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 6b95c6a2ea..ab6b5044e6 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -128,7 +128,9 @@ static int verify_tag(const char *name, const char *ref,\n \n static int do_sign(struct strbuf *buffer)\n {\n-\treturn sign_buffer(buffer, buffer, get_signing_key());\n+\tif (hash_algo_by_ptr(the_hash_algo) == GIT_HASH_SHA1)\n+\t\treturn sign_buffer(buffer, buffer, get_signing_key());\n+\treturn sign_with_header(buffer, get_signing_key());\n }\n \n static const char tag_template[] =\ndiff --git a/commit.c b/commit.c\nindex d39ce5076d..e3e4f74a36 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -970,7 +970,7 @@ static const char *gpg_sig_headers[] = {\n \t\"gpgsig-sha256\",\n };\n \n-static int do_sign_commit(struct strbuf *buf, const char *keyid)\n+int sign_with_header(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n@@ -1010,12 +1010,24 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n \treturn 0;\n }\n \n+\n+\n int parse_signed_commit(const struct commit *commit,\n \t\t\tstruct strbuf *payload, struct strbuf *signature)\n {\n-\n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n+\tint ret = parse_buffer_signed_by_header(buffer, size, payload, signature);\n+\n+\tunuse_commit_buffer(commit, buffer);\n+\treturn ret;\n+}\n+\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature)\n+{\n \tint in_signature, saw_signature = -1;\n \tconst char *line, *tail;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n@@ -1048,7 +1060,6 @@ int parse_signed_commit(const struct commit *commit,\n \t\t}\n \t\tline = next;\n \t}\n-\tunuse_commit_buffer(commit, buffer);\n \treturn saw_signature;\n }\n \n@@ -1490,7 +1501,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,\n \tif (encoding_is_utf8 && !verify_utf8(&buffer))\n \t\tfprintf(stderr, _(commit_utf8_warn));\n \n-\tif (sign_commit && do_sign_commit(&buffer, sign_commit)) {\n+\tif (sign_commit && sign_with_header(&buffer, sign_commit)) {\n \t\tresult = -1;\n \t\tgoto out;\n \t}\ndiff --git a/commit.h b/commit.h\nindex 008a0fa4a0..e9898df412 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -402,4 +402,12 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void\n LAST_ARG_MUST_BE_NULL\n int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);\n \n+/* Sign a commit or tag buffer, storing the result in a header. */\n+int sign_with_header(struct strbuf *buf, const char *keyid);\n+/* Parse the signature out of a header. */\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature);\n+\n #endif /* COMMIT_H */\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b25f5c21d8..a63766a19b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -363,13 +364,16 @@ size_t parse_signed_buffer(const char *buf, size_t size)\n \n int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n {\n-\tsize_t match = parse_signed_buffer(buf, size);\n-\tif (match != size) {\n-\t\tstrbuf_add(payload, buf, match);\n-\t\tstrbuf_add(signature, buf + match, size - match);\n-\t\treturn 1;\n+\tif (hash_algo_by_ptr(the_hash_algo) == GIT_HASH_SHA1) {\n+\t\tsize_t match = parse_signed_buffer(buf, size);\n+\t\tif (match != size) {\n+\t\t\tstrbuf_add(payload, buf, match);\n+\t\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\t\treturn 1;\n+\t\t}\n+\t\treturn 0;\n \t}\n-\treturn 0;\n+\treturn parse_buffer_signed_by_header(buf, size, payload, signature);\n }\n \n void set_signing_key(const char *key)\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 212f1165bb..933530a14c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1167,6 +1167,8 @@ static void find_subpos(const char *buf,\n \tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n+\t/* parse signature first; we might not even have a subject line */\n+\tparse_signature(buf, end - buf, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1178,9 +1180,6 @@ static void find_subpos(const char *buf,\n \t/* skip any empty lines */\n \twhile (*buf == '\\n')\n \t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n \t*sig = strbuf_detach(&signature, siglen);\n \tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n@@ -1267,7 +1266,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (atom->u.contents.option == C_LINES) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\t\t\tconst char *contents_end = bodypos + nonsiglen;\n \n \t\t\t/* Size is the length of the message after removing the signature */\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 121e5c6edb..801802be9d 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -192,7 +192,9 @@ test_expect_success GPG 'push signed tag' '\n \ttest_must_fail compare_refs local signed-tag server signed-tag\n '\n \n-test_expect_success GPG 'push signed tag with signed-tags capability' '\n+# SHA-256 signatures are stored in the header and can't be round-tripped through\n+# fast-export.\n+test_expect_success GPG,SHA1 'push signed tag with signed-tags capability' '\n \t(cd local &&\n \tgit checkout master &&\n \tgit tag -s -m signed-tag signed-tag-2 &&\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex b3c1092338..caeabfb293 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -54,6 +54,17 @@ test_atom() {\n \t\"\n }\n \n+test_atom_hash () {\n+\tlocal val\n+\tif [ \"$(test_oid algo)\" = sha1 ]\n+\tthen\n+\t\tval=\"$3\"\n+\telse\n+\t\tval=\"$4\"\n+\tfi\n+\ttest_atom \"$1\" \"$2\" \"$val\"\n+}\n+\n hexlen=$(test_oid hexsz)\n disklen=$(test_oid disklen)\n \n@@ -625,30 +636,35 @@ sig='-----BEGIN PGP SIGNATURE-----\n PREREQ=GPG\n test_atom refs/tags/signed-empty subject ''\n test_atom refs/tags/signed-empty contents:subject ''\n-test_atom refs/tags/signed-empty body \"$sig\"\n+test_atom_hash refs/tags/signed-empty body \"$sig\" ''\n test_atom refs/tags/signed-empty contents:body ''\n test_atom refs/tags/signed-empty contents:signature \"$sig\"\n-test_atom refs/tags/signed-empty contents \"$sig\"\n+test_atom_hash refs/tags/signed-empty contents \"$sig\" ''\n \n test_atom refs/tags/signed-short subject 'subject line'\n test_atom refs/tags/signed-short contents:subject 'subject line'\n-test_atom refs/tags/signed-short body \"$sig\"\n+test_atom_hash refs/tags/signed-short body \"$sig\" ''\n test_atom refs/tags/signed-short contents:body ''\n test_atom refs/tags/signed-short contents:signature \"$sig\"\n-test_atom refs/tags/signed-short contents \"subject line\n-$sig\"\n+test_atom_hash refs/tags/signed-short contents \"subject line\n+$sig\" 'subject line\n+'\n \n test_atom refs/tags/signed-long subject 'subject line'\n test_atom refs/tags/signed-long contents:subject 'subject line'\n-test_atom refs/tags/signed-long body \"body contents\n-$sig\"\n+test_atom_hash refs/tags/signed-long body \"body contents\n+$sig\" 'body contents\n+'\n test_atom refs/tags/signed-long contents:body 'body contents\n '\n test_atom refs/tags/signed-long contents:signature \"$sig\"\n-test_atom refs/tags/signed-long contents \"subject line\n+test_atom_hash refs/tags/signed-long contents \"subject line\n \n body contents\n-$sig\"\n+$sig\" 'subject line\n+\n+body contents\n+'\n \n sort >expected < refs/tags/bogo\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 6db92bd3ba..bd74b2d7e0 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -425,7 +425,13 @@ test_expect_success \\\n # creating annotated tags:\n \n get_tag_msg () {\n-\tgit cat-file tag \"$1\" | sed -e \"/BEGIN PGP/q\"\n+\tif [ \"$(test_oid algo)\" = sha1 ]\n+\tthen\n+\t\tgit cat-file tag \"$1\" | sed -e \"/BEGIN PGP/q\"\n+\telse\n+\n+\t\tgit cat-file tag \"$1\" | sed -e '/^gpgsig-sha256/{s/^gpgsig-sha256 //;h;d};/^ /d;${p;x;/^$/d}'\n+\tfi\n }\n \n # run test_tick before committing always gives the time in that timezone\ndiff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh\nindex 5c5bc32ccb..d95a03d315 100755\n--- a/t/t7030-verify-tag.sh\n+++ b/t/t7030-verify-tag.sh\n@@ -79,6 +79,23 @@ test_expect_success GPG 'verify and show signatures' '\n \t)\n '\n \n+test_expect_success GPG 'signature has expected format' '\n+\tfor tag in initial second merge fourth-signed sixth-signed seventh-signed\n+\tdo\n+\t\tif [ \"$(test_oid algo)\"\t= sha1 ]\n+\t\tthen\n+\t\t\tgit cat-file tag seventh-signed >output &&\n+\t\t\t! grep gpgsig output &&\n+\t\t\tgrep \"^-----BEGIN PGP SIGNATURE-----\" output\n+\t\telse\n+\t\t\tgit cat-file tag seventh-signed >output &&\n+\t\t\tgrep gpgsig-sha256 output &&\n+\t\t\t! grep \"^-----BEGIN PGP SIGNATURE-----\" output\n+\t\tfi &&\n+\t\techo $tag OK || exit 1\n+\tdone\n+'\n+\n test_expect_success GPGSM 'verify and show signatures x509' '\n \tgit verify-tag ninth-signed-x509 2>actual &&\n \tgrep \"Good signature from\" actual &&\n"},{"id":"392321","messageId":"20200222201749.937983-24-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 23/24] fast-import: add a generic function to iterate over marks","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:48Z","receivedAt":"2020-02-22T20:18:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently, we can iterate over marks only to dump them to a file. In the\nfuture, we'll want to perform an arbitrary operation over the items of a\nmark set. Add a function, for_each_mark, that iterates over marks in a\nset and performs an arbitrary callback function for each mark. Switch\nthe mark dumping routine to use this function now that it's available.\n\nSigned-off-by: brian m. carlson \n---\n fast-import.c | 50 ++++++++++++++++++++++++++------------------------\n 1 file changed, 26 insertions(+), 24 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 8aaa7f6289..6711f71ba7 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -132,6 +132,7 @@ struct recent_command {\n };\n \n typedef void (*mark_set_inserter_t)(struct mark_set *s, struct object_id *oid, uintmax_t mark);\n+typedef void (*each_mark_fn_t)(uintmax_t mark, void *obj, void *cbp);\n \n /* Configured limits on output */\n static unsigned long max_depth = 50;\n@@ -232,6 +233,29 @@ static void parse_get_mark(const char *p);\n static void parse_cat_blob(const char *p);\n static void parse_ls(const char *p, struct branch *b);\n \n+static void for_each_mark(struct mark_set *m, uintmax_t base, each_mark_fn_t callback, void *p)\n+{\n+\tuintmax_t k;\n+\tif (m->shift) {\n+\t\tfor (k = 0; k < 1024; k++) {\n+\t\t\tif (m->data.sets[k])\n+\t\t\t\tfor_each_mark(m->data.sets[k], base + (k << m->shift), callback, p);\n+\t\t}\n+\t} else {\n+\t\tfor (k = 0; k < 1024; k++) {\n+\t\t\tif (m->data.marked[k])\n+\t\t\t\tcallback(base + k, m->data.marked[k], p);\n+\t\t}\n+\t}\n+}\n+\n+static void dump_marks_fn(uintmax_t mark, void *object, void *cbp) {\n+\tstruct object_entry *e = object;\n+\tFILE *f = cbp;\n+\n+\tfprintf(f, \":%\" PRIuMAX \" %s\\n\", mark, oid_to_hex(&e->idx.oid));\n+}\n+\n static void write_branch_report(FILE *rpt, struct branch *b)\n {\n \tfprintf(rpt, \"%s:\\n\", b->name);\n@@ -260,8 +284,6 @@ static void write_branch_report(FILE *rpt, struct branch *b)\n \tfputc('\\n', rpt);\n }\n \n-static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);\n-\n static void write_crash_report(const char *err)\n {\n \tchar *loc = git_pathdup(\"fast_import_crash_%\"PRIuMAX, (uintmax_t) getpid());\n@@ -340,7 +362,7 @@ static void write_crash_report(const char *err)\n \tif (export_marks_file)\n \t\tfprintf(rpt, \" exported to %s\\n\", export_marks_file);\n \telse\n-\t\tdump_marks_helper(rpt, 0, marks);\n+\t\tfor_each_mark(marks, 0, dump_marks_fn, rpt);\n \n \tfputc('\\n', rpt);\n \tfputs(\"-------------------\\n\", rpt);\n@@ -1655,26 +1677,6 @@ static void dump_tags(void)\n \tstrbuf_release(&err);\n }\n \n-static void dump_marks_helper(FILE *f,\n-\tuintmax_t base,\n-\tstruct mark_set *m)\n-{\n-\tuintmax_t k;\n-\tif (m->shift) {\n-\t\tfor (k = 0; k < 1024; k++) {\n-\t\t\tif (m->data.sets[k])\n-\t\t\t\tdump_marks_helper(f, base + (k << m->shift),\n-\t\t\t\t\tm->data.sets[k]);\n-\t\t}\n-\t} else {\n-\t\tfor (k = 0; k < 1024; k++) {\n-\t\t\tif (m->data.marked[k])\n-\t\t\t\tfprintf(f, \":%\" PRIuMAX \" %s\\n\", base + k,\n-\t\t\t\t\toid_to_hex(&m->data.marked[k]->idx.oid));\n-\t\t}\n-\t}\n-}\n-\n static void dump_marks(void)\n {\n \tstruct lock_file mark_lock = LOCK_INIT;\n@@ -1704,7 +1706,7 @@ static void dump_marks(void)\n \t\treturn;\n \t}\n \n-\tdump_marks_helper(f, 0, marks);\n+\tfor_each_mark(marks, 0, dump_marks_fn, f);\n \tif (commit_lock_file(&mark_lock)) {\n \t\tfailure |= error_errno(\"Unable to write file %s\",\n \t\t\t\t export_marks_file);\n"},{"id":"392322","messageId":"20200222201749.937983-14-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 13/24] builtin/init-db: allow specifying hash algorithm on command line","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:38Z","receivedAt":"2020-02-22T20:18:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Allow the user to specify the hash algorithm on the command line by\nusing the --object-format option to git init. Validate that the user is\nnot attempting to reinitialize a repository with a different hash\nalgorithm. Ensure that if we are writing a non-SHA-1 repository that we\nset the repository version to 1 and write the objectFormat extension.\n\nRestrict this option to work only when ENABLE_SHA256 is set until the\ncodebase is in a situation to fully support this.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-init.txt | 7 ++++-\n builtin/clone.c | 2 +-\n builtin/init-db.c | 52 +++++++++++++++++++++++++++++++++-----\n cache.h | 3 ++-\n 4 files changed, 55 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\nindex 32880aafb0..adc6adfd38 100644\n--- a/Documentation/git-init.txt\n+++ b/Documentation/git-init.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git init' [-q | --quiet] [--bare] [--template=]\n-\t [--separate-git-dir ]\n+\t [--separate-git-dir ] [--object-format=]] [directory]\n \n \n@@ -48,6 +48,11 @@ Only print error and warning messages; all other output will be suppressed.\n Create a bare repository. If `GIT_DIR` environment is not set, it is set to the\n current working directory.\n \n+--object-format=::\n+\n+Specify the given object format (hash algorithm) for the repository. The valid\n+values are 'sha1' and (if enabled) 'sha256'. 'sha1' is the default.\n+\n --template=::\n \n Specify the directory from which templates will be used. (See the \"TEMPLATE\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4f6150c55c..961996a110 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1097,7 +1097,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tinit_db(git_dir, real_git_dir, option_template, INIT_DB_QUIET);\n+\tinit_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, INIT_DB_QUIET);\n \n \tif (real_git_dir)\n \t\tgit_dir = real_git_dir;\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex b11f07064d..d05552f0ae 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -177,7 +177,8 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n }\n \n static int create_default_files(const char *template_path,\n-\t\t\t\tconst char *original_git_dir)\n+\t\t\t\tconst char *original_git_dir,\n+\t\t\t\tconst struct repository_format *fmt)\n {\n \tstruct stat st1;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -187,6 +188,7 @@ static int create_default_files(const char *template_path,\n \tint reinit;\n \tint filemode;\n \tstruct strbuf err = STRBUF_INIT;\n+\tint repo_version = GIT_REPO_VERSION;\n \n \t/* Just look for `init.templatedir` */\n \tinit_db_template_dir = NULL; /* re-set in case it was set before */\n@@ -244,11 +246,23 @@ static int create_default_files(const char *template_path,\n \t\t\texit(1);\n \t}\n \n+#ifndef ENABLE_SHA256\n+\tif (fmt->hash_algo != GIT_HASH_SHA1)\n+\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n+#endif\n+\n+\tif (fmt->hash_algo != GIT_HASH_SHA1)\n+\t\trepo_version = GIT_REPO_VERSION_READ;\n+\n \t/* This forces creation of new config file */\n \txsnprintf(repo_version_string, sizeof(repo_version_string),\n-\t\t \"%d\", GIT_REPO_VERSION);\n+\t\t \"%d\", repo_version);\n \tgit_config_set(\"core.repositoryformatversion\", repo_version_string);\n \n+\tif (fmt->hash_algo != GIT_HASH_SHA1)\n+\t\tgit_config_set(\"extensions.objectformat\",\n+\t\t\t hash_algos[fmt->hash_algo].name);\n+\n \t/* Check filemode trustability */\n \tpath = git_path_buf(&buf, \"config\");\n \tfilemode = TEST_FILEMODE;\n@@ -340,12 +354,26 @@ static void separate_git_dir(const char *git_dir, const char *git_link)\n \twrite_file(git_link, \"gitdir: %s\", git_dir);\n }\n \n+static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash)\n+{\n+\t/*\n+\t * If we already have an initialized repo, don't allow the user to\n+\t * specify a different algorithm, as that could cause corruption.\n+\t * Otherwise, if the user has specified one on the command line, use it.\n+\t */\n+\tif (repo_fmt->version >= 0 && hash != GIT_HASH_UNKNOWN && hash != repo_fmt->hash_algo)\n+\t\tdie(_(\"attempt to reinitialize repository with different hash\"));\n+\telse if (hash != GIT_HASH_UNKNOWN)\n+\t\trepo_fmt->hash_algo = hash;\n+}\n+\n int init_db(const char *git_dir, const char *real_git_dir,\n-\t const char *template_dir, unsigned int flags)\n+\t const char *template_dir, int hash, unsigned int flags)\n {\n \tint reinit;\n \tint exist_ok = flags & INIT_DB_EXIST_OK;\n \tchar *original_git_dir = real_pathdup(git_dir, 1);\n+\tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n \n \tif (real_git_dir) {\n \t\tstruct stat st;\n@@ -378,9 +406,11 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t * config file, so this will not fail. What we are catching\n \t * is an attempt to reinitialize new repository with an old tool.\n \t */\n-\tcheck_repository_format(NULL);\n+\tcheck_repository_format(&repo_fmt);\n \n-\treinit = create_default_files(template_dir, original_git_dir);\n+\tvalidate_hash_algorithm(&repo_fmt, hash);\n+\n+\treinit = create_default_files(template_dir, original_git_dir, &repo_fmt);\n \n \tcreate_object_directory();\n \n@@ -482,6 +512,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \tconst char *work_tree;\n \tconst char *template_dir = NULL;\n \tunsigned int flags = 0;\n+\tconst char *object_format = NULL;\n+\tint hash_algo = GIT_HASH_UNKNOWN;\n \tconst struct option init_db_options[] = {\n \t\tOPT_STRING(0, \"template\", &template_dir, N_(\"template-directory\"),\n \t\t\t\tN_(\"directory from which templates will be used\")),\n@@ -494,6 +526,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('q', \"quiet\", &flags, N_(\"be quiet\"), INIT_DB_QUIET),\n \t\tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n \t\t\t N_(\"separate git dir from working tree\")),\n+\t\tOPT_STRING(0, \"object-format\", &object_format, N_(\"hash\"),\n+\t\t\t N_(\"specify the hash algorithm to use\")),\n \t\tOPT_END()\n \t};\n \n@@ -546,6 +580,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tfree(cwd);\n \t}\n \n+\tif (object_format) {\n+\t\thash_algo = hash_algo_by_name(object_format);\n+\t\tif (hash_algo == GIT_HASH_UNKNOWN)\n+\t\t\tdie(_(\"unknown hash algorithm '%s'\"), object_format);\n+\t}\n+\n \tif (init_shared_repository != -1)\n \t\tset_shared_repository(init_shared_repository);\n \n@@ -597,5 +637,5 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \tUNLEAK(work_tree);\n \n \tflags |= INIT_DB_EXIST_OK;\n-\treturn init_db(git_dir, real_git_dir, template_dir, flags);\n+\treturn init_db(git_dir, real_git_dir, template_dir, hash_algo, flags);\n }\ndiff --git a/cache.h b/cache.h\nindex 29ee02a8d4..7a47e023ba 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -627,7 +627,8 @@ int path_inside_repo(const char *prefix, const char *path);\n #define INIT_DB_EXIST_OK 0x0002\n \n int init_db(const char *git_dir, const char *real_git_dir,\n-\t const char *template_dir, unsigned int flags);\n+\t const char *template_dir, int hash_algo,\n+\t unsigned int flags);\n \n void sanitize_stdfds(void);\n int daemonize(void);\n"},{"id":"392323","messageId":"20200222201749.937983-18-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 17/24] commit: use expected signature header for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:42Z","receivedAt":"2020-02-22T20:18:22Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The transition plan anticipates that we will allow signatures using\nmultiple algorithms in a single commit. In order to do so, we need to\nuse a different header per algorithm so that it will be obvious over\nwhich data to compute the signature.\n\nThe transition plan specifies that we should use \"gpgsig-sha256\", so\nwire up the commit code such that it can write and parse the current\nalgorithm, and it can remove the headers for any algorithm when creating\na new commit. Add tests to ensure that we write using the right header\nand that git fsck doesn't reject these commits.\n\nSigned-off-by: brian m. carlson \n---\n builtin/commit.c | 2 +-\n commit.c | 30 +++++++++++++++++++++++-------\n sequencer.c | 2 +-\n t/t1450-fsck.sh | 24 ++++++++++++++++++++++++\n t/t7510-signed-commit.sh | 16 +++++++++++++---\n 5 files changed, 62 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 7ba33a3bec..798d362a2e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (amend) {\n-\t\tconst char *exclude_gpgsig[2] = { \"gpgsig\", NULL };\n+\t\tconst char *exclude_gpgsig[3] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n \t\textra = read_commit_extra_headers(current_head, exclude_gpgsig);\n \t} else {\n \t\tstruct commit_extra_header **tail = &extra;\ndiff --git a/commit.c b/commit.c\nindex a6cfa41a4e..534e14f22a 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -961,14 +961,22 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)\n \treturn ret;\n }\n \n-static const char gpg_sig_header[] = \"gpgsig\";\n-static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1;\n+/*\n+ * Indexed by hash algorithm identifier.\n+ */\n+static const char *gpg_sig_headers[] = {\n+\tNULL,\n+\t\"gpgsig\",\n+\t\"gpgsig-sha256\",\n+};\n \n static int do_sign_commit(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n \tconst char *eoh;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n+\tint gpg_sig_header_len = strlen(gpg_sig_header);\n \n \t/* find the end of the header */\n \teoh = strstr(buf->buf, \"\\n\\n\");\n@@ -1010,6 +1018,8 @@ int parse_signed_commit(const struct commit *commit,\n \tconst char *buffer = get_commit_buffer(commit, &size);\n \tint in_signature, saw_signature = -1;\n \tconst char *line, *tail;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n+\tint gpg_sig_header_len = strlen(gpg_sig_header);\n \n \tline = buffer;\n \ttail = buffer + size;\n@@ -1056,11 +1066,17 @@ int remove_signature(struct strbuf *buf)\n \n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig_end = next;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ') {\n-\t\t\tsig_start = line;\n-\t\t\tsig_end = next;\n-\t\t\tin_signature = 1;\n+\t\telse if (starts_with(line, \"gpgsig\")) {\n+\t\t\tint i;\n+\t\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n+\t\t\t\tconst char *p;\n+\t\t\t\tif (skip_prefix(line, gpg_sig_headers[i], &p) &&\n+\t\t\t\t *p == ' ') {\n+\t\t\t\t\tsig_start = line;\n+\t\t\t\t\tsig_end = next;\n+\t\t\t\t\tin_signature = 1;\n+\t\t\t\t}\n+\t\t\t}\n \t\t} else {\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\ndiff --git a/sequencer.c b/sequencer.c\nindex ba90a513b9..cbd920afb2 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1321,7 +1321,7 @@ static int try_to_commit(struct repository *r,\n \t\treturn -1;\n \n \tif (flags & AMEND_MSG) {\n-\t\tconst char *exclude_gpgsig[] = { \"gpgsig\", NULL };\n+\t\tconst char *exclude_gpgsig[] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n \t\tconst char *out_enc = get_commit_output_encoding();\n \t\tconst char *message = logmsg_reencode(current_head, NULL,\n \t\t\t\t\t\t out_enc);\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 02478bc4ec..70a8307154 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -133,6 +133,30 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' '\n \ttest_i18ngrep \"worktrees/other/HEAD points to something strange\" out\n '\n \n+test_expect_success 'commit with multiple signatures is okay' '\n+\tgit cat-file commit HEAD >basis &&\n+\tcat >sigs <<-EOF &&\n+\tgpgsig -----BEGIN PGP SIGNATURE-----\n+\t VGhpcyBpcyBub3QgcmVhbGx5IGEgc2lnbmF0dXJlLg==\n+\t -----END PGP SIGNATURE-----\n+\tgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n+\t VGhpcyBpcyBub3QgcmVhbGx5IGEgc2lnbmF0dXJlLg==\n+\t -----END PGP SIGNATURE-----\n+\tEOF\n+\tsed -e \"/^committer/q\" basis >okay &&\n+\tcat sigs >>okay &&\n+\techo >>okay &&\n+\tsed -e \"1,/^$/d\" basis >>okay &&\n+\tcat okay &&\n+\tnew=$(git hash-object -t commit -w --stdin out &&\n+\tcat out &&\n+\t! grep \"commit $new\" out\n+'\n+\n test_expect_success 'email without @ is okay' '\n \tgit cat-file commit HEAD >basis &&\n \tsed \"s/@/AT/\" basis >okay &&\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 0c06d22a00..6baaa1ad91 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -6,6 +6,11 @@ GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n test_expect_success GPG 'create signed commits' '\n+\ttest_oid_cache <<-\\EOF &&\n+\theader sha1:gpgsig\n+\theader sha256:gpgsig-sha256\n+\tEOF\n+\n \ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n \n \techo 1 >file && git add file &&\n@@ -155,6 +160,11 @@ test_expect_success GPG 'verify signatures with --raw' '\n \t)\n '\n \n+test_expect_success GPG 'proper header is used for hash algorithm' '\n+\tgit cat-file commit fourth-signed >output &&\n+\tgrep \"^$(test_oid header) -----BEGIN PGP SIGNATURE-----\" output\n+'\n+\n test_expect_success GPG 'show signed commit with signature' '\n \tgit show -s initial >commit &&\n \tgit show -s --show-signature initial >show &&\n@@ -162,7 +172,7 @@ test_expect_success GPG 'show signed commit with signature' '\n \tgit cat-file commit initial >cat &&\n \tgrep -v -e \"gpg: \" -e \"Warning: \" show >show.commit &&\n \tgrep -e \"gpg: \" -e \"Warning: \" show >show.gpg &&\n-\tgrep -v \"^ \" cat | grep -v \"^gpgsig \" >cat.commit &&\n+\tgrep -v \"^ \" cat | grep -v \"^$(test_oid header) \" >cat.commit &&\n \ttest_cmp show.commit commit &&\n \ttest_cmp show.gpg verify.2 &&\n \ttest_cmp cat.commit verify.1\n@@ -299,10 +309,10 @@ test_expect_success GPG 'check config gpg.format values' '\n test_expect_success GPG 'detect fudged commit with double signature' '\n \tsed -e \"/gpgsig/,/END PGP/d\" forged1 >double-base &&\n \tsed -n -e \"/gpgsig/,/END PGP/p\" forged1 | \\\n-\t\tsed -e \"s/^gpgsig//;s/^ //\" | gpg --dearmor >double-sig1.sig &&\n+\t\tsed -e \"s/^$(test_oid header)//;s/^ //\" | gpg --dearmor >double-sig1.sig &&\n \tgpg -o double-sig2.sig -u 29472784 --detach-sign double-base &&\n \tcat double-sig1.sig double-sig2.sig | gpg --enarmor >double-combined.asc &&\n-\tsed -e \"s/^\\(-.*\\)ARMORED FILE/\\1SIGNATURE/;1s/^/gpgsig /;2,\\$s/^/ /\" \\\n+\tsed -e \"s/^\\(-.*\\)ARMORED FILE/\\1SIGNATURE/;1s/^/$(test_oid header) /;2,\\$s/^/ /\" \\\n \t\tdouble-combined.asc > double-gpgsig &&\n \tsed -e \"/committer/r double-gpgsig\" double-base >double-commit &&\n \tgit hash-object -w -t commit double-commit >double-commit.commit &&\n"},{"id":"392324","messageId":"20200222201749.937983-19-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:43Z","receivedAt":"2020-02-22T20:18:23Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function which parses a buffer with a signature at the end,\nparse_signature, and this function is used for signed tags. However,\nthe transition plan has SHA-256 tags using a header, which is a\nmaterially different syntax. The current interface is not suitable for\nparsing such tags.\n\nAdjust the parse_signature interface to store the parsed data in two\nstrbufs and turn the existing function into parse_signed_buffer. The\nlatter is still used in places where we want to strip off the signature\nin a SHA-1 tag or in places where we know we always have a signed\nbuffer, such as push certs.\n\nAdjust all the callers to deal with this new interface.\n\nSigned-off-by: brian m. carlson \n---\n builtin/fmt-merge-msg.c | 26 ++++++++++++++++++--------\n builtin/receive-pack.c | 4 ++--\n builtin/tag.c | 16 ++++++++++++----\n commit.c | 9 ++++++---\n gpg-interface.c | 13 ++++++++++++-\n gpg-interface.h | 9 ++++++++-\n log-tree.c | 14 ++++++++------\n ref-filter.c | 18 ++++++++++++++----\n tag.c | 15 ++++++++-------\n 9 files changed, 88 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 05a92c59d8..29f647e2d9 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -472,6 +472,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n \t\t\t const char *buf,\n \t\t\t unsigned long len)\n {\n+\n \tconst char *tag_body = strstr(buf, \"\\n\\n\");\n \tif (tag_body) {\n \t\ttag_body += 2;\n@@ -492,24 +493,31 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \tfor (i = 0; i < origins.nr; i++) {\n \t\tstruct object_id *oid = origins.items[i].util;\n \t\tenum object_type type;\n-\t\tunsigned long size, len;\n-\t\tchar *buf = read_object_file(oid, &type, &size);\n+\t\tunsigned long size;\n+\t\tchar *buf = read_object_file(oid, &type, &size), *orig = buf;\n \t\tstruct signature_check sigc = { 0 };\n+\t\tstruct strbuf payload = STRBUF_INIT;\n+\t\tstruct strbuf signature = STRBUF_INIT;\n \t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tsize_t len = size;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n-\n-\t\tif (size == len)\n-\t\t\t; /* merely annotated */\n-\t\telse if (!check_signature(buf, len, buf + len, size - len,\n+\t\tif (!parse_signature(buf, size, &payload, &signature))\n+\t\t\tlen = size; /* merely annotated */\n+\t\telse if (!check_signature(payload.buf, payload.len,\n+\t\t\t\t\t signature.buf, signature.len,\n \t\t\t\t\t &sigc)) {\n \t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n \t\t\tsignature_check_clear(&sigc);\n \t\t} else\n \t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \n+\t\tif (payload.len) {\n+\t\t\tbuf = payload.buf;\n+\t\t\tlen = payload.len;\n+\t\t}\n+\n \t\tif (!tag_number++) {\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t\tfirst_tag = i;\n@@ -531,8 +539,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n \t\tstrbuf_release(&sig);\n+\t\tstrbuf_release(&payload);\n+\t\tstrbuf_release(&signature);\n \tnext:\n-\t\tfree(buf);\n+\t\tfree(orig);\n \t}\n \tif (tagbuf.len) {\n \t\tstrbuf_addch(out, '\\n');\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 411e0b4d99..4c814c1963 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -636,7 +636,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n \t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n \t\t\t\tpush_cert.len - bogs, &sigcheck);\n \n@@ -1568,7 +1568,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e0a4c25382..6b95c6a2ea 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -174,11 +174,17 @@ static void write_tag_body(int fd, const struct object_id *oid)\n {\n \tunsigned long size;\n \tenum object_type type;\n-\tchar *buf, *sp;\n+\tchar *buf, *sp, *orig;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n-\tbuf = read_object_file(oid, &type, &size);\n+\torig = buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\treturn;\n+\tif (parse_signature(buf, size, &payload, &signature)) {\n+\t\tbuf = payload.buf;\n+\t\tsize = payload.len;\n+\t}\n \t/* skip header */\n \tsp = strstr(buf, \"\\n\\n\");\n \n@@ -187,9 +193,11 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, buf + size - sp);\n \n-\tfree(buf);\n+\tfree(orig);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n }\n \n static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result)\ndiff --git a/commit.c b/commit.c\nindex 534e14f22a..d39ce5076d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1097,8 +1097,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tstruct merge_remote_desc *desc;\n \tstruct commit_extra_header *mergetag;\n \tchar *buf;\n-\tunsigned long size, len;\n+\tunsigned long size;\n \tenum object_type type;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \tdesc = merge_remote_util(parent);\n \tif (!desc || !desc->obj)\n@@ -1106,8 +1108,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_object_file(&desc->obj->oid, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n-\tif (size == len)\n+\tif (!parse_signature(buf, size, &payload, &signature))\n \t\tgoto free_return;\n \t/*\n \t * We could verify this signature and either omit the tag when\n@@ -1126,6 +1127,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \n \t**tail = mergetag;\n \t*tail = &mergetag->next;\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn;\n \n free_return:\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2d538bcd6e..b25f5c21d8 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signed_buffer(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n@@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n \treturn match;\n }\n \n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n+{\n+\tsize_t match = parse_signed_buffer(buf, size);\n+\tif (match != size) {\n+\t\tstrbuf_add(payload, buf, match);\n+\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..80567e4894 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -37,13 +37,20 @@ struct signature_check {\n \n void signature_check_clear(struct signature_check *sigc);\n \n+/*\n+ * Look at a GPG signed tag object. If such a signature exists, store it in\n+ * signature and the signed content in payload. Return 1 if a signature was\n+ * found, and 0 otherwise.\n+ */\n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature);\n+\n /*\n * Look at GPG signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signed_buffer(const char *buf, size_t size);\n \n /*\n * Create a detached signature for the contents of \"buffer\" and append\ndiff --git a/log-tree.c b/log-tree.c\nindex cae38dcc66..75bd61a531 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -499,7 +499,9 @@ static int show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tstruct signature_check sigc = { 0 };\n \tint status, nth;\n-\tsize_t payload_size, gpg_message_offset;\n+\tsize_t gpg_message_offset;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -523,13 +525,11 @@ static int show_one_mergetag(struct commit *commit,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \tgpg_message_offset = verify_message.len;\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n \tstatus = -1;\n-\tif (extra->len > payload_size) {\n+\tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tif (!check_signature(extra->value, payload_size,\n-\t\t\t\t extra->value + payload_size,\n-\t\t\t\t extra->len - payload_size, &sigc)) {\n+\t\tif (!check_signature(payload.buf, payload.len,\n+\t\t\t\t signature.buf, signature.len, &sigc)) {\n \t\t\tstrbuf_addstr(&verify_message, sigc.gpg_output);\n \t\t\tsignature_check_clear(&sigc);\n \t\t\tstatus = 0; /* good */\n@@ -540,6 +540,8 @@ static int show_one_mergetag(struct commit *commit,\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6867e33648..212f1165bb 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1161,7 +1161,13 @@ static void find_subpos(const char *buf,\n \t\t\tunsigned long *nonsiglen,\n \t\t\tconst char **sig, unsigned long *siglen)\n {\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tconst char *eol;\n+\tconst char *end = buf + strlen(buf);\n+\tconst char *sigstart;\n+\n+\n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n@@ -1174,13 +1180,14 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n+\tparse_signature(buf, end - buf, &payload, &signature);\n+\t*sig = strbuf_detach(&signature, siglen);\n+\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n \t/* subject goes to first empty line */\n-\twhile (buf < *sig && *buf && *buf != '\\n') {\n+\twhile (buf < sigstart && *buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n \t\tif (*eol)\n \t\t\teol++;\n@@ -1196,7 +1203,7 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \t*body = buf;\n \t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n+\t*nonsiglen = sigstart - buf;\n }\n \n /*\n@@ -1234,6 +1241,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\tstruct used_atom *atom = &used_atom[i];\n \t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1273,6 +1281,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n+\n+\t\tfree((void *)sigpos);\n \t}\n }\n \ndiff --git a/tag.c b/tag.c\nindex 71b544467e..5d04506d10 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,26 +13,27 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n+\tif (!parse_signature(buf, size, &payload, &signature)) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n+\t\t\twrite_in_full(1, buf, size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n+\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn ret;\n }\n \n"},{"id":"392330","messageId":"20200222201749.937983-21-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 20/24] fast-import: permit reading multiple marks files","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:45Z","receivedAt":"2020-02-22T20:18:23Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll want to read marks files for submodules as well.\nRefactor the existing code to make it possible to read multiple marks\nfiles, each into their own marks set.\n\nSigned-off-by: brian m. carlson \n---\n fast-import.c | 32 ++++++++++++++++++--------------\n 1 file changed, 18 insertions(+), 14 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex b8b65a801c..b9ecd89699 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -493,9 +493,8 @@ static char *pool_strdup(const char *s)\n \treturn r;\n }\n \n-static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n+static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry *oe)\n {\n-\tstruct mark_set *s = marks;\n \twhile ((idnum >> s->shift) >= 1024) {\n \t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\ts->shift = marks->shift + 10;\n@@ -919,7 +918,7 @@ static int store_object(\n \n \te = insert_object(&oid);\n \tif (mark)\n-\t\tinsert_mark(mark, e);\n+\t\tinsert_mark(marks, mark, e);\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n@@ -1117,7 +1116,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \te = insert_object(&oid);\n \n \tif (mark)\n-\t\tinsert_mark(mark, e);\n+\t\tinsert_mark(marks, mark, e);\n \n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n@@ -1712,16 +1711,9 @@ static void dump_marks(void)\n \t}\n }\n \n-static void read_marks(void)\n+static void read_mark_file(struct mark_set *s, FILE *f)\n {\n \tchar line[512];\n-\tFILE *f = fopen(import_marks_file, \"r\");\n-\tif (f)\n-\t\t;\n-\telse if (import_marks_file_ignore_missing && errno == ENOENT)\n-\t\tgoto done; /* Marks file does not exist */\n-\telse\n-\t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n \twhile (fgets(line, sizeof(line), f)) {\n \t\tuintmax_t mark;\n \t\tchar *end;\n@@ -1747,8 +1739,20 @@ static void read_marks(void)\n \t\t\te->pack_id = MAX_PACK_ID;\n \t\t\te->idx.offset = 1; /* just not zero! */\n \t\t}\n-\t\tinsert_mark(mark, e);\n+\t\tinsert_mark(s, mark, e);\n \t}\n+}\n+\n+static void read_marks(void)\n+{\n+\tFILE *f = fopen(import_marks_file, \"r\");\n+\tif (f)\n+\t\t;\n+\telse if (import_marks_file_ignore_missing && errno == ENOENT)\n+\t\tgoto done; /* Marks file does not exist */\n+\telse\n+\t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n+\tread_mark_file(marks, f);\n \tfclose(f);\n done:\n \timport_marks_file_done = 1;\n@@ -3130,7 +3134,7 @@ static void parse_alias(void)\n \t\tdie(_(\"Expected 'to' command, got %s\"), command_buf.buf);\n \te = find_object(&b.oid);\n \tassert(e);\n-\tinsert_mark(next_mark, e);\n+\tinsert_mark(marks, next_mark, e);\n }\n \n static char* make_fast_import_path(const char *path)\n"},{"id":"392325","messageId":"20200222201749.937983-25-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 24/24] fast-import: add options for rewriting submodules","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:49Z","receivedAt":"2020-02-22T20:18:24Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When converting a repository using submodules from one hash algorithm to\nanother, it is necessary to rewrite the submodules from the old\nalgorithm to the new algorithm, since only references to submodules, not\ntheir contents, are written to the fast-export stream. Without rewriting\nthe submodules, fast-import fails with an \"Invalid dataref\" error when\nencountering a submodule in another algorithm.\n\nAdd a pair of options, --rewrite-submodules-from and\n--rewrite-submodules-to, that take a list of marks produced by\nfast-export and fast-import, respectively, when processing the\nsubmodule. Use these marks to map the submodule commits from the old\nalgorithm to the new algorithm.\n\nWe read marks into two corresponding struct mark_set objects and then\nperform a mapping from the old to the new using a hash table. This lets\nus reuse the same mark parsing code that is used elsewhere and allows us\nto efficiently read and match marks based on their ID, since mark files\nneed not be sorted.\n\nNote that because we're using a khash table for the object IDs, and this\ntable copies values of struct object_id instead of taking references to\nthem, it's necessary to zero the struct object_id values that we use to\ninsert and look up in the table. Otherwise, we would end up with SHA-1\nvalues that don't match because of whatever stack garbage might be left\nin the unused area.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-fast-import.txt | 20 ++++++\n fast-import.c | 112 ++++++++++++++++++++++++++++--\n t/t9300-fast-import.sh | 109 +++++++++++++++++++++++++++++\n 3 files changed, 236 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 7889f95940..77c6b3d001 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -122,6 +122,26 @@ Locations of Marks Files\n Relative and non-relative marks may be combined by interweaving\n --(no-)-relative-marks with the --(import|export)-marks= options.\n \n+Submodule Rewriting\n+~~~~~~~~~~~~~~~~~~~\n+\n+--rewrite-submodules-from=:::\n+--rewrite-submodules-to=:::\n+ Rewrite the object IDs for the submodule specified by from the values\n+\tused in the from to those used in the to . The from marks should\n+\thave been created by `git fast-export`, and the to marks should have been\n+\tcreated by `git fast-import` when importing that same submodule.\n++\n+ may be any arbitrary string not containing a colon character, but the\n+same value must be used with both options when specifying corresponding marks.\n+Multiple submodules may be specified with different values for . It is an\n+error not to use these options in corresponding pairs.\n++\n+These options are primarily useful when converting a repository from one hash\n+algorithm to another; without them, fast-import will fail if it encounters a\n+submodule because it has no way of writing the object ID into the new hash\n+algorithm.\n+\n Performance and Compression Tuning\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n \ndiff --git a/fast-import.c b/fast-import.c\nindex 6711f71ba7..202dda11a6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -18,6 +18,7 @@\n #include \"object-store.h\"\n #include \"mem-pool.h\"\n #include \"commit-reach.h\"\n+#include \"khash.h\"\n \n #define PACK_ID_BITS 16\n #define MAX_PACK_ID ((1<hash, 0, sizeof(oid->hash));\n+\n+\talgo = parse_oid_hex_any(hex, oid, end);\n+\tif (algo == GIT_HASH_UNKNOWN)\n+\t\treturn -1;\n+\n+\tit = kh_get_oid_map(sub_oid_map, *oid);\n+\t/* No such object? */\n+\tif (it == kh_end(sub_oid_map)) {\n+\t\t/* If we're using the same algorithm, pass it through. */\n+\t\tif (hash_algos[algo].format_id == the_hash_algo->format_id)\n+\t\t\treturn 0;\n+\t\treturn -1;\n+\t}\n+\toidcpy(oid, kh_value(sub_oid_map, it));\n+\treturn 0;\n+}\n+\n /*\n * Given a pointer into a string, parse a mark reference:\n *\n@@ -2232,7 +2272,7 @@ static void file_change_m(const char *p, struct branch *b)\n \t\tinline_data = 1;\n \t\toe = NULL; /* not used with inline_data, but makes gcc happy */\n \t} else {\n-\t\tif (parse_oid_hex(p, &oid, &p))\n+\t\tif (parse_mapped_oid_hex(p, &oid, &p))\n \t\t\tdie(\"Invalid dataref: %s\", command_buf.buf);\n \t\toe = find_object(&oid);\n \t\tif (*p++ != ' ')\n@@ -2406,7 +2446,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa\n \t\tinline_data = 1;\n \t\toe = NULL; /* not used with inline_data, but makes gcc happy */\n \t} else {\n-\t\tif (parse_oid_hex(p, &oid, &p))\n+\t\tif (parse_mapped_oid_hex(p, &oid, &p))\n \t\t\tdie(\"Invalid dataref: %s\", command_buf.buf);\n \t\toe = find_object(&oid);\n \t\tif (*p++ != ' ')\n@@ -2941,7 +2981,7 @@ static void parse_cat_blob(const char *p)\n \t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \t\toidcpy(&oid, &oe->idx.oid);\n \t} else {\n-\t\tif (parse_oid_hex(p, &oid, &p))\n+\t\tif (parse_mapped_oid_hex(p, &oid, &p))\n \t\t\tdie(\"Invalid dataref: %s\", command_buf.buf);\n \t\tif (*p)\n \t\t\tdie(\"Garbage after SHA1: %s\", command_buf.buf);\n@@ -3005,6 +3045,42 @@ static struct object_entry *dereference(struct object_entry *oe,\n \treturn find_object(oid);\n }\n \n+static void insert_mapped_mark(uintmax_t mark, void *object, void *cbp)\n+{\n+\tstruct object_id *fromoid = object;\n+\tstruct object_id *tooid = find_mark(cbp, mark);\n+\tint ret;\n+\tkhiter_t it;\n+\n+\tit = kh_put_oid_map(sub_oid_map, *fromoid, &ret);\n+\t/* We've already seen this object. */\n+\tif (ret == 0)\n+\t\treturn;\n+\tkh_value(sub_oid_map, it) = tooid;\n+}\n+\n+static void build_mark_map_one(struct mark_set *from, struct mark_set *to)\n+{\n+\tfor_each_mark(from, 0, insert_mapped_mark, to);\n+}\n+\n+static void build_mark_map(struct string_list *from, struct string_list *to)\n+{\n+\tstruct string_list_item *fromp, *top;\n+\n+\tsub_oid_map = kh_init_oid_map();\n+\n+\tfor_each_string_list_item(fromp, from) {\n+\t\ttop = string_list_lookup(to, fromp->string);\n+\t\tif (!fromp->util) {\n+\t\t\tdie(_(\"Missing from marks for submodule '%s'\"), fromp->string);\n+\t\t} else if (!top || !top->util) {\n+\t\t\tdie(_(\"Missing to marks for submodule '%s'\"), fromp->string);\n+\t\t}\n+\t\tbuild_mark_map_one(fromp->util, top->util);\n+\t}\n+}\n+\n static struct object_entry *parse_treeish_dataref(const char **p)\n {\n \tstruct object_id oid;\n@@ -3016,7 +3092,7 @@ static struct object_entry *parse_treeish_dataref(const char **p)\n \t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \t\toidcpy(&oid, &e->idx.oid);\n \t} else {\t/* */\n-\t\tif (parse_oid_hex(*p, &oid, p))\n+\t\tif (parse_mapped_oid_hex(*p, &oid, p))\n \t\t\tdie(\"Invalid dataref: %s\", command_buf.buf);\n \t\te = find_object(&oid);\n \t\tif (*(*p)++ != ' ')\n@@ -3222,6 +3298,26 @@ static void option_export_pack_edges(const char *edges)\n \tpack_edges = xfopen(edges, \"a\");\n }\n \n+static void option_rewrite_submodules(const char *arg, struct string_list *list)\n+{\n+\tstruct mark_set *ms;\n+\tFILE *fp;\n+\tchar *s = xstrdup(arg);\n+\tchar *f = strchr(s, ':');\n+\tif (!f)\n+\t\tdie(_(\"Expected format name:filename for submodule rewrite option\"));\n+\t*f = '\\0';\n+\tf++;\n+\tms = xcalloc(1, sizeof(*ms));\n+\tstring_list_insert(list, s)->util = ms;\n+\n+\tfp = fopen(f, \"r\");\n+\tif (!fp)\n+\t\tdie_errno(\"cannot read '%s'\", f);\n+\tread_mark_file(ms, fp, insert_oid_entry);\n+\tfclose(fp);\n+}\n+\n static int parse_one_option(const char *option)\n {\n \tif (skip_prefix(option, \"max-pack-size=\", &option)) {\n@@ -3284,6 +3380,11 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\toption_export_marks(arg);\n \t} else if (!strcmp(feature, \"alias\")) {\n \t\t; /* Don't die - this feature is supported */\n+\t} else if (skip_prefix(feature, \"rewrite-submodules-to=\", &arg)) {\n+\t\toption_rewrite_submodules(arg, &sub_marks_to);\n+\t} else if (skip_prefix(feature, \"rewrite-submodules-from=\", &arg)) {\n+\t\toption_rewrite_submodules(arg, &sub_marks_from);\n+\t} else if (skip_prefix(feature, \"rewrite-submodules-from=\", &arg)) {\n \t} else if (!strcmp(feature, \"get-mark\")) {\n \t\t; /* Don't die - this feature is supported */\n \t} else if (!strcmp(feature, \"cat-blob\")) {\n@@ -3389,6 +3490,7 @@ static void parse_argv(void)\n \tseen_data_command = 1;\n \tif (import_marks_file)\n \t\tread_marks();\n+\tbuild_mark_map(&sub_marks_from, &sub_marks_to);\n }\n \n int cmd_main(int argc, const char **argv)\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex ae9950a9c2..22c6c27763 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -3382,4 +3382,113 @@ test_expect_success 'X: handling encoding' '\n \tgit log -1 --format=%B encoding | grep $(printf \"\\317\\200\")\n '\n \n+###\n+### series Y (submodules and hash algorithms)\n+###\n+\n+cat >Y-sub-input <<\\Y_INPUT_END\n+blob\n+mark :1\n+data 4\n+foo\n+\n+reset refs/heads/master\n+commit refs/heads/master\n+mark :2\n+author Full Name 1000000000 +0100\n+committer Full Name 1000000000 +0100\n+data 24\n+Test submodule commit 1\n+M 100644 :1 file\n+\n+blob\n+mark :3\n+data 8\n+foo\n+bar\n+\n+commit refs/heads/master\n+mark :4\n+author Full Name 1000000001 +0100\n+committer Full Name 1000000001 +0100\n+data 24\n+Test submodule commit 2\n+from :2\n+M 100644 :3 file\n+Y_INPUT_END\n+\n+# Note that the submodule object IDs are intentionally not translated.\n+cat >Y-main-input <<\\Y_INPUT_END\n+blob\n+mark :1\n+data 4\n+foo\n+\n+reset refs/heads/master\n+commit refs/heads/master\n+mark :2\n+author Full Name 2000000000 +0100\n+committer Full Name 2000000000 +0100\n+data 14\n+Test commit 1\n+M 100644 :1 file\n+\n+blob\n+mark :3\n+data 73\n+[submodule \"sub1\"]\n+\tpath = sub1\n+\turl = https://void.example.com/main.git\n+\n+commit refs/heads/master\n+mark :4\n+author Full Name 2000000001 +0100\n+committer Full Name 2000000001 +0100\n+data 14\n+Test commit 2\n+from :2\n+M 100644 :3 .gitmodules\n+M 160000 0712c5be7cf681388e355ef47525aaf23aee1a6d sub1\n+\n+blob\n+mark :5\n+data 8\n+foo\n+bar\n+\n+commit refs/heads/master\n+mark :6\n+author Full Name 2000000002 +0100\n+committer Full Name 2000000002 +0100\n+data 14\n+Test commit 3\n+from :4\n+M 100644 :5 file\n+M 160000 ff729f5e62f72c0c3978207d9a80e5f3a65f14d7 sub1\n+Y_INPUT_END\n+\n+cat >Y-marks <<\\Y_INPUT_END\n+:2 0712c5be7cf681388e355ef47525aaf23aee1a6d\n+:4 ff729f5e62f72c0c3978207d9a80e5f3a65f14d7\n+Y_INPUT_END\n+\n+test_expect_success 'Y: setup' '\n+\ttest_oid_cache <<-EOF\n+\tYmaster sha1:9afed2f9161ddf416c0a1863b8b0725b00070504\n+\tYmaster sha256:c0a1010da1df187b2e287654793df01b464bd6f8e3f17fc1481a7dadf84caee3\n+\tEOF\n+'\n+\n+test_expect_success 'Y: rewrite submodules' '\n+\tgit init main1 &&\n+\t(\n+\t\tcd main1 &&\n+\t\tgit init sub2 &&\n+\t\tgit -C sub2 fast-import --export-marks=../sub2-marks <../Y-sub-input &&\n+\t\tgit fast-import --rewrite-submodules-from=sub:../Y-marks \\\n+\t\t\t--rewrite-submodules-to=sub:sub2-marks <../Y-main-input &&\n+\t\ttest \"$(git rev-parse master)\" = \"$(test_oid Ymaster)\"\n+\t)\n+'\n+\n test_done\n"},{"id":"392326","messageId":"20200222201749.937983-22-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 21/24] fast-import: add helper function for inserting mark object entries","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:46Z","receivedAt":"2020-02-22T20:18:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently, everything we want to insert into a mark set is an object\nentry. However, in the future, we will want to insert objects of other\ntypes. Teach read_mark_file to take a function pointer which helps us\ninsert the object we want into our mark set.\n\nSigned-off-by: brian m. carlson \n---\n fast-import.c | 37 ++++++++++++++++++++++---------------\n 1 file changed, 22 insertions(+), 15 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex b9ecd89699..3ce4a04473 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -131,6 +131,8 @@ struct recent_command {\n \tchar *buf;\n };\n \n+typedef void (*mark_set_inserter_t)(struct mark_set *s, struct object_id *oid, uintmax_t mark);\n+\n /* Configured limits on output */\n static unsigned long max_depth = 50;\n static off_t max_packsize;\n@@ -1711,14 +1713,30 @@ static void dump_marks(void)\n \t}\n }\n \n-static void read_mark_file(struct mark_set *s, FILE *f)\n+static void insert_object_entry(struct mark_set *s, struct object_id *oid, uintmax_t mark)\n+{\n+\tstruct object_entry *e;\n+\te = find_object(oid);\n+\tif (!e) {\n+\t\tenum object_type type = oid_object_info(the_repository,\n+\t\t\t\t\t\t\toid, NULL);\n+\t\tif (type < 0)\n+\t\t\tdie(\"object not found: %s\", oid_to_hex(oid));\n+\t\te = insert_object(oid);\n+\t\te->type = type;\n+\t\te->pack_id = MAX_PACK_ID;\n+\t\te->idx.offset = 1; /* just not zero! */\n+\t}\n+\tinsert_mark(s, mark, e);\n+}\n+\n+static void read_mark_file(struct mark_set *s, FILE *f, mark_set_inserter_t inserter)\n {\n \tchar line[512];\n \twhile (fgets(line, sizeof(line), f)) {\n \t\tuintmax_t mark;\n \t\tchar *end;\n \t\tstruct object_id oid;\n-\t\tstruct object_entry *e;\n \n \t\tend = strchr(line, '\\n');\n \t\tif (line[0] != ':' || !end)\n@@ -1728,18 +1746,7 @@ static void read_mark_file(struct mark_set *s, FILE *f)\n \t\tif (!mark || end == line + 1\n \t\t\t|| *end != ' ' || get_oid_hex(end + 1, &oid))\n \t\t\tdie(\"corrupt mark line: %s\", line);\n-\t\te = find_object(&oid);\n-\t\tif (!e) {\n-\t\t\tenum object_type type = oid_object_info(the_repository,\n-\t\t\t\t\t\t\t\t&oid, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"object not found: %s\", oid_to_hex(&oid));\n-\t\t\te = insert_object(&oid);\n-\t\t\te->type = type;\n-\t\t\te->pack_id = MAX_PACK_ID;\n-\t\t\te->idx.offset = 1; /* just not zero! */\n-\t\t}\n-\t\tinsert_mark(s, mark, e);\n+\t\tinserter(s, &oid, mark);\n \t}\n }\n \n@@ -1752,7 +1759,7 @@ static void read_marks(void)\n \t\tgoto done; /* Marks file does not exist */\n \telse\n \t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n-\tread_mark_file(marks, f);\n+\tread_mark_file(marks, f, insert_object_entry);\n \tfclose(f);\n done:\n \timport_marks_file_done = 1;\n"},{"id":"392327","messageId":"20200222201749.937983-5-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 04/24] hex: add functions to parse hex object IDs in any algorithm","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:29Z","receivedAt":"2020-02-22T20:18:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There are some places where we need to parse a hex object ID in any\nalgorithm without knowing beforehand which algorithm is in use. An\nexample is when parsing fast-import marks.\n\nAdd a get_oid_hex_any to parse an object ID and return the algorithm it\nbelongs to, and additionally add parse_oid_hex_any which is the\nequivalent change for parse_oid_hex. If the object is not parseable, we\nreturn GIT_HASH_UNKNOWN.\n\nSigned-off-by: brian m. carlson \n---\n cache.h | 10 ++++++++++\n hex.c | 22 ++++++++++++++++++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 4c9c21a11d..158d7ccfd8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1523,6 +1523,16 @@ int parse_oid_hex(const char *hex, struct object_id *oid, const char **end);\n int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end,\n \t\t\tconst struct git_hash_algo *algo);\n \n+\n+/*\n+ * These functions work like get_oid_hex and parse_oid_hex, but they will parse\n+ * a hex value for any algorithm. The algorithm is detected based on the length\n+ * and the algorithm in use is returned. If this is not a hex object ID in any\n+ * algorithm, returns GIT_HASH_UNKNOWN.\n+ */\n+int get_oid_hex_any(const char *hex, struct object_id *oid);\n+int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);\n+\n /*\n * This reads short-hand syntax that not only evaluates to a commit\n * object name, but also can act as if the end user spelled the name\ndiff --git a/hex.c b/hex.c\nindex 10e24dc2e4..da51e64929 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -72,6 +72,20 @@ int get_oid_hex_algop(const char *hex, struct object_id *oid,\n \treturn get_hash_hex_algop(hex, oid->hash, algop);\n }\n \n+/*\n+ * NOTE: This function relies on hash algorithms being in order from shortest\n+ * length to longest length.\n+ */\n+int get_oid_hex_any(const char *hex, struct object_id *oid)\n+{\n+\tint i;\n+\tfor (i = GIT_HASH_NALGOS - 1; i > 0; i--) {\n+\t\tif (!get_hash_hex_algop(hex, oid->hash, &hash_algos[i]))\n+\t\t\treturn i;\n+\t}\n+\treturn GIT_HASH_UNKNOWN;\n+}\n+\n int get_oid_hex(const char *hex, struct object_id *oid)\n {\n \treturn get_oid_hex_algop(hex, oid, the_hash_algo);\n@@ -87,6 +101,14 @@ int parse_oid_hex_algop(const char *hex, struct object_id *oid,\n \treturn ret;\n }\n \n+int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end)\n+{\n+\tint ret = get_oid_hex_any(hex, oid);\n+\tif (ret)\n+\t\t*end = hex + hash_algos[ret].hexsz;\n+\treturn ret;\n+}\n+\n int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)\n {\n \treturn parse_oid_hex_algop(hex, oid, end, the_hash_algo);\n"},{"id":"392328","messageId":"20200222201749.937983-17-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 16/24] worktree: allow repository version 1","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:41Z","receivedAt":"2020-02-22T20:18:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Git supports both repository versions 0 and 1. These formats are\nidentical except for the presence of extensions. When using an\nextension, such as for a different hash algorithm, a check for only\nversion 0 causes the check to fail. Instead, call\nverify_repository_format to verify that we have an appropriate version\nand no unknown extensions.\n\nSigned-off-by: brian m. carlson \n---\n worktree.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 5b4793caa3..d1d23aadb4 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -449,7 +449,7 @@ const struct worktree *find_shared_symref(const char *symref,\n int submodule_uses_worktrees(const char *path)\n {\n \tchar *submodule_gitdir;\n-\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT, err = STRBUF_INIT;\n \tDIR *dir;\n \tstruct dirent *d;\n \tint ret = 0;\n@@ -463,18 +463,16 @@ int submodule_uses_worktrees(const char *path)\n \tget_common_dir_noenv(&sb, submodule_gitdir);\n \tfree(submodule_gitdir);\n \n-\t/*\n-\t * The check below is only known to be good for repository format\n-\t * version 0 at the time of writing this code.\n-\t */\n \tstrbuf_addstr(&sb, \"/config\");\n \tread_repository_format(&format, sb.buf);\n-\tif (format.version != 0) {\n+\tif (verify_repository_format(&format, &err)) {\n+\t\tstrbuf_release(&err);\n \t\tstrbuf_release(&sb);\n \t\tclear_repository_format(&format);\n \t\treturn 1;\n \t}\n \tclear_repository_format(&format);\n+\tstrbuf_release(&err);\n \n \t/* Replace config by worktrees. */\n \tstrbuf_setlen(&sb, sb.len - strlen(\"config\"));\n"},{"id":"392329","messageId":"20200222201749.937983-6-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 05/24] repository: require a build flag to use SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:30Z","receivedAt":"2020-02-22T20:18:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"At this point, SHA-256 support is experimental and some behavior may\nchange. To avoid surprising unsuspecting users, require a build flag,\nENABLE_SHA256, to allow use of a non-SHA-1 algorithm. Enable this flag\nby default when the DEVELOPER make flag is set so that contributors can\ntest this case adequately.\n\nSigned-off-by: brian m. carlson \n---\n config.mak.dev | 2 ++\n repository.c | 4 ++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/config.mak.dev b/config.mak.dev\nindex bf1f3fcdee..2e19435915 100644\n--- a/config.mak.dev\n+++ b/config.mak.dev\n@@ -17,6 +17,8 @@ DEVELOPER_CFLAGS += -Wstrict-prototypes\n DEVELOPER_CFLAGS += -Wunused\n DEVELOPER_CFLAGS += -Wvla\n \n+DEVELOPER_CFLAGS += -DENABLE_SHA256\n+\n ifndef COMPILER_FEATURES\n COMPILER_FEATURES := $(shell ./detect-compiler $(CC))\n endif\ndiff --git a/repository.c b/repository.c\nindex a4174ddb06..6f7f6f002b 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -89,6 +89,10 @@ void repo_set_gitdir(struct repository *repo,\n void repo_set_hash_algo(struct repository *repo, int hash_algo)\n {\n \trepo->hash_algo = &hash_algos[hash_algo];\n+#ifndef ENABLE_SHA256\n+\tif (hash_algo != GIT_HASH_SHA1)\n+\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), repo->hash_algo->name);\n+#endif\n }\n \n /*\n"},{"id":"392331","messageId":"20200222201749.937983-3-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 02/24] hash: implement and use a context cloning function","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:27Z","receivedAt":"2020-02-22T20:18:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"For all of our SHA-1 implementations and most of our SHA-256\nimplementations, the hash context we use is a real struct. For these\nimplementations, it's possible to copy a hash context by making a copy\nof the struct.\n\nHowever, for our libgcrypt implementation, our hash context is a\npointer. Consequently, copying it does not lead to an independent hash\ncontext like we intended.\n\nFortunately, however, libgcrypt provides us with a handy function to\ncopy hash contexts. Let's add a cloning function to the hash algorithm\nAPI, and use it in the one place we need to make a hash context copy.\nWith this change, our libgcrypt SHA-256 implementation is fully\nfunctional with all of our other hash implementations.\n\nSigned-off-by: brian m. carlson \n---\n csum-file.c | 2 +-\n hash.h | 21 +++++++++++++++++++++\n sha1-file.c | 18 ++++++++++++++++++\n sha256/gcrypt.h | 6 ++++++\n 4 files changed, 46 insertions(+), 1 deletion(-)\n\ndiff --git a/csum-file.c b/csum-file.c\nindex 53ce37f7ca..0f35fa5ee4 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -157,7 +157,7 @@ void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpo\n {\n \thashflush(f);\n \tcheckpoint->offset = f->total;\n-\tcheckpoint->ctx = f->ctx;\n+\tthe_hash_algo->clone_fn(&checkpoint->ctx, &f->ctx);\n }\n \n int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint)\ndiff --git a/hash.h b/hash.h\nindex 52a4f1a3f4..e0f3f16b06 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -16,6 +16,7 @@\n #endif\n \n #if defined(SHA256_GCRYPT)\n+#define SHA256_NEEDS_CLONE_HELPER\n #include \"sha256/gcrypt.h\"\n #elif defined(SHA256_OPENSSL)\n #include \n@@ -54,12 +55,28 @@\n #define git_SHA256_Update\tplatform_SHA256_Update\n #define git_SHA256_Final\tplatform_SHA256_Final\n \n+#ifdef platform_SHA256_Clone\n+#define git_SHA256_Clone\tplatform_SHA256_Clone\n+#endif\n+\n #ifdef SHA1_MAX_BLOCK_SIZE\n #include \"compat/sha1-chunked.h\"\n #undef git_SHA1_Update\n #define git_SHA1_Update\t\tgit_SHA1_Update_Chunked\n #endif\n \n+static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)\n+{\n+\tmemcpy(dst, src, sizeof(*dst));\n+}\n+\n+#ifndef SHA256_NEEDS_CLONE_HELPER\n+static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *src)\n+{\n+\tmemcpy(dst, src, sizeof(*dst));\n+}\n+#endif\n+\n /*\n * Note that these constants are suitable for indexing the hash_algos array and\n * comparing against each other, but are otherwise arbitrary, so they should not\n@@ -85,6 +102,7 @@ union git_hash_ctx {\n typedef union git_hash_ctx git_hash_ctx;\n \n typedef void (*git_hash_init_fn)(git_hash_ctx *ctx);\n+typedef void (*git_hash_clone_fn)(git_hash_ctx *dst, const git_hash_ctx *src);\n typedef void (*git_hash_update_fn)(git_hash_ctx *ctx, const void *in, size_t len);\n typedef void (*git_hash_final_fn)(unsigned char *hash, git_hash_ctx *ctx);\n \n@@ -110,6 +128,9 @@ struct git_hash_algo {\n \t/* The hash initialization function. */\n \tgit_hash_init_fn init_fn;\n \n+\t/* The hash context cloning function. */\n+\tgit_hash_clone_fn clone_fn;\n+\n \t/* The hash update function. */\n \tgit_hash_update_fn update_fn;\n \ndiff --git a/sha1-file.c b/sha1-file.c\nindex d785de8a85..c36ca5a545 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -74,6 +74,11 @@ static void git_hash_sha1_init(git_hash_ctx *ctx)\n \tgit_SHA1_Init(&ctx->sha1);\n }\n \n+static void git_hash_sha1_clone(git_hash_ctx *dst, const git_hash_ctx *src)\n+{\n+\tgit_SHA1_Clone(&dst->sha1, &src->sha1);\n+}\n+\n static void git_hash_sha1_update(git_hash_ctx *ctx, const void *data, size_t len)\n {\n \tgit_SHA1_Update(&ctx->sha1, data, len);\n@@ -90,6 +95,11 @@ static void git_hash_sha256_init(git_hash_ctx *ctx)\n \tgit_SHA256_Init(&ctx->sha256);\n }\n \n+static void git_hash_sha256_clone(git_hash_ctx *dst, const git_hash_ctx *src)\n+{\n+\tgit_SHA256_Clone(&dst->sha256, &src->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@@ -105,6 +115,11 @@ static void git_hash_unknown_init(git_hash_ctx *ctx)\n \tBUG(\"trying to init unknown hash\");\n }\n \n+static void git_hash_unknown_clone(git_hash_ctx *dst, const git_hash_ctx *src)\n+{\n+\tBUG(\"trying to clone unknown hash\");\n+}\n+\n static void git_hash_unknown_update(git_hash_ctx *ctx, const void *data, size_t len)\n {\n \tBUG(\"trying to update unknown hash\");\n@@ -123,6 +138,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\t0,\n \t\t0,\n \t\tgit_hash_unknown_init,\n+\t\tgit_hash_unknown_clone,\n \t\tgit_hash_unknown_update,\n \t\tgit_hash_unknown_final,\n \t\tNULL,\n@@ -136,6 +152,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\tGIT_SHA1_HEXSZ,\n \t\tGIT_SHA1_BLKSZ,\n \t\tgit_hash_sha1_init,\n+\t\tgit_hash_sha1_clone,\n \t\tgit_hash_sha1_update,\n \t\tgit_hash_sha1_final,\n \t\t&empty_tree_oid,\n@@ -149,6 +166,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {\n \t\tGIT_SHA256_HEXSZ,\n \t\tGIT_SHA256_BLKSZ,\n \t\tgit_hash_sha256_init,\n+\t\tgit_hash_sha256_clone,\n \t\tgit_hash_sha256_update,\n \t\tgit_hash_sha256_final,\n \t\t&empty_tree_oid_sha256,\ndiff --git a/sha256/gcrypt.h b/sha256/gcrypt.h\nindex 09bd8bb200..501da5ed91 100644\n--- a/sha256/gcrypt.h\n+++ b/sha256/gcrypt.h\n@@ -22,8 +22,14 @@ inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)\n \tmemcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);\n }\n \n+inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src)\n+{\n+\tgcry_md_copy(dst, *src);\n+}\n+\n #define platform_SHA256_CTX gcrypt_SHA256_CTX\n #define platform_SHA256_Init gcrypt_SHA256_Init\n+#define platform_SHA256_Clone gcrypt_SHA256_Clone\n #define platform_SHA256_Update gcrypt_SHA256_Update\n #define platform_SHA256_Final gcrypt_SHA256_Final\n \n"},{"id":"392332","messageId":"20200222201749.937983-16-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 15/24] init-db: move writing repo version into a function","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:40Z","receivedAt":"2020-02-22T20:18:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we perform a clone, we won't know the remote side's hash algorithm\nuntil we've read the heads. Consequently, we'll need to rewrite the\nrepository format version and hash algorithm once we know what the\nremote side has. Move the code that does this into its own function so\nthat we can call it from clone in the future.\n\nSigned-off-by: brian m. carlson \n---\n builtin/init-db.c | 42 ++++++++++++++++++++++++------------------\n cache.h | 1 +\n 2 files changed, 25 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex ab4fd682ab..5d96e59885 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -178,6 +178,29 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n \treturn 1;\n }\n \n+void initialize_repository_version(int hash_algo)\n+{\n+\tchar repo_version_string[10];\n+\tint repo_version = GIT_REPO_VERSION;\n+\n+#ifndef ENABLE_SHA256\n+\tif (hash_algo != GIT_HASH_SHA1)\n+\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[hash_algo].name);\n+#endif\n+\n+\tif (hash_algo != GIT_HASH_SHA1)\n+\t\trepo_version = GIT_REPO_VERSION_READ;\n+\n+\t/* This forces creation of new config file */\n+\txsnprintf(repo_version_string, sizeof(repo_version_string),\n+\t\t \"%d\", repo_version);\n+\tgit_config_set(\"core.repositoryformatversion\", repo_version_string);\n+\n+\tif (hash_algo != GIT_HASH_SHA1)\n+\t\tgit_config_set(\"extensions.objectformat\",\n+\t\t\t hash_algos[hash_algo].name);\n+}\n+\n static int create_default_files(const char *template_path,\n \t\t\t\tconst char *original_git_dir,\n \t\t\t\tconst struct repository_format *fmt)\n@@ -185,12 +208,10 @@ static int create_default_files(const char *template_path,\n \tstruct stat st1;\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *path;\n-\tchar repo_version_string[10];\n \tchar junk[2];\n \tint reinit;\n \tint filemode;\n \tstruct strbuf err = STRBUF_INIT;\n-\tint repo_version = GIT_REPO_VERSION;\n \n \t/* Just look for `init.templatedir` */\n \tinit_db_template_dir = NULL; /* re-set in case it was set before */\n@@ -248,22 +269,7 @@ static int create_default_files(const char *template_path,\n \t\t\texit(1);\n \t}\n \n-#ifndef ENABLE_SHA256\n-\tif (fmt->hash_algo != GIT_HASH_SHA1)\n-\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n-#endif\n-\n-\tif (fmt->hash_algo != GIT_HASH_SHA1)\n-\t\trepo_version = GIT_REPO_VERSION_READ;\n-\n-\t/* This forces creation of new config file */\n-\txsnprintf(repo_version_string, sizeof(repo_version_string),\n-\t\t \"%d\", repo_version);\n-\tgit_config_set(\"core.repositoryformatversion\", repo_version_string);\n-\n-\tif (fmt->hash_algo != GIT_HASH_SHA1)\n-\t\tgit_config_set(\"extensions.objectformat\",\n-\t\t\t hash_algos[fmt->hash_algo].name);\n+\tinitialize_repository_version(fmt->hash_algo);\n \n \t/* Check filemode trustability */\n \tpath = git_path_buf(&buf, \"config\");\ndiff --git a/cache.h b/cache.h\nindex 7a47e023ba..0653318d2b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -629,6 +629,7 @@ int path_inside_repo(const char *prefix, const char *path);\n int init_db(const char *git_dir, const char *real_git_dir,\n \t const char *template_dir, int hash_algo,\n \t unsigned int flags);\n+void initialize_repository_version(int hash_algo);\n \n void sanitize_stdfds(void);\n int daemonize(void);\n"},{"id":"392333","messageId":"20200222201749.937983-9-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 08/24] t6300: make hash algorithm independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:33Z","receivedAt":"2020-02-22T20:18:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"One of the git for-each-ref tests asks to sort by object ID. However,\nwhen sorted, the order of the refs differs between SHA-1 and SHA-256.\nSort the expected output so that the test passes.\n\nSigned-off-by: brian m. carlson \n---\n t/t6300-for-each-ref.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2406b93d35..b3c1092338 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -650,7 +650,7 @@ test_atom refs/tags/signed-long contents \"subject line\n body contents\n $sig\"\n \n-cat >expected <expected < refs/tags/bogo\n $(git rev-parse refs/tags/master) refs/tags/master\n EOF\n"},{"id":"392334","messageId":"20200222201749.937983-10-sandals@crustytoothpaste.net","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 09/24] t/helper/test-dump-split-index: initialize git repository","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-22T20:17:34Z","receivedAt":"2020-02-22T20:18:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In this test helper, we read the index. In order to have the proper\nhash algorithm set up, we must call setup_git_directory. Do so, so that\nthe test works when extensions.objectFormat is set.\n\nSigned-off-by: brian m. carlson \n---\n t/helper/test-dump-split-index.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-dump-split-index.c b/t/helper/test-dump-split-index.c\nindex 63c689d6ee..a209880eb3 100644\n--- a/t/helper/test-dump-split-index.c\n+++ b/t/helper/test-dump-split-index.c\n@@ -13,6 +13,8 @@ int cmd__dump_split_index(int ac, const char **av)\n \tstruct split_index *si;\n \tint i;\n \n+\tsetup_git_directory();\n+\n \tdo_read_index(&the_index, av[1], 1);\n \tprintf(\"own %s\\n\", oid_to_hex(&the_index.oid));\n \tsi = the_index.split_index;\n"},{"id":"392366","messageId":"20200223215759.GC564691@coredump.intra.peff.net","threadId":"52869","inReplyTo":"20200222201749.937983-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 01/24] builtin/pack-objects: make hash agnostic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-23T21:57:59Z","receivedAt":"2020-02-23T21:58:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 22, 2020 at 08:17:26PM +0000, brian m. carlson wrote:\n\n> Avoid hard-coding a hash size, instead preferring to use the_hash_algo.\n> [...]\n> -\t\t\thashwrite(out, base_sha1, 20);\n> +\t\t\thashwrite(out, base_sha1, the_hash_algo->rawsz);\n\nYeah, looks obviously correct (and I think this is new from the\npack-reuse patches of mine that Christian sent recently).\n\nThe name \"base_sha1\" is clearly not great either. It could be changed\ntrivially, but the real sin is that it comes from\nnth_packed_object_sha1(). It could be replaced with\nnth_packed_object_oid() at the cost of an extra hash copy, which isn't\ntoo bad.\n\nIt would be nice to get rid of that function entirely. In most cases,\nit's either free to do so (we end up copying the result into an oid\nanyway) or we pay for one extra hashcpy (out of the mmap into a local\nstruct). But the one in pack-check.c:verify_packfile() is tricky; we\nstore a pointer per object into the index mmap, and we'd have to switch\nto storing an oid per object. Given that the code isn't commonly run\n(and other operations like _generating_ the index in the first place are\nclearly OK with the extra memory hit), I think I'd be OK with that in\nthe name of cleanliness.\n\nAll of that is sort of orthogonal to your goals, I think, so I don't\nmind at all calling it out of scope for your series. As long as we use\nthe_hash_algo->rawsz, these bare pointer accesses aren't wrong (it's\njust that it's easy to accidentally get it wrong, as this code shows).\n\nI think it would also be correct to cast the mmap'd bytes to a \"struct\nobject_id\" given that the struct contains the hash bytes as the first\nmember. I worry a little that we'd get no compiler warning of the\nbreakage if that assumption changes, but it also seems unlikely to do\nso.\n\n-Peff\n"},{"id":"392372","messageId":"20200224030142.GA618173@coredump.intra.peff.net","threadId":"52869","inReplyTo":"20200223215759.GC564691@coredump.intra.peff.net","subject":"Re: [PATCH v2 01/24] builtin/pack-objects: make hash agnostic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-24T03:01:42Z","receivedAt":"2020-02-24T03:01:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 23, 2020 at 04:57:59PM -0500, Jeff King wrote:\n\n> It would be nice to get rid of [nth_packed_object_sha1] entirely. In most cases,\n> it's either free to do so (we end up copying the result into an oid\n> anyway) or we pay for one extra hashcpy (out of the mmap into a local\n> struct). But the one in pack-check.c:verify_packfile() is tricky; we\n> store a pointer per object into the index mmap, and we'd have to switch\n> to storing an oid per object. Given that the code isn't commonly run\n> (and other operations like _generating_ the index in the first place are\n> clearly OK with the extra memory hit), I think I'd be OK with that in\n> the name of cleanliness.\n> \n> All of that is sort of orthogonal to your goals, I think, so I don't\n> mind at all calling it out of scope for your series. As long as we use\n> the_hash_algo->rawsz, these bare pointer accesses aren't wrong (it's\n> just that it's easy to accidentally get it wrong, as this code shows).\n\nI looked into this a bit further. It turns out that the current code in\nverify_packfile() was explicitly trying to avoid that extra memory. But\nthe good news is that I think we can improve it further, cleaning up the\nexisting type-punning and using even less memory than now.\n\nI'll prepare a separate series to do that. I had thought the cleanup\nmight also make this whole 20 / the_hash_algo->rawsz thing go away, but\nit doesn't (the name hashwrite() made me think we ought to be using an\noidwrite(), but of course that's silly; the \"hash\" here is a \"file with\na hash checksum\" and not \"an object id hash\"). So the two topics really\nare independent.\n\n-Peff\n"},{"id":"392373","messageId":"20200224034259.GG6462@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"20200223215759.GC564691@coredump.intra.peff.net","subject":"Re: [PATCH v2 01/24] builtin/pack-objects: make hash agnostic","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-24T03:42:59Z","receivedAt":"2020-02-24T03:43:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-02-23 at 21:57:59, Jeff King wrote:\n> On Sat, Feb 22, 2020 at 08:17:26PM +0000, brian m. carlson wrote:\n> \n> > Avoid hard-coding a hash size, instead preferring to use the_hash_algo.\n> > [...]\n> > -\t\t\thashwrite(out, base_sha1, 20);\n> > +\t\t\thashwrite(out, base_sha1, the_hash_algo->rawsz);\n> \n> Yeah, looks obviously correct (and I think this is new from the\n> pack-reuse patches of mine that Christian sent recently).\n\nI believe it is, which is why I CC'd you on it.\n\n> The name \"base_sha1\" is clearly not great either. It could be changed\n> trivially, but the real sin is that it comes from\n> nth_packed_object_sha1(). It could be replaced with\n> nth_packed_object_oid() at the cost of an extra hash copy, which isn't\n> too bad.\n\nI probably should send a series getting rid of the rest of the \"sha1\"\nplaces in our code, because there are a lot of them, but I just haven't\ngotten around to it yet. And yeah, as you mentioned, there are still a\nlot of places using nth_packed_object_sha1.\n\n> It would be nice to get rid of that function entirely. In most cases,\n> it's either free to do so (we end up copying the result into an oid\n> anyway) or we pay for one extra hashcpy (out of the mmap into a local\n> struct). But the one in pack-check.c:verify_packfile() is tricky; we\n> store a pointer per object into the index mmap, and we'd have to switch\n> to storing an oid per object. Given that the code isn't commonly run\n> (and other operations like _generating_ the index in the first place are\n> clearly OK with the extra memory hit), I think I'd be OK with that in\n> the name of cleanliness.\n\nYeah, that's why I hadn't switched it out earlier.\n\n> I think it would also be correct to cast the mmap'd bytes to a \"struct\n> object_id\" given that the struct contains the hash bytes as the first\n> member. I worry a little that we'd get no compiler warning of the\n> breakage if that assumption changes, but it also seems unlikely to do\n> so.\n\nIn the future, struct object_id will get a new member (an algorithm\nvalue), but I think it's fine to make the assumption that the hash bytes\nare first.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"392375","messageId":"20200224042004.GA731094@coredump.intra.peff.net","threadId":"52869","inReplyTo":"20200224034259.GG6462@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 01/24] builtin/pack-objects: make hash agnostic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-24T04:20:04Z","receivedAt":"2020-02-24T04:20:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 24, 2020 at 03:42:59AM +0000, brian m. carlson wrote:\n\n> > Yeah, looks obviously correct (and I think this is new from the\n> > pack-reuse patches of mine that Christian sent recently).\n> \n> I believe it is, which is why I CC'd you on it.\n\nHeh, yeah. I knew you knew, but was saying so for the rest of the\naudience. :)\n\n> > I think it would also be correct to cast the mmap'd bytes to a \"struct\n> > object_id\" given that the struct contains the hash bytes as the first\n> > member. I worry a little that we'd get no compiler warning of the\n> > breakage if that assumption changes, but it also seems unlikely to do\n> > so.\n> \n> In the future, struct object_id will get a new member (an algorithm\n> value), but I think it's fine to make the assumption that the hash bytes\n> are first.\n\nYeah, I think that would continue to work, although weirdness would\nensue if anybody ever dereferenced the algorithm member in one of the\ntype-punned structs. If we can avoid it entirely, I think we should (and\nit was easy to remove the spot in pack-check).\n\nPatches incoming.\n\n-Peff\n"},{"id":"392415","messageId":"xmqqsgizrhjv.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-8-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 07/24] t6300: abstract away SHA-1-specific constants","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:01:08Z","receivedAt":"2020-02-24T18:01:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> Adjust the test so that it computes variables for object IDs instead of\n> using hard-coded hashes.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> t/t6300-for-each-ref.sh | 25 ++++++++++++++++---------\n> 1 file changed, 16 insertions(+), 9 deletions(-)\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 9c910ce746..2406b93d35 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -20,6 +20,10 @@ setdate_and_increment () {\n> }\n> \n> test_expect_success setup '\n> +\ttest_oid_cache <<-EOF &&\n> +\tdisklen sha1:138\n> +\tdisklen sha256:154\n> +\tEOF\n> \tsetdate_and_increment &&\n> \techo \"Using $datestamp\" > one &&\n> \tgit add one &&\n> @@ -50,6 +54,9 @@ test_atom() {\n> \t\"\n> }\n> \n> +hexlen=$(test_oid hexsz)\n> +disklen=$(test_oid disklen)\n> +\n> test_atom head refname refs/heads/master\n> test_atom head refname: refs/heads/master\n> test_atom head refname:short master\n> @@ -82,9 +89,9 @@ test_atom head push:rstrip=-1 refs\n> test_atom head push:strip=1 remotes/myfork/master\n> test_atom head push:strip=-1 master\n> test_atom head objecttype commit\n> -test_atom head objectsize 171\n> -test_atom head objectsize:disk 138\n> -test_atom head deltabase 0000000000000000000000000000000000000000\n> +test_atom head objectsize $((131 + hexlen))\n\n171 == 131 + 40 and that is because we are looking at the initial\ncommit, whose contents has a single object name (i.e. its tree).\n\nMakes sense.\n\n> +test_atom head objectsize:disk $disklen\n> +test_atom head deltabase $ZERO_OID\n> test_atom head objectname $(git rev-parse refs/heads/master)\n> test_atom head objectname:short $(git rev-parse --short refs/heads/master)\n> test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n> @@ -125,11 +132,11 @@ test_atom tag refname:short testtag\n> test_atom tag upstream ''\n> test_atom tag push ''\n> test_atom tag objecttype tag\n> -test_atom tag objectsize 154\n> -test_atom tag objectsize:disk 138\n> -test_atom tag '*objectsize:disk' 138\n> -test_atom tag deltabase 0000000000000000000000000000000000000000\n> -test_atom tag '*deltabase' 0000000000000000000000000000000000000000\n> +test_atom tag objectsize $((114 + hexlen))\n\nLikewise, 154 == 114 + 40 because an annotated tag has an object\npointer to a single object (i.e. its pointee).\n\nMakes sense, too.\n\n> +test_atom tag objectsize:disk $disklen\n> +test_atom tag '*objectsize:disk' $disklen\n> +test_atom tag deltabase $ZERO_OID\n> +test_atom tag '*deltabase' $ZERO_OID\n> test_atom tag objectname $(git rev-parse refs/tags/testtag)\n> test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)\n> test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n> @@ -139,7 +146,7 @@ test_atom tag parent ''\n> test_atom tag numparent ''\n> test_atom tag object $(git rev-parse refs/tags/testtag^0)\n> test_atom tag type 'commit'\n> -test_atom tag '*objectname' 'ea122842f48be4afb2d1fc6a4b96c05885ab7463'\n> +test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})\n> test_atom tag '*objecttype' 'commit'\n> test_atom tag author ''\n> test_atom tag authorname ''\n"},{"id":"392416","messageId":"xmqqo8tnrhbw.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-11-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 10/24] t/helper: initialize repository if necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:05:55Z","receivedAt":"2020-02-24T18:06:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> The repository helper is used in t5318 to read commit graphs whether\n> we're in a repository or not. However, without a repository, we have no\n> way to properly initialize the hash algorithm, meaning that the file is\n> misread.\n>\n> Fix this by calling setup_git_directory_gently, which will read the\n> environment variable the testsuite sets to ensure that the correct hash\n> algorithm is set.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> t/helper/test-repository.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\n> index f7f8618445..ecc768e4cb 100644\n> --- a/t/helper/test-repository.c\n> +++ b/t/helper/test-repository.c\n> @@ -75,6 +75,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n> \n> int cmd__repository(int argc, const char **argv)\n> {\n> +\tint nongit_ok = 0;\n> +\n> +\tsetup_git_directory_gently(&nongit_ok);\n\nNo need to initialize nongit_ok to any specific value before calling\nsetup_git_directory_gently() and I personally find this initialization\nunhelpful to new readers, as it misleadingly hints the setup process\nmay be affected by the value passed in by the value in nongit_ok,\nwhen in reality the variable is purely used as outout-only (the\nfirst thing the function does is to clear it).\n\nWas it necessary to work around a compiler warning or something?\n\n> \tif (argc < 2)\n> \t\tdie(\"must have at least 2 arguments\");\n> \tif (!strcmp(argv[1], \"parse_commit_in_graph\")) {\n"},{"id":"392417","messageId":"20200224181204.GA2130083@coredump.intra.peff.net","threadId":"52869","inReplyTo":"xmqqsgizrhjv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 07/24] t6300: abstract away SHA-1-specific constants","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-24T18:12:04Z","receivedAt":"2020-02-24T18:12:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 24, 2020 at 10:01:08AM -0800, Junio C Hamano wrote:\n\n> > -test_atom head objectsize 171\n> > -test_atom head objectsize:disk 138\n> > -test_atom head deltabase 0000000000000000000000000000000000000000\n> > +test_atom head objectsize $((131 + hexlen))\n> \n> 171 == 131 + 40 and that is because we are looking at the initial\n> commit, whose contents has a single object name (i.e. its tree).\n\nI wonder if it would be more readable to just pipe \"cat-file\" through\n\"wc -c\", rather than hard-coding. Then there's no magic number at all.\n\n> > +test_atom head objectsize:disk $disklen\n\nLikewise for $disklen, if it's a loose object we could just get the\ninformation from the filesystem. That would stop us caring about the\nhash, _and_ it would make us robust to random changes in the zlib\ncompression.\n\nI'm not sure if we also check packed objects. If so, you can compute the\nsize from the output of show-index, which gives the offsets of each\nobject. That's also how Git does it internally, though, so I'm not sure\nif that is getting too close to just testing nothing (but IMHO the thing\nwe're really covering here is the format routines).\n\n-Peff\n"},{"id":"392418","messageId":"xmqqh7zfrgxi.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-14-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 13/24] builtin/init-db: allow specifying hash algorithm on command line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:14:33Z","receivedAt":"2020-02-24T18:14:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> 'git init' [-q | --quiet] [--bare] [--template=]\n> -\t [--separate-git-dir ]\n> +\t [--separate-git-dir ] [--object-format= in pair.\n\n> +#ifndef ENABLE_SHA256\n> +\tif (fmt->hash_algo != GIT_HASH_SHA1)\n> +\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n\nCould you fold the overlong line here?\n\n> int init_db(const char *git_dir, const char *real_git_dir,\n> -\t const char *template_dir, unsigned int flags)\n> +\t const char *template_dir, int hash, unsigned int flags)\n\nPerhaps rename \"hash\" to \"hash_algo\"? I know that it is very\nunlikely for a variable whose name is 'hash' to be mistaken as a raw\nhash value when its type is \"int\" (as opposed to say char *), but\nstill. I wouldn't be saying this if its type were an \"enum\nhash_algo\" or something like that.\n\n> +\tconst char *object_format = NULL;\n> +\tint hash_algo = GIT_HASH_UNKNOWN;\n\nThis one _is_ good.\n\n> \tconst struct option init_db_options[] = {\n> \t\tOPT_STRING(0, \"template\", &template_dir, N_(\"template-directory\"),\n> \t\t\t\tN_(\"directory from which templates will be used\")),\n> @@ -494,6 +526,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT('q', \"quiet\", &flags, N_(\"be quiet\"), INIT_DB_QUIET),\n> \t\tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n> \t\t\t N_(\"separate git dir from working tree\")),\n> +\t\tOPT_STRING(0, \"object-format\", &object_format, N_(\"hash\"),\n> +\t\t\t N_(\"specify the hash algorithm to use\")),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -546,6 +580,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n> \t\tfree(cwd);\n> \t}\n> \n> +\tif (object_format) {\n> +\t\thash_algo = hash_algo_by_name(object_format);\n> +\t\tif (hash_algo == GIT_HASH_UNKNOWN)\n> +\t\t\tdie(_(\"unknown hash algorithm '%s'\"), object_format);\n> +\t}\n> +\n> \tif (init_shared_repository != -1)\n> \t\tset_shared_repository(init_shared_repository);\n> \n> @@ -597,5 +637,5 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n> \tUNLEAK(work_tree);\n> \n> \tflags |= INIT_DB_EXIST_OK;\n> -\treturn init_db(git_dir, real_git_dir, template_dir, flags);\n> +\treturn init_db(git_dir, real_git_dir, template_dir, hash_algo, flags);\n> }\n> diff --git a/cache.h b/cache.h\n> index 29ee02a8d4..7a47e023ba 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -627,7 +627,8 @@ int path_inside_repo(const char *prefix, const char *path);\n> #define INIT_DB_EXIST_OK 0x0002\n> \n> int init_db(const char *git_dir, const char *real_git_dir,\n> -\t const char *template_dir, unsigned int flags);\n> +\t const char *template_dir, int hash_algo,\n\nSo is this one.\n\n> +\t unsigned int flags);\n> \n> void sanitize_stdfds(void);\n> int daemonize(void);\n"},{"id":"392421","messageId":"xmqqd0a3rggt.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-18-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 17/24] commit: use expected signature header for SHA-256","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:24:34Z","receivedAt":"2020-02-24T18:24:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 7ba33a3bec..798d362a2e 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t}\n> \n> \tif (amend) {\n> -\t\tconst char *exclude_gpgsig[2] = { \"gpgsig\", NULL };\n> +\t\tconst char *exclude_gpgsig[3] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n\nFor futureproofing, we should eventually revamp the machinery of\nread_commit_extra_headers() so that we do not have to maintain this\narray. Instead, the second parameter, that is passed down to\nread_commit_extra_header_lines() helper, should become a callback\nfunction that lets the caller say \"no, I do not want this header\",\nand we can use it to exclude \"gpgsig\", \"gpgsig-sha256\", and anything\nthat begins with \"gpgsig-\" in the future. We might even want to\nexclude \"gpgsig-$algo\" where \"$algo\" is the name of the algorithm\nthat this version of Git does not yet recognise while giving a\nwarning, or perhaps we may want to cause read_commit_extra_headers()\nto yield an error on gpgsig with unknown hashalgo. These become\neasier to arrange with such a machinery update.\n\nOne question I have is if it makes sense to do so now before this\nseries, or we leave a \"NEEDSWORK:\" note and complete SHA-2 transition\nwith the current mechanism.\n\n> +test_expect_success 'commit with multiple signatures is okay' '\n> +\tgit cat-file commit HEAD >basis &&\n> +\tcat >sigs <<-EOF &&\n> +\tgpgsig -----BEGIN PGP SIGNATURE-----\n> +\t VGhpcyBpcyBub3QgcmVhbGx5IGEgc2lnbmF0dXJlLg==\n> +\t -----END PGP SIGNATURE-----\n> +\tgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n> +\t VGhpcyBpcyBub3QgcmVhbGx5IGEgc2lnbmF0dXJlLg==\n> +\t -----END PGP SIGNATURE-----\n> +\tEOF\n> +\tsed -e \"/^committer/q\" basis >okay &&\n> +\tcat sigs >>okay &&\n> +\techo >>okay &&\n> +\tsed -e \"1,/^$/d\" basis >>okay &&\n> +\tcat okay &&\n> +\tnew=$(git hash-object -t commit -w --stdin +\ttest_when_finished \"remove_object $new\" &&\n> +\tgit update-ref refs/heads/bogus \"$new\" &&\n> +\ttest_when_finished \"git update-ref -d refs/heads/bogus\" &&\n> +\tgit fsck 2>out &&\n> +\tcat out &&\n> +\t! grep \"commit $new\" out\n> +'\n\nThere seem to be a few debugging leftover cats in the above.\n"},{"id":"392422","messageId":"xmqq8skrrgd6.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-19-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:26:45Z","receivedAt":"2020-02-24T18:26:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> We have a function which parses a buffer with a signature at the end,\n> parse_signature, and this function is used for signed tags. However,\n> the transition plan has SHA-256 tags using a header, which is a\n> materially different syntax. The current interface is not suitable for\n> parsing such tags.\n>\n> Adjust the parse_signature interface to store the parsed data in two\n> strbufs and turn the existing function into parse_signed_buffer. The\n> latter is still used in places where we want to strip off the signature\n> in a SHA-1 tag or in places where we know we always have a signed\n> buffer, such as push certs.\n>\n> Adjust all the callers to deal with this new interface.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> builtin/fmt-merge-msg.c | 26 ++++++++++++++++++--------\n> builtin/receive-pack.c | 4 ++--\n> builtin/tag.c | 16 ++++++++++++----\n> commit.c | 9 ++++++---\n> gpg-interface.c | 13 ++++++++++++-\n> gpg-interface.h | 9 ++++++++-\n> log-tree.c | 14 ++++++++------\n> ref-filter.c | 18 ++++++++++++++----\n> tag.c | 15 ++++++++-------\n> 9 files changed, 88 insertions(+), 36 deletions(-)\n>\n> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\n> index 05a92c59d8..29f647e2d9 100644\n> --- a/builtin/fmt-merge-msg.c\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -472,6 +472,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n> \t\t\t const char *buf,\n> \t\t\t unsigned long len)\n> {\n> +\n> \tconst char *tag_body = strstr(buf, \"\\n\\n\");\n> \tif (tag_body) {\n> \t\ttag_body += 2;\n\nIs this hunk a pun to rhyme with the strstr ;-)?\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 2d538bcd6e..b25f5c21d8 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n> \t\tfputs(output, stderr);\n> }\n> \n> -size_t parse_signature(const char *buf, size_t size)\n> +size_t parse_signed_buffer(const char *buf, size_t size)\n> {\n> \tsize_t len = 0;\n> \tsize_t match = size;\n> @@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n> \treturn match;\n> }\n> \n> +int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n> +{\n> +\tsize_t match = parse_signed_buffer(buf, size);\n> +\tif (match != size) {\n> +\t\tstrbuf_add(payload, buf, match);\n> +\t\tstrbuf_add(signature, buf + match, size - match);\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n\nMakes sense.\n\n"},{"id":"392424","messageId":"xmqq36azrg6g.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-20-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 19/24] tag: store SHA-256 signatures in a header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:30:47Z","receivedAt":"2020-02-24T18:30:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> In the future, we'll want to allow a user to sign both the SHA-1 version\n> of a tag and the SHA-256 version at the same time. Since for SHA-1 the\n> signature is appended to the tag message, we must use a different way to\n> allow multiple signature.\n\n\"multiple signatureS\"?\n\n> The transition plan envisions this using a gpgsig-sha256 header, much as\n> for commits. Refactor the commit code that performs parsing of this\n> header and use it for tags that use SHA-256. Check that we get tags in\n> the correct format depending on what algorithm we're using.\n\nSounds sensible.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index b3c1092338..caeabfb293 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -54,6 +54,17 @@ test_atom() {\n> \t\"\n> }\n> \n> +test_atom_hash () {\n> +\tlocal val\n> +\tif [ \"$(test_oid algo)\" = sha1 ]\n> +\tthen\n\n if test \"$(...)\" = sha1\n then\n\nI saw there are a few other copies of these in the result of\napplying the whole series.\n\n> +\telse\n> +\n> +\t\tgit cat-file tag \"$1\" | sed -e '/^gpgsig-sha256/{s/^gpgsig-sha256 //;h;d};/^ /d;${p;x;/^$/d}'\n\nPlease line-wrap an overlong line, immediately after the pipe '|' character.\n\n> +\tfi\n> }\n"},{"id":"392425","messageId":"xmqqy2srq1g3.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 00/24] SHA-256 stage 4 implementation, part 1/3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T18:34:20Z","receivedAt":"2020-02-24T18:34:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> This is a series for part 1 of 3 of a stage 4 SHA-256 implementation.\n> It is mostly the same as v1[0], which was RFC. The interested reader is\n> referred there for the in-depth explanations.\n>\n> A few interesting changes have taken place since v1. First, I\n> discovered a regression in a recent series which introduced a hard-coded\n> constant, so patch 1 addresses this.\n>\n> Second, I discovered a mistaken assumption that we were making about our\n> hash implementations: that copying the struct was sufficient to copy the\n> context. This is not true for libgcrypt, where our context is a pointer\n> instead, so patch 2 addresses this with a helper function.\n>\n> Finally, I've added a check to prevent non-developers from creating\n> SHA-256 repositories, since this series is not sufficient to implement\n> full SHA-256 support. Even as a developer, creating a SHA-256\n> repository immediately leads to a broken state, since we don't recognize\n> the extension (yet) and therefore promptly refuse to operate on it in\n> any way. Preventing this experience seemed prudent.\n\nI am very tempted to take 1/24 separately and queue it at the tip of\nthe jk/packfile-reuse-cleanup topic.\n\nI didn't read the fast-import bits at the end of the series, but\neverything before those steps made sense to me.\n\nThanks. \n\n"},{"id":"392435","messageId":"xmqqd0a3pvjl.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200224181204.GA2130083@coredump.intra.peff.net","subject":"Re: [PATCH v2 07/24] t6300: abstract away SHA-1-specific constants","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-24T20:41:50Z","receivedAt":"2020-02-24T20:41:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 24, 2020 at 10:01:08AM -0800, Junio C Hamano wrote:\n>\n>> > -test_atom head objectsize 171\n>> > -test_atom head objectsize:disk 138\n>> > -test_atom head deltabase 0000000000000000000000000000000000000000\n>> > +test_atom head objectsize $((131 + hexlen))\n>> \n>> 171 == 131 + 40 and that is because we are looking at the initial\n>> commit, whose contents has a single object name (i.e. its tree).\n>\n> I wonder if it would be more readable to just pipe \"cat-file\" through\n> \"wc -c\", rather than hard-coding. Then there's no magic number at all.\n\nAfter seeing nearby tests use output from $(git rev-parse) as the\nexpected output, I had a similar thought.\n\n>> > +test_atom head objectsize:disk $disklen\n>\n> Likewise for $disklen, if it's a loose object we could just get the\n> information from the filesystem. That would stop us caring about the\n> hash, _and_ it would make us robust to random changes in the zlib\n> compression.\n>\n> I'm not sure if we also check packed objects. If so, you can compute the\n> size from the output of show-index, which gives the offsets of each\n> object. That's also how Git does it internally, though, so I'm not sure\n> if that is getting too close to just testing nothing (but IMHO the thing\n> we're really covering here is the format routines).\n\nSomebody may find it tempting to use \"cat-file --batch-check=\"\nand at that point it would really become fuzzy what we are checking ;-)\n"},{"id":"392457","messageId":"20200225000505.GH6462@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"xmqqo8tnrhbw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 10/24] t/helper: initialize repository if necessary","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-25T00:05:05Z","receivedAt":"2020-02-25T00:05:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-02-24 at 18:05:55, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> > diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\n> > index f7f8618445..ecc768e4cb 100644\n> > --- a/t/helper/test-repository.c\n> > +++ b/t/helper/test-repository.c\n> > @@ -75,6 +75,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n> > \n> > int cmd__repository(int argc, const char **argv)\n> > {\n> > +\tint nongit_ok = 0;\n> > +\n> > +\tsetup_git_directory_gently(&nongit_ok);\n> \n> No need to initialize nongit_ok to any specific value before calling\n> setup_git_directory_gently() and I personally find this initialization\n> unhelpful to new readers, as it misleadingly hints the setup process\n> may be affected by the value passed in by the value in nongit_ok,\n> when in reality the variable is purely used as outout-only (the\n> first thing the function does is to clear it).\n> \n> Was it necessary to work around a compiler warning or something?\n\nI don't recall, but since this seems to be the only place we do that, I\nassume it's not, and I'll just drop it.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"392458","messageId":"20200225001116.GI6462@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"xmqqh7zfrgxi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 13/24] builtin/init-db: allow specifying hash algorithm on command line","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-25T00:11:16Z","receivedAt":"2020-02-25T00:11:22Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-02-24 at 18:14:33, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > 'git init' [-q | --quiet] [--bare] [--template=]\n> > -\t [--separate-git-dir ]\n> > +\t [--separate-git-dir ] [--object-format= \n> A missing closing ket> in pair.\n\nGood catch.\n\n> > +#ifndef ENABLE_SHA256\n> > +\tif (fmt->hash_algo != GIT_HASH_SHA1)\n> > +\t\tdie(_(\"The hash algorithm %s is not supported in this build.\"), hash_algos[fmt->hash_algo].name);\n> \n> Could you fold the overlong line here?\n\nAbsolutely.\n\n> > int init_db(const char *git_dir, const char *real_git_dir,\n> > -\t const char *template_dir, unsigned int flags)\n> > +\t const char *template_dir, int hash, unsigned int flags)\n> \n> Perhaps rename \"hash\" to \"hash_algo\"? I know that it is very\n> unlikely for a variable whose name is 'hash' to be mistaken as a raw\n> hash value when its type is \"int\" (as opposed to say char *), but\n> still. I wouldn't be saying this if its type were an \"enum\n> hash_algo\" or something like that.\n\nSure, that's reasonable.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"392464","messageId":"nycvar.QRO.7.76.6.2002251127320.46@tvgsbejvaqbjf.bet","threadId":"52869","inReplyTo":"20200222201749.937983-19-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-02-25T10:29:26Z","receivedAt":"2020-02-25T10:29:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\nOn Sat, 22 Feb 2020, brian m. carlson wrote:\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 6867e33648..212f1165bb 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1161,7 +1161,13 @@ static void find_subpos(const char *buf,\n> \t\t\tunsigned long *nonsiglen,\n> \t\t\tconst char **sig, unsigned long *siglen)\n> {\n> +\tstruct strbuf payload = STRBUF_INIT;\n> +\tstruct strbuf signature = STRBUF_INIT;\n> \tconst char *eol;\n> +\tconst char *end = buf + strlen(buf);\n> +\tconst char *sigstart;\n> +\n> +\n> \t/* skip past header until we hit empty line */\n> \twhile (*buf && *buf != '\\n') {\n> \t\teol = strchrnul(buf, '\\n');\n> @@ -1174,13 +1180,14 @@ static void find_subpos(const char *buf,\n> \t\tbuf++;\n>\n> \t/* parse signature first; we might not even have a subject line */\n> -\t*sig = buf + parse_signature(buf, strlen(buf));\n> -\t*siglen = strlen(*sig);\n> +\tparse_signature(buf, end - buf, &payload, &signature);\n> +\t*sig = strbuf_detach(&signature, siglen);\n\nWhile I like the spirit of this patch, it makes the Windows build fail. I\nput this on top of Git for Windows' `shears/pu` branch to fix it (maybe\nyou could adopt a variation of it?):\n\n-- snipsnap --\nSubject: [PATCH] fixup??? gpg-interface: improve interface for parsing tags\n\nIn 3f69139fa39 (gpg-interface: improve interface for parsing tags,\n2020-02-22), we introduce a call to `strbuf_detach()`. The second\nparameter of that function takes a pointer to the variable where the\nlength of the string should be stored.\n\nHowever, `strbuf_detach()` uses the 21st century data type `size_t`,\nwhile the existing code in `find_subpos()` uses the `unsigned long` data\ntype that the 1980/1980 so desperately want back.\n\nUnsurprisingly, this causes problems with the Windows build, where\n`sizeof(unsigned long) == 4` even in 64-bit builds (which, I feel the\nneed to add, is totally legitimate).\n\nWe should probably change the data type to the correct one in a\npreparatory patch, before improving the interface for parsing tags.\n\nSigned-off-by: Johannes Schindelin \n---\n ref-filter.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 164ab62d15e..bab34b9ef74 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1156,10 +1156,10 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n }\n\n static void find_subpos(const char *buf,\n-\t\t\tconst char **sub, unsigned long *sublen,\n-\t\t\tconst char **body, unsigned long *bodylen,\n-\t\t\tunsigned long *nonsiglen,\n-\t\t\tconst char **sig, unsigned long *siglen)\n+\t\t\tconst char **sub, size_t *sublen,\n+\t\t\tconst char **body, size_t *bodylen,\n+\t\t\tsize_t *nonsiglen,\n+\t\t\tconst char **sig, size_t *siglen)\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n@@ -1234,7 +1234,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n-\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n+\tsize_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n--\n2.25.1.windows.1\n\n"},{"id":"392477","messageId":"xmqqh7zemptx.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"nycvar.QRO.7.76.6.2002251127320.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-25T19:25:46Z","receivedAt":"2020-02-25T19:25:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> \t/* parse signature first; we might not even have a subject line */\n>> -\t*sig = buf + parse_signature(buf, strlen(buf));\n>> -\t*siglen = strlen(*sig);\n>> +\tparse_signature(buf, end - buf, &payload, &signature);\n>> +\t*sig = strbuf_detach(&signature, siglen);\n>\n> While I like the spirit of this patch, it makes the Windows build fail. I\n> put this on top of Git for Windows' `shears/pu` branch to fix it (maybe\n> you could adopt a variation of it?):\n\nFWIW, it's of course not just Windows, but on anything whose size_t\n!= ulong, e.g. https://travis-ci.org/git/git/jobs/654661788 (Linux32)\n"},{"id":"392502","messageId":"20200226022332.GB7911@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"nycvar.QRO.7.76.6.2002251127320.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-26T02:23:32Z","receivedAt":"2020-02-26T02:23:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-02-25 at 10:29:26, Johannes Schindelin wrote:\n> Hi brian,\n> \n> On Sat, 22 Feb 2020, brian m. carlson wrote:\n> \n> > diff --git a/ref-filter.c b/ref-filter.c\n> > index 6867e33648..212f1165bb 100644\n> > --- a/ref-filter.c\n> > +++ b/ref-filter.c\n> > @@ -1161,7 +1161,13 @@ static void find_subpos(const char *buf,\n> > \t\t\tunsigned long *nonsiglen,\n> > \t\t\tconst char **sig, unsigned long *siglen)\n> > {\n> > +\tstruct strbuf payload = STRBUF_INIT;\n> > +\tstruct strbuf signature = STRBUF_INIT;\n> > \tconst char *eol;\n> > +\tconst char *end = buf + strlen(buf);\n> > +\tconst char *sigstart;\n> > +\n> > +\n> > \t/* skip past header until we hit empty line */\n> > \twhile (*buf && *buf != '\\n') {\n> > \t\teol = strchrnul(buf, '\\n');\n> > @@ -1174,13 +1180,14 @@ static void find_subpos(const char *buf,\n> > \t\tbuf++;\n> >\n> > \t/* parse signature first; we might not even have a subject line */\n> > -\t*sig = buf + parse_signature(buf, strlen(buf));\n> > -\t*siglen = strlen(*sig);\n> > +\tparse_signature(buf, end - buf, &payload, &signature);\n> > +\t*sig = strbuf_detach(&signature, siglen);\n> \n> While I like the spirit of this patch, it makes the Windows build fail. I\n> put this on top of Git for Windows' `shears/pu` branch to fix it (maybe\n> you could adopt a variation of it?):\n\nI'm happy to squash this in. Sorry for the breakage, and thanks for\ncatching this.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"392505","messageId":"20200226030545.GD7911@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"xmqqh7zemptx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-02-26T03:05:45Z","receivedAt":"2020-02-26T03:05:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-02-25 at 19:25:46, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n> \n> >> \t/* parse signature first; we might not even have a subject line */\n> >> -\t*sig = buf + parse_signature(buf, strlen(buf));\n> >> -\t*siglen = strlen(*sig);\n> >> +\tparse_signature(buf, end - buf, &payload, &signature);\n> >> +\t*sig = strbuf_detach(&signature, siglen);\n> >\n> > While I like the spirit of this patch, it makes the Windows build fail. I\n> > put this on top of Git for Windows' `shears/pu` branch to fix it (maybe\n> > you could adopt a variation of it?):\n> \n> FWIW, it's of course not just Windows, but on anything whose size_t\n> != ulong, e.g. https://travis-ci.org/git/git/jobs/654661788 (Linux32)\n\nI will likely not get to a reroll before Thursday at the earliest due to\nother commitments, so if it's more convenient for you to eject this from\npu and just pick up v3 when I get around to it, please feel free.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"392506","messageId":"xmqq4kvekpow.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"20200226030545.GD7911@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-26T03:11:43Z","receivedAt":"2020-02-26T03:11:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I will likely not get to a reroll before Thursday at the earliest due to\n> other commitments, so if it's more convenient for you to eject this from\n> pu and just pick up v3 when I get around to it, please feel free.\n\nThanks, but I think a minimal SQUASH??? I added while queuing to\n'pu' seems to have made it pass so that would have to do for now.\n"},{"id":"392613","messageId":"nycvar.QRO.7.76.6.2002271423580.46@tvgsbejvaqbjf.bet","threadId":"52869","inReplyTo":"20200226022332.GB7911@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-02-27T13:24:56Z","receivedAt":"2020-02-27T13:25:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\nOn Wed, 26 Feb 2020, brian m. carlson wrote:\n\n> On 2020-02-25 at 10:29:26, Johannes Schindelin wrote:\n> >\n> > On Sat, 22 Feb 2020, brian m. carlson wrote:\n> >\n> > > diff --git a/ref-filter.c b/ref-filter.c\n> > > index 6867e33648..212f1165bb 100644\n> > > --- a/ref-filter.c\n> > > +++ b/ref-filter.c\n> > > @@ -1161,7 +1161,13 @@ static void find_subpos(const char *buf,\n> > > \t\t\tunsigned long *nonsiglen,\n> > > \t\t\tconst char **sig, unsigned long *siglen)\n> > > {\n> > > +\tstruct strbuf payload = STRBUF_INIT;\n> > > +\tstruct strbuf signature = STRBUF_INIT;\n> > > \tconst char *eol;\n> > > +\tconst char *end = buf + strlen(buf);\n> > > +\tconst char *sigstart;\n> > > +\n> > > +\n> > > \t/* skip past header until we hit empty line */\n> > > \twhile (*buf && *buf != '\\n') {\n> > > \t\teol = strchrnul(buf, '\\n');\n> > > @@ -1174,13 +1180,14 @@ static void find_subpos(const char *buf,\n> > > \t\tbuf++;\n> > >\n> > > \t/* parse signature first; we might not even have a subject line */\n> > > -\t*sig = buf + parse_signature(buf, strlen(buf));\n> > > -\t*siglen = strlen(*sig);\n> > > +\tparse_signature(buf, end - buf, &payload, &signature);\n> > > +\t*sig = strbuf_detach(&signature, siglen);\n> >\n> > While I like the spirit of this patch, it makes the Windows build fail. I\n> > put this on top of Git for Windows' `shears/pu` branch to fix it (maybe\n> > you could adopt a variation of it?):\n>\n> I'm happy to squash this in. Sorry for the breakage, and thanks for\n> catching this.\n\nYou're welcome, but credit for catching it should go to Azure Pipelines\n;-)\n\nTo be honest, I am rather happy how these CI builds help us catch things\nalready when they are in `pu`. _Quite_ happy.\n\nCiao,\nDscho\n"},{"id":"392615","messageId":"xmqqh7zcgjcf.fsf@gitster-ct.c.googlers.com","threadId":"52869","inReplyTo":"nycvar.QRO.7.76.6.2002271423580.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-27T15:06:56Z","receivedAt":"2020-02-27T15:07:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> You're welcome, but credit for catching it should go to Azure Pipelines\n> ;-)\n>\n> To be honest, I am rather happy how these CI builds help us catch things\n> already when they are in `pu`. _Quite_ happy.\n\nOh, you do not have to be honest for that---you show that all the\ntime ;-)\n\nAnd I am happy that I can throw a new topic to 'pu' and CI will\ncatch issues before I even look at it seriously to consider if it is\nworth keeping it.\n\nOne sad thing about Azure Pipelines thing is that, as far as I\nrecall, we hear about breakages it discovered only from you and\nnobody else, while we hear about breakages discovered by Travis by\nat least multiple people, and that is *not* because the coverage is\nwider or report is more accesible and easier to read there. It's\nthat our CI at Travis forced awareness of its existence on us by\nsimply being the only one out there for a long time.\n\nWe are bound to add issues to the tip of 'pu' (e.g. 64-vs-32,\nbytesex, or GNUisms) that are easy to discover by building on\ndifferent platforms and in different configurations every once in a\nwhile. We should advertise more to gain awareness so that other\npeople can also notice breakages Travis's limited coverage may have\nmissed even when you are on vacation.\n\nIf you want quantitative measure of success ;-) (sorry, I couldn't\nresist as it has been Perf period around here), I want to see at\nleast two people outside Microsoft to notice and report breakages\nfirst found by Azure Pipelines CI in the coming 6 months. How can\nwe help make that happen?\n\nThanks.\n\n\n"},{"id":"399185","messageId":"xmqqimg5o5fq.fsf@gitster.c.googlers.com","threadId":"52869","inReplyTo":"20200222201749.937983-21-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 20/24] fast-import: permit reading multiple marks files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-05T16:14:49Z","receivedAt":"2020-06-05T16:15:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> In the future, we'll want to read marks files for submodules as well.\n> Refactor the existing code to make it possible to read multiple marks\n> files, each into their own marks set.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> fast-import.c | 32 ++++++++++++++++++--------------\n> 1 file changed, 18 insertions(+), 14 deletions(-)\n\nThis conversion looks a bit strange. We are working off of 'marks'\nthat is file scope global, and the topmost caller passes the pointer\nto that single instance down the callchain to this function (among\nothers) in \"s\", and it all looked no-op to my eye, but this actually\nupdates the global 'marks'.\n\n> diff --git a/fast-import.c b/fast-import.c\n> index b8b65a801c..b9ecd89699 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -493,25 +493,24 @@ static char *pool_strdup(const char *s)\n> \treturn r;\n> }\n> \n> -static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n> +static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry *oe)\n> {\n> -\tstruct mark_set *s = marks;\n\nThis is no longer needed as the caller gave the 'marks' to us in\n's'. Up to this line, this step is a no-op patch. \n\n> \twhile ((idnum >> s->shift) >= 1024) {\n> \t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n\nHere we overwrite 's', and we lost the pointer the caller gave us!!!\n\n> \t\ts->shift = marks->shift + 10;\n\nAnd we still work off of global marks here and\n\n> \t\ts->data.sets[0] = marks;\n> \t\tmarks = s;\n\ntry to swap it here. But if our caller is also using 's' that was\npassed from its caller, this assignment would not be seen by our\ncaller. We'd need to pass a pointer to caller's 's' to this\nfunction and update it around here.\n\n> \t}\n> \twhile (s->shift) {\n> \t\tuintmax_t i = idnum >> s->shift;\n> \t\tidnum -= i << s->shift;\n> \t\tif (!s->data.sets[i]) {\n> \t\t\ts->data.sets[i] = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n> \t\t\ts->data.sets[i]->shift = s->shift - 10;\n> \t\t}\n> \t\ts = s->data.sets[i];\n> \t}\n> \tif (!s->data.marked[idnum])\n> \t\tmarks_set_count++;\n\nI am not sure if this global also needs a separate counter and I\ndidn't bother to check.\n\n> \ts->data.marked[idnum] = oe;\n> }\n\nWith this patch t9300 still seems to pass.\n\nI do not know if this is the fix for Billes Tibor's \"fast-import\noom\" issue but the\nchange to this function stood out.\n\nThanks.\n\n\n fast-import.c | 20 +++++++++++---------\n 1 file changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 0dfa14dc8c..f39b7890ac 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -534,13 +534,15 @@ static char *pool_strdup(const char *s)\n \treturn r;\n }\n \n-static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry *oe)\n+static void insert_mark(struct mark_set **sp, uintmax_t idnum, struct object_entry *oe)\n {\n+\tstruct mark_set *s = *sp;\n+\n \twhile ((idnum >> s->shift) >= 1024) {\n \t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n-\t\ts->shift = marks->shift + 10;\n-\t\ts->data.sets[0] = marks;\n-\t\tmarks = s;\n+\t\ts->shift = (*sp)->shift + 10;\n+\t\ts->data.sets[0] = (*sp);\n+\t\t(*sp) = s;\n \t}\n \twhile (s->shift) {\n \t\tuintmax_t i = idnum >> s->shift;\n@@ -958,7 +960,7 @@ static int store_object(\n \n \te = insert_object(&oid);\n \tif (mark)\n-\t\tinsert_mark(marks, mark, e);\n+\t\tinsert_mark(&marks, mark, e);\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n@@ -1156,7 +1158,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \te = insert_object(&oid);\n \n \tif (mark)\n-\t\tinsert_mark(marks, mark, e);\n+\t\tinsert_mark(&marks, mark, e);\n \n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n@@ -1745,12 +1747,12 @@ static void insert_object_entry(struct mark_set *s, struct object_id *oid, uintm\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\n \t}\n-\tinsert_mark(s, mark, e);\n+\tinsert_mark(&s, mark, e);\n }\n \n static void insert_oid_entry(struct mark_set *s, struct object_id *oid, uintmax_t mark)\n {\n-\tinsert_mark(s, mark, xmemdupz(oid, sizeof(*oid)));\n+\tinsert_mark(&s, mark, xmemdupz(oid, sizeof(*oid)));\n }\n \n static void read_mark_file(struct mark_set *s, FILE *f, mark_set_inserter_t inserter)\n@@ -3242,7 +3244,7 @@ static void parse_alias(void)\n \t\tdie(_(\"Expected 'to' command, got %s\"), command_buf.buf);\n \te = find_object(&b.oid);\n \tassert(e);\n-\tinsert_mark(marks, next_mark, e);\n+\tinsert_mark(&marks, next_mark, e);\n }\n \n static char* make_fast_import_path(const char *path)\n"},{"id":"399186","messageId":"xmqqeeqto4x1.fsf@gitster.c.googlers.com","threadId":"52869","inReplyTo":"xmqqimg5o5fq.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 20/24] fast-import: permit reading multiple marks files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-05T16:26:02Z","receivedAt":"2020-06-05T16:29:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> @@ -1745,12 +1747,12 @@ static void insert_object_entry(struct mark_set *s, struct object_id *oid, uintm\n> \t\te->pack_id = MAX_PACK_ID;\n> \t\te->idx.offset = 1; /* just not zero! */\n> \t}\n> -\tinsert_mark(s, mark, e);\n> +\tinsert_mark(&s, mark, e);\n> }\n> \n> static void insert_oid_entry(struct mark_set *s, struct object_id *oid, uintmax_t mark)\n> {\n> -\tinsert_mark(s, mark, xmemdupz(oid, sizeof(*oid)));\n> +\tinsert_mark(&s, mark, xmemdupz(oid, sizeof(*oid)));\n> }\n> \n\nActually the above \"fix\" still has the same issue; we may have made\nthe on-stack copy of 's' that are parameters these two functions see\nas callees, but the caller of these two functions would not see the\neffect of the munging done deep in the callchain by insert_mark().\n\nThe read_marks() function calls read_mark_file() with \"marks\", but I\nthink you'd need to pass pointer to that global singleton and teach\nmark_set_inserter_t function to take a pointer to a pointer to\nstruct mark_set throughout, so that the instance the top-level\ncaller (read_marks in this callchain) gets updated.\n\nHere is another try.\n\n fast-import.c | 28 +++++++++++++++-------------\n 1 file changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 0dfa14dc8c..ed87d6e380 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -150,7 +150,7 @@ struct recent_command {\n \tchar *buf;\n };\n \n-typedef void (*mark_set_inserter_t)(struct mark_set *s, struct object_id *oid, uintmax_t mark);\n+typedef void (*mark_set_inserter_t)(struct mark_set **s, struct object_id *oid, uintmax_t mark);\n typedef void (*each_mark_fn_t)(uintmax_t mark, void *obj, void *cbp);\n \n /* Configured limits on output */\n@@ -534,13 +534,15 @@ static char *pool_strdup(const char *s)\n \treturn r;\n }\n \n-static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry *oe)\n+static void insert_mark(struct mark_set **sp, uintmax_t idnum, struct object_entry *oe)\n {\n+\tstruct mark_set *s = *sp;\n+\n \twhile ((idnum >> s->shift) >= 1024) {\n \t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n-\t\ts->shift = marks->shift + 10;\n-\t\ts->data.sets[0] = marks;\n-\t\tmarks = s;\n+\t\ts->shift = (*sp)->shift + 10;\n+\t\ts->data.sets[0] = (*sp);\n+\t\t(*sp) = s;\n \t}\n \twhile (s->shift) {\n \t\tuintmax_t i = idnum >> s->shift;\n@@ -958,7 +960,7 @@ static int store_object(\n \n \te = insert_object(&oid);\n \tif (mark)\n-\t\tinsert_mark(marks, mark, e);\n+\t\tinsert_mark(&marks, mark, e);\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n@@ -1156,7 +1158,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \te = insert_object(&oid);\n \n \tif (mark)\n-\t\tinsert_mark(marks, mark, e);\n+\t\tinsert_mark(&marks, mark, e);\n \n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n@@ -1731,7 +1733,7 @@ static void dump_marks(void)\n \t}\n }\n \n-static void insert_object_entry(struct mark_set *s, struct object_id *oid, uintmax_t mark)\n+static void insert_object_entry(struct mark_set **s, struct object_id *oid, uintmax_t mark)\n {\n \tstruct object_entry *e;\n \te = find_object(oid);\n@@ -1748,12 +1750,12 @@ static void insert_object_entry(struct mark_set *s, struct object_id *oid, uintm\n \tinsert_mark(s, mark, e);\n }\n \n-static void insert_oid_entry(struct mark_set *s, struct object_id *oid, uintmax_t mark)\n+static void insert_oid_entry(struct mark_set **s, struct object_id *oid, uintmax_t mark)\n {\n \tinsert_mark(s, mark, xmemdupz(oid, sizeof(*oid)));\n }\n \n-static void read_mark_file(struct mark_set *s, FILE *f, mark_set_inserter_t inserter)\n+static void read_mark_file(struct mark_set **s, FILE *f, mark_set_inserter_t inserter)\n {\n \tchar line[512];\n \twhile (fgets(line, sizeof(line), f)) {\n@@ -1786,7 +1788,7 @@ static void read_marks(void)\n \t\tgoto done; /* Marks file does not exist */\n \telse\n \t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n-\tread_mark_file(marks, f, insert_object_entry);\n+\tread_mark_file(&marks, f, insert_object_entry);\n \tfclose(f);\n done:\n \timport_marks_file_done = 1;\n@@ -3242,7 +3244,7 @@ static void parse_alias(void)\n \t\tdie(_(\"Expected 'to' command, got %s\"), command_buf.buf);\n \te = find_object(&b.oid);\n \tassert(e);\n-\tinsert_mark(marks, next_mark, e);\n+\tinsert_mark(&marks, next_mark, e);\n }\n \n static char* make_fast_import_path(const char *path)\n@@ -3340,7 +3342,7 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list)\n \tfp = fopen(f, \"r\");\n \tif (!fp)\n \t\tdie_errno(\"cannot read '%s'\", f);\n-\tread_mark_file(ms, fp, insert_oid_entry);\n+\tread_mark_file(&ms, fp, insert_oid_entry);\n \tfclose(fp);\n }\n \n"},{"id":"399216","messageId":"20200605223910.GE6569@camp.crustytoothpaste.net","threadId":"52869","inReplyTo":"xmqqeeqto4x1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 20/24] fast-import: permit reading multiple marks files","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-05T22:39:10Z","receivedAt":"2020-06-05T22:39:54Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-05 at 16:26:02, Junio C Hamano wrote:\n> diff --git a/fast-import.c b/fast-import.c\n> index 0dfa14dc8c..ed87d6e380 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -150,7 +150,7 @@ struct recent_command {\n> \tchar *buf;\n> };\n> \n> -typedef void (*mark_set_inserter_t)(struct mark_set *s, struct object_id *oid, uintmax_t mark);\n> +typedef void (*mark_set_inserter_t)(struct mark_set **s, struct object_id *oid, uintmax_t mark);\n> typedef void (*each_mark_fn_t)(uintmax_t mark, void *obj, void *cbp);\n> \n> /* Configured limits on output */\n> @@ -534,13 +534,15 @@ static char *pool_strdup(const char *s)\n> \treturn r;\n> }\n> \n> -static void insert_mark(struct mark_set *s, uintmax_t idnum, struct object_entry *oe)\n> +static void insert_mark(struct mark_set **sp, uintmax_t idnum, struct object_entry *oe)\n> {\n> +\tstruct mark_set *s = *sp;\n> +\n> \twhile ((idnum >> s->shift) >= 1024) {\n> \t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n> -\t\ts->shift = marks->shift + 10;\n> -\t\ts->data.sets[0] = marks;\n> -\t\tmarks = s;\n> +\t\ts->shift = (*sp)->shift + 10;\n> +\t\ts->data.sets[0] = (*sp);\n> +\t\t(*sp) = s;\n> \t}\n\nYeah, this is much better. I'm not sure how I missed converting that\nblock, but it definitely leaks heavily, and obviously we don't want to\nupdate the actual marks variable in every case, since that defeats the\npurpose of the patch.\n\nWe don't actually hit this case in any of the tests because we don't\nhave any tests that have 1024 marks in them. I'm having trouble coming\nup with a test even with a large number of marks because I don't think\nwe produce different behavior here; we just leak a lot of memory.\n\nThis does fix the reported issue, as I suspected.\n\nDo you want to write this up into a patch with a commit message, or\nshould I? If the latter, may I have your sign-off for it?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"399218","messageId":"xmqqlfl1m8e9.fsf@gitster.c.googlers.com","threadId":"52869","inReplyTo":"20200605223910.GE6569@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 20/24] fast-import: permit reading multiple marks files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-05T22:53:50Z","receivedAt":"2020-06-05T22:53:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> We don't actually hit this case in any of the tests because we don't\n> have any tests that have 1024 marks in them. I'm having trouble coming\n> up with a test even with a large number of marks because I don't think\n> we produce different behavior here; we just leak a lot of memory.\n>\n> This does fix the reported issue, as I suspected.\n>\n> Do you want to write this up into a patch with a commit message, or\n> should I? If the latter, may I have your sign-off for it?\n\nSure, anything I write for this project you can assume to be signed\noff by me. As I was just \"fixing\" what I thought was a botched\nconversion, without really figuring out where it actually leads to\nleaks, I'd need further work if I were to wrap it up with a sensible\nlog message, so thanks for volunteering ;-)\n\nSigned-off-by: Junio C Hamano \n"}]}