| Message ID | 20200222201749.937983-1-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | SHA-256 stage 4 implementation, part 1/3 | expand |
"brian m. carlson" <sandals@crustytoothpaste.net> writes: > This is a series for part 1 of 3 of a stage 4 SHA-256 implementation. > It is mostly the same as v1[0], which was RFC. The interested reader is > referred there for the in-depth explanations. > > A few interesting changes have taken place since v1. First, I > discovered a regression in a recent series which introduced a hard-coded > constant, so patch 1 addresses this. > > Second, I discovered a mistaken assumption that we were making about our > hash implementations: that copying the struct was sufficient to copy the > context. This is not true for libgcrypt, where our context is a pointer > instead, so patch 2 addresses this with a helper function. > > Finally, I've added a check to prevent non-developers from creating > SHA-256 repositories, since this series is not sufficient to implement > full SHA-256 support. Even as a developer, creating a SHA-256 > repository immediately leads to a broken state, since we don't recognize > the extension (yet) and therefore promptly refuse to operate on it in > any way. Preventing this experience seemed prudent. I am very tempted to take 1/24 separately and queue it at the tip of the jk/packfile-reuse-cleanup topic. I didn't read the fast-import bits at the end of the series, but everything before those steps made sense to me. Thanks.
This is a series for part 1 of 3 of a stage 4 SHA-256 implementation. It is mostly the same as v1[0], which was RFC. The interested reader is referred there for the in-depth explanations. A few interesting changes have taken place since v1. First, I discovered a regression in a recent series which introduced a hard-coded constant, so patch 1 addresses this. Second, I discovered a mistaken assumption that we were making about our hash implementations: that copying the struct was sufficient to copy the context. This is not true for libgcrypt, where our context is a pointer instead, so patch 2 addresses this with a helper function. Finally, I've added a check to prevent non-developers from creating SHA-256 repositories, since this series is not sufficient to implement full SHA-256 support. Even as a developer, creating a SHA-256 repository immediately leads to a broken state, since we don't recognize the extension (yet) and therefore promptly refuse to operate on it in any way. Preventing this experience seemed prudent. There are a few other minor changes indicated in the range-diff below. If folks have opinions about things like option names (or really, anything else), I'd love to hear them. There are two more parts for this implementation and one more set of test fixes. They will be coming in due course. [0] https://lore.kernel.org/git/20200113124729.3684846-1-sandals@crustytoothpaste.net/ brian m. carlson (24): builtin/pack-objects: make hash agnostic hash: implement and use a context cloning function hex: introduce parsing variants taking hash algorithms hex: add functions to parse hex object IDs in any algorithm repository: require a build flag to use SHA-256 t: use hash-specific lookup tables to define test constants t6300: abstract away SHA-1-specific constants t6300: make hash algorithm independent t/helper/test-dump-split-index: initialize git repository t/helper: initialize repository if necessary t/helper: make repository tests hash independent setup: allow check_repository_format to read repository format builtin/init-db: allow specifying hash algorithm on command line builtin/init-db: add environment variable for new repo hash init-db: move writing repo version into a function worktree: allow repository version 1 commit: use expected signature header for SHA-256 gpg-interface: improve interface for parsing tags tag: store SHA-256 signatures in a header fast-import: permit reading multiple marks files fast-import: add helper function for inserting mark object entries fast-import: make find_marks work on any mark set fast-import: add a generic function to iterate over marks fast-import: add options for rewriting submodules Documentation/git-fast-import.txt | 20 +++ Documentation/git-init.txt | 7 +- Documentation/git.txt | 6 + builtin/clone.c | 2 +- builtin/commit.c | 2 +- builtin/fmt-merge-msg.c | 26 +++- builtin/init-db.c | 75 +++++++-- builtin/mktag.c | 14 ++ builtin/pack-objects.c | 2 +- builtin/receive-pack.c | 4 +- builtin/tag.c | 20 ++- cache.h | 25 ++- commit.c | 58 +++++-- commit.h | 8 + config.mak.dev | 2 + csum-file.c | 2 +- fast-import.c | 246 ++++++++++++++++++++++-------- gpg-interface.c | 17 ++- gpg-interface.h | 9 +- hash.h | 21 +++ hex.c | 57 ++++++- log-tree.c | 14 +- path.c | 2 +- ref-filter.c | 23 ++- repository.c | 4 + sequencer.c | 2 +- setup.c | 6 +- sha1-file.c | 18 +++ sha256/gcrypt.h | 6 + t/helper/test-dump-split-index.c | 2 + t/helper/test-repository.c | 14 +- t/t1450-fsck.sh | 24 +++ t/t5801-remote-helpers.sh | 4 +- t/t6300-for-each-ref.sh | 61 +++++--- t/t7004-tag.sh | 8 +- t/t7030-verify-tag.sh | 17 +++ t/t7510-signed-commit.sh | 16 +- t/t9300-fast-import.sh | 109 +++++++++++++ t/test-lib.sh | 29 ++-- tag.c | 15 +- worktree.c | 10 +- 41 files changed, 810 insertions(+), 197 deletions(-) Range-diff against v1: -: ---------- > 1: 0037d0f950 builtin/pack-objects: make hash agnostic -: ---------- > 2: e6f632eb38 hash: implement and use a context cloning function 1: 17e86d7e8a = 3: 205c67144b hex: introduce parsing variants taking hash algorithms 2: 8a4e05f03f = 4: 09e3f4ca8e hex: add functions to parse hex object IDs in any algorithm 3: 4efa071dd6 = 5: 3df541b2f0 repository: require a build flag to use SHA-256 4: 0c26727e8c = 6: 1161b70d24 t: use hash-specific lookup tables to define test constants 5: 3aff50578d = 7: fd3a85f59f t6300: abstract away SHA-1-specific constants 6: 2736da7f60 = 8: fb93b0900b t6300: make hash algorithm independent 7: 343cd6c9b2 = 9: 1d873fc05a t/helper/test-dump-split-index: initialize git repository 8: 1c8e31fd41 = 10: 8e918b28d5 t/helper: initialize repository if necessary 9: 9dade56ee5 = 11: 7ae0d820eb t/helper: make repository tests hash independent 10: b08bec90e7 = 12: f13b1465cf setup: allow check_repository_format to read repository format 11: acfdc2b0c8 ! 13: 273a2c06ce builtin/init-db: allow specifying hash algorithm on command line @@ Commit message algorithm. Ensure that if we are writing a non-SHA-1 repository that we set the repository version to 1 and write the objectFormat extension. + Restrict this option to work only when ENABLE_SHA256 is set until the + codebase is in a situation to fully support this. + Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> ## Documentation/git-init.txt ## @@ builtin/init-db.c: static int create_default_files(const char *template_path, exit(1); } ++#ifndef ENABLE_SHA256 ++ if (fmt->hash_algo != GIT_HASH_SHA1) ++ die(_("The hash algorithm %s is not supported in this build."), hash_algos[fmt->hash_algo].name); ++#endif ++ + if (fmt->hash_algo != GIT_HASH_SHA1) + repo_version = GIT_REPO_VERSION_READ; + 12: adbd01c070 = 14: 6f3c736259 builtin/init-db: add environment variable for new repo hash 13: 21cdc16e15 ! 15: 3966b560b5 init-db: move writing repo version into a function @@ builtin/init-db.c: static int needs_work_tree_config(const char *git_dir, const + char repo_version_string[10]; + int repo_version = GIT_REPO_VERSION; + ++#ifndef ENABLE_SHA256 ++ if (hash_algo != GIT_HASH_SHA1) ++ die(_("The hash algorithm %s is not supported in this build."), hash_algos[hash_algo].name); ++#endif ++ + if (hash_algo != GIT_HASH_SHA1) + repo_version = GIT_REPO_VERSION_READ; + @@ builtin/init-db.c: static int create_default_files(const char *template_path, exit(1); } +-#ifndef ENABLE_SHA256 +- if (fmt->hash_algo != GIT_HASH_SHA1) +- die(_("The hash algorithm %s is not supported in this build."), hash_algos[fmt->hash_algo].name); +-#endif +- - if (fmt->hash_algo != GIT_HASH_SHA1) - repo_version = GIT_REPO_VERSION_READ; - 14: 45423efc1c = 16: 5af56f4f6a worktree: allow repository version 1 15: 7b99132acd ! 17: fb51a683f1 commit: use expected signature header for SHA-256 @@ commit.c: int remove_signature(struct strbuf *buf) ## sequencer.c ## @@ sequencer.c: static int try_to_commit(struct repository *r, - if (parse_head(r, ¤t_head)) return -1; + if (flags & AMEND_MSG) { - const char *exclude_gpgsig[] = { "gpgsig", NULL }; + const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL }; 16: b4eecdbc28 ! 18: 3afac82c18 gpg-interface: improve interface for parsing tags @@ log-tree.c: static int show_one_mergetag(struct commit *commit, + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; - hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid); - tag = lookup_tag(the_repository, &oid); + hash_object_file(the_hash_algo, extra->value, extra->len, + type_name(OBJ_TAG), &oid); @@ log-tree.c: static int show_one_mergetag(struct commit *commit, "parent #%d, tagged '%s'\n", nth + 1, tag->tag); gpg_message_offset = verify_message.len; 17: 5d9c8753b1 = 19: 5e6d1ccbb2 tag: store SHA-256 signatures in a header 18: 4a279f679d = 20: fb15c66ba1 fast-import: permit reading multiple marks files 19: 01ef63b04d = 21: 273468b0ac fast-import: add helper function for inserting mark object entries 20: 2469dc1324 = 22: bba3cf497d fast-import: make find_marks work on any mark set 21: 0ad1e4d3a8 = 23: 714ae09a8c fast-import: add a generic function to iterate over marks 22: bc53f8aaec = 24: 5c50871f30 fast-import: add options for rewriting submodules