{"thread":{"id":"46635","subject":"[RFC PATCH 1/6] vcs-svn: remove unused prototypes","startedAt":"2017-08-21T00:00:35Z","lastAt":"2017-08-21T21:16:35Z","messageCount":11,"participants":["brian m. carlson","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"326841","messageId":"20170821000022.26729-2-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 1/6] vcs-svn: remove unused prototypes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:17Z","receivedAt":"2017-08-21T00:00:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The Subversion code had prototypes for several functions which were not\never defined or used.  These functions all had names starting with\n\"repo_\", some of which conflict with those in repository.h.  To avoid\nthe conflict, remove those unused prototypes.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n vcs-svn/repo_tree.h | 7 -------\n 1 file changed, 7 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h\nindex 889c6a3c95..d0f5690dca 100644\n--- a/vcs-svn/repo_tree.h\n+++ b/vcs-svn/repo_tree.h\n@@ -10,14 +10,7 @@ struct strbuf;\n \n uint32_t next_blob_mark(void);\n void repo_copy(uint32_t revision, const char *src, const char *dst);\n-void repo_add(const char *path, uint32_t mode, uint32_t blob_mark);\n const char *repo_read_path(const char *path, uint32_t *mode_out);\n void repo_delete(const char *path);\n-void repo_commit(uint32_t revision, const char *author,\n-\t\tconst struct strbuf *log, const char *uuid, const char *url,\n-\t\tlong unsigned timestamp);\n-void repo_diff(uint32_t r1, uint32_t r2);\n-void repo_init(void);\n-void repo_reset(void);\n \n #endif\n"},{"id":"326842","messageId":"20170821000022.26729-7-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 6/6] Switch empty tree and blob lookups to use hash abstraction","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:22Z","receivedAt":"2017-08-21T00:00:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Switch the uses of empty_tree_oid and empty_blob_oid to use the\ncurrent_hash abstraction that represents the current hash algorithm in\nuse.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n builtin/am.c       | 2 +-\n builtin/checkout.c | 2 +-\n builtin/diff.c     | 2 +-\n builtin/pull.c     | 2 +-\n cache.h            | 8 ++++----\n diff-lib.c         | 2 +-\n merge-recursive.c  | 2 +-\n notes-merge.c      | 2 +-\n sequencer.c        | 6 +++---\n submodule.c        | 2 +-\n 10 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 40cc6d6fe8..347b30bd1f 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1435,7 +1435,7 @@ static void write_index_patch(const struct am_state *state)\n \tif (!get_oid_tree(\"HEAD\", &head))\n \t\ttree = lookup_tree(&head);\n \telse\n-\t\ttree = lookup_tree(&empty_tree_oid);\n+\t\ttree = lookup_tree(current_hash->empty_tree);\n \n \tfp = xfopen(am_path(state, \"patch\"), \"w\");\n \tinit_revisions(&rev_info, NULL);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2d75ac66c7..efdec232b7 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -515,7 +515,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\t}\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t   &old->commit->object.oid :\n-\t\t\t\t\t   &empty_tree_oid);\n+\t\t\t\t\t   current_hash->empty_tree);\n \t\tinit_tree_desc(&trees[0], tree->buffer, tree->size);\n \t\ttree = parse_tree_indirect(&new->commit->object.oid);\n \t\tinit_tree_desc(&trees[1], tree->buffer, tree->size);\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 7cde6abbcf..4cb5d23899 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -382,7 +382,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\t\tadd_head_to_pending(&rev);\n \t\t\t\tif (!rev.pending.nr) {\n \t\t\t\t\tstruct tree *tree;\n-\t\t\t\t\ttree = lookup_tree(&empty_tree_oid);\n+\t\t\t\t\ttree = lookup_tree(current_hash->empty_tree);\n \t\t\t\t\tadd_pending_object(&rev, &tree->object, \"HEAD\");\n \t\t\t\t}\n \t\t\t\tbreak;\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 9b86e519b1..4fa5db2fc9 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -537,7 +537,7 @@ static int pull_into_void(const struct object_id *merge_head,\n \t * index/worktree changes that the user already made on the unborn\n \t * branch.\n \t */\n-\tif (checkout_fast_forward(&empty_tree_oid, merge_head, 0))\n+\tif (checkout_fast_forward(current_hash->empty_tree, merge_head, 0))\n \t\treturn 1;\n \n \tif (update_ref(\"initial pull\", \"HEAD\", merge_head->hash, curr_head->hash, 0, UPDATE_REFS_DIE_ON_ERR))\ndiff --git a/cache.h b/cache.h\nindex d759824803..23d2e29fcd 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1051,22 +1051,22 @@ extern const struct object_id empty_blob_oid;\n \n static inline int is_empty_blob_sha1(const unsigned char *sha1)\n {\n-\treturn !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);\n+\treturn !hashcmp(sha1, current_hash->empty_blob->hash);\n }\n \n static inline int is_empty_blob_oid(const struct object_id *oid)\n {\n-\treturn !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);\n+\treturn !oidcmp(oid, current_hash->empty_blob);\n }\n \n static inline int is_empty_tree_sha1(const unsigned char *sha1)\n {\n-\treturn !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);\n+\treturn !hashcmp(sha1, current_hash->empty_tree->hash);\n }\n \n static inline int is_empty_tree_oid(const struct object_id *oid)\n {\n-\treturn !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);\n+\treturn !oidcmp(oid, current_hash->empty_tree);\n }\n \n /* set default permissions by passing mode arguments to open(2) */\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 2a52b07954..0575f4cb1b 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -216,7 +216,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t} else if (revs->diffopt.ita_invisible_in_index &&\n \t\t\t\t   ce_intent_to_add(ce)) {\n \t\t\t\tdiff_addremove(&revs->diffopt, '+', ce->ce_mode,\n-\t\t\t\t\t       &empty_tree_oid, 0,\n+\t\t\t\t\t       current_hash->empty_tree, 0,\n \t\t\t\t\t       ce->name, 0);\n \t\t\t\tcontinue;\n \t\t\t}\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 1494ffdb82..981fb668f9 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2043,7 +2043,7 @@ int merge_recursive(struct merge_options *o,\n \t\t/* if there is no common ancestor, use an empty tree */\n \t\tstruct tree *tree;\n \n-\t\ttree = lookup_tree(&empty_tree_oid);\n+\t\ttree = lookup_tree(current_hash->empty_tree);\n \t\tmerged_common_ancestors = make_virtual_commit(tree, \"ancestor\");\n \t}\n \ndiff --git a/notes-merge.c b/notes-merge.c\nindex c12b354f10..5b1e2ef8d6 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -597,7 +597,7 @@ int notes_merge(struct notes_merge_options *o,\n \tbases = get_merge_bases(local, remote);\n \tif (!bases) {\n \t\tbase_oid = &null_oid;\n-\t\tbase_tree_oid = &empty_tree_oid;\n+\t\tbase_tree_oid = current_hash->empty_tree;\n \t\tif (o->verbosity >= 4)\n \t\t\tprintf(\"No merge base found; doing history-less merge\\n\");\n \t} else if (!bases->next) {\ndiff --git a/sequencer.c b/sequencer.c\nindex f5e85a398d..86961a29cb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -345,7 +345,7 @@ static int read_oneliner(struct strbuf *buf,\n \n static struct tree *empty_tree(void)\n {\n-\treturn lookup_tree(&empty_tree_oid);\n+\treturn lookup_tree(current_hash->empty_tree);\n }\n \n static int error_dirty_index(struct replay_opts *opts)\n@@ -703,7 +703,7 @@ static int is_original_commit_empty(struct commit *commit)\n \t\t\t\toid_to_hex(&parent->object.oid));\n \t\tptree_oid = &parent->tree->object.oid;\n \t} else {\n-\t\tptree_oid = &empty_tree_oid; /* commit is root */\n+\t\tptree_oid = current_hash->empty_tree; /* commit is root */\n \t}\n \n \treturn !oidcmp(ptree_oid, &commit->tree->object.oid);\n@@ -956,7 +956,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t} else {\n \t\tunborn = get_oid(\"HEAD\", &head);\n \t\tif (unborn)\n-\t\t\toidcpy(&head, &empty_tree_oid);\n+\t\t\toidcpy(&head, current_hash->empty_tree);\n \t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0, 0))\n \t\t\treturn error_dirty_index(opts);\n \t}\ndiff --git a/submodule.c b/submodule.c\nindex 27de65a4a0..12ad6b1338 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -656,7 +656,7 @@ void show_submodule_inline_diff(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset,\n \t\tconst struct diff_options *o)\n {\n-\tconst struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;\n+\tconst struct object_id *old = current_hash->empty_tree, *new = current_hash->empty_tree;\n \tstruct commit *left = NULL, *right = NULL;\n \tstruct commit_list *merge_bases = NULL;\n \tstruct strbuf submodule_dir = STRBUF_INIT;\n"},{"id":"326843","messageId":"20170821000022.26729-6-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 5/6] Integrate hash algorithm support with repo setup","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:21Z","receivedAt":"2017-08-21T00:00:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In future versions of Git, we plan to support an additional hash\nalgorithm.  Integrate the enumeration of hash algorithms with repository\nsetup, and store a pointer to the enumerated data in struct repository.\nOf course, we currently only support SHA-1, so hard-code this value in\nread_repository_format.  In the future, we'll enumerate this value from\nthe configuration.\n\nAdd a constant, current_hash, which points to the hash_algo structure\npointer in the repository global.  Include repository.h in cache.h since\nwe now need to have access to these struct and variable definitions.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n cache.h      | 4 ++++\n repository.c | 7 +++++++\n repository.h | 5 +++++\n setup.c      | 2 ++\n 4 files changed, 18 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 375a7fb15e..d759824803 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -13,6 +13,7 @@\n #include \"hash.h\"\n #include \"path.h\"\n #include \"sha1-array.h\"\n+#include \"repository.h\"\n \n #ifndef platform_SHA_CTX\n /*\n@@ -112,6 +113,8 @@ struct git_hash_algo {\n };\n extern const struct git_hash_algo hash_algos[1];\n \n+#define current_hash the_repository->hash_algo\n+\n #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n #define DTYPE(de)\t((de)->d_type)\n #else\n@@ -894,6 +897,7 @@ struct repository_format {\n \tint version;\n \tint precious_objects;\n \tint is_bare;\n+\tint hash_algo;\n \tchar *work_tree;\n \tstruct string_list unknown_extensions;\n };\ndiff --git a/repository.c b/repository.c\nindex 1617467568..37764f627a 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -62,6 +62,11 @@ void repo_set_gitdir(struct repository *repo, const char *path)\n \trepo_setup_env(repo);\n }\n \n+void repo_set_hash_algo(struct repository *repo, int hash_algo)\n+{\n+\trepo->hash_algo = &hash_algos[hash_algo];\n+}\n+\n /*\n  * Attempt to resolve and set the provided 'gitdir' for repository 'repo'.\n  * Return 0 upon success and a non-zero value upon failure.\n@@ -134,6 +139,8 @@ int repo_init(struct repository *repo, const char *gitdir, const char *worktree)\n \tif (read_and_verify_repository_format(&format, repo->commondir))\n \t\tgoto error;\n \n+\trepo->hash_algo = &hash_algos[format.hash_algo];\n+\n \tif (worktree)\n \t\trepo_set_worktree(repo, worktree);\n \ndiff --git a/repository.h b/repository.h\nindex 417787f3ef..f171172150 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -4,6 +4,7 @@\n struct config_set;\n struct index_state;\n struct submodule_cache;\n+struct git_hash_algo;\n \n struct repository {\n \t/* Environment */\n@@ -67,6 +68,9 @@ struct repository {\n \t */\n \tstruct index_state *index;\n \n+\t/* Repository's current hash algorithm. */\n+\tconst struct git_hash_algo *hash_algo;\n+\n \t/* Configurations */\n \t/*\n \t * Bit used during initialization to indicate if repository state (like\n@@ -86,6 +90,7 @@ extern struct repository *the_repository;\n \n extern void repo_set_gitdir(struct repository *repo, const char *path);\n extern void repo_set_worktree(struct repository *repo, const char *path);\n+extern void repo_set_hash_algo(struct repository *repo, int algo);\n extern int repo_init(struct repository *repo, const char *gitdir, const char *worktree);\n extern int repo_submodule_init(struct repository *submodule,\n \t\t\t       struct repository *superproject,\ndiff --git a/setup.c b/setup.c\nindex 115e70a4e8..289e24811c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -491,6 +491,7 @@ int read_repository_format(struct repository_format *format, const char *path)\n \tmemset(format, 0, sizeof(*format));\n \tformat->version = -1;\n \tformat->is_bare = -1;\n+\tformat->hash_algo = GIT_HASH_SHA1;\n \tstring_list_init(&format->unknown_extensions, 1);\n \tgit_config_from_file(check_repo_format, path, format);\n \treturn format->version;\n@@ -1125,6 +1126,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t\trepo_set_gitdir(the_repository, gitdir);\n \t\t\tsetup_git_env();\n \t\t}\n+\t\trepo_set_hash_algo(the_repository, repo_fmt.hash_algo);\n \t}\n \n \tstrbuf_release(&dir);\n"},{"id":"326844","messageId":"20170821000022.26729-5-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 4/6] Add structure representing hash algorithm","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:20Z","receivedAt":"2017-08-21T00:00:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Since in the future we want to support an additional hash algorithm, add\na structure that represents a hash algorithm and all the data that must\ngo along with it.  Add a constant to allow easy enumeration of hash\nalgorithms.  Implement function typedefs to create an abstract API that\ncan be used by any hash algorithm, and wrappers for the existing SHA1\nfunctions that conform to this API.\n\nDon't include an entry in the hash algorithm structure for the null\nobject ID.  As this value is all zeros, any suitably sized all-zero\nobject ID can be used, and there's no need to store a given one on a\nper-hash basis.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n cache.h     | 36 ++++++++++++++++++++++++++++++++++++\n sha1_file.c | 29 +++++++++++++++++++++++++++++\n 2 files changed, 65 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 1c69d2a05a..375a7fb15e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -76,6 +76,42 @@ struct object_id {\n \tunsigned char hash[GIT_MAX_RAWSZ];\n };\n \n+#define GIT_HASH_SHA1 0\n+\n+typedef void (*git_hash_init_fn)(void *ctx);\n+typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);\n+typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);\n+\n+struct git_hash_algo {\n+\t/* The name of the algorithm, as appears in the config file. */\n+\tconst char *name;\n+\n+\t/* The size of a hash context (e.g. git_SHA_CTX). */\n+\tsize_t ctxsz;\n+\n+\t/* The length of the hash in bytes. */\n+\tsize_t rawsz;\n+\n+\t/* The length of the hash in hex characters. */\n+\tsize_t hexsz;\n+\n+\t/* The hash initialization function. */\n+\tgit_hash_init_fn init_fn;\n+\n+\t/* The hash update function. */\n+\tgit_hash_update_fn update_fn;\n+\n+\t/* The hash finalization function. */\n+\tgit_hash_final_fn final_fn;\n+\n+\t/* The OID of the empty tree. */\n+\tconst struct object_id *empty_tree;\n+\n+\t/* The OID of the empty blob. */\n+\tconst struct object_id *empty_blob;\n+};\n+extern const struct git_hash_algo hash_algos[1];\n+\n #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n #define DTYPE(de)\t((de)->d_type)\n #else\ndiff --git a/sha1_file.c b/sha1_file.c\nindex b60ae15f70..bd9ff02802 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -41,6 +41,35 @@ const struct object_id empty_blob_oid = {\n \tEMPTY_BLOB_SHA1_BIN_LITERAL\n };\n \n+static inline void git_hash_sha1_init(void *ctx)\n+{\n+\tgit_SHA1_Init((git_SHA_CTX *)ctx);\n+}\n+\n+static inline void git_hash_sha1_update(void *ctx, const void *data, size_t len)\n+{\n+\tgit_SHA1_Update((git_SHA_CTX *)ctx, data, len);\n+}\n+\n+static inline void git_hash_sha1_final(unsigned char *hash, void *ctx)\n+{\n+\tgit_SHA1_Final(hash, (git_SHA_CTX *)ctx);\n+}\n+\n+const struct git_hash_algo hash_algos[1] = {\n+\t[GIT_HASH_SHA1] = {\n+\t\t.name = \"sha-1\",\n+\t\t.ctxsz = sizeof(git_SHA_CTX),\n+\t\t.rawsz = GIT_SHA1_RAWSZ,\n+\t\t.hexsz = GIT_SHA1_HEXSZ,\n+\t\t.init_fn = git_hash_sha1_init,\n+\t\t.update_fn = git_hash_sha1_update,\n+\t\t.final_fn = git_hash_sha1_final,\n+\t\t.empty_tree = &empty_tree_oid,\n+\t\t.empty_blob = &empty_blob_oid,\n+\t},\n+};\n+\n /*\n  * This is meant to hold a *small* number of objects that you would\n  * want read_sha1_file() to be able to return, but yet you do not want\n"},{"id":"326845","messageId":"20170821000022.26729-1-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":null,"subject":"[RFC PATCH 0/6] Hash Abstraction","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:16Z","receivedAt":"2017-08-21T00:00:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"= Overview\n\nThis is an RFC series proposing a basic abstraction for hash functions.\n\nAs we get closer to converting the remainder of the codebase to use\nstruct object_id, we should think about the design we want our hash\nfunction abstraction to take.  This series is a proposal for one idea to\nstart discussion.  Input on any aspect of this proposal is welcome.\n\nThis series exposes a struct git_hash_algo that contains basic\ninformation about a given hash algorithm that distinguishes it from\nother algorithms: name, lengths, implementing functions, and empty tree\nand blob constants.  It also exposes an array of hash algorithms, and a\nconstant for indexing them.\n\nThe series also demonstrates a simple conversion using the abstraction\nover empty blob and tree values.\n\nIn order to avoid conflicting with the struct repository work and with\nthe goal of avoiding global variables as much as possible, I've pushed\nthe hash algorithm into struct repository and exposed it via a #define.\nThis necessitiates pulling repository.h into cache.h, which I don't\nthink is fatal.  Doing that, in turn, necessitated some work on the\nSubversion code to avoid conflicts.\n\nIt should be fine for Junio to pick up the first two patches from this\nseries, as they're relatively independent and valuable without the rest\nof the series.  The rest should not be applied immediately, although\nthey do pass the testsuite.\n\nI proposed this series now as it will inform the way we go about\nconverting other parts of the codebase, especially some of the pack\nalgorithms.  Because we share some hash computation code between pack\nchecksums and object hashing, we need to decide whether to expose pack\nchecksums as struct object_id, even though they are technically not\nobject IDs.  Furthermore, if we end up needing to stuff an algorithm\nvalue into struct object_id, we'll no longer be able to directly\nreference object IDs in a pack without a copy.\n\nThis series is available from the usual places as branch hash-struct,\nbased against master.\n\n= Naming\n\nThe length names are similar to the current constant names\nintentionally.  I've used the \"hash_algo\" name for both the integer\nconstant and the pointer to struct, although we could change the latter\nto \"hash_impl\" or such as people like.\n\nI chose to name the define \"current_hash\" and expose no other defines.\nThe name is relatively short since we're going to be typing it a lot.\nHowever, if people like, we can capitalize it or expose other defines\n(say, a GIT_HASH_RAWSZ or GIT_HASH_HEXSZ) instead of or in addition to\ncurrent_hash, which would make this name less interesting.\n\nFeel free to propose alternatives to the naming of anything in this\nseries.\n\n= Open Issues\n\nI originally decided to convert hex.c as an example, but quickly found\nout that this caused segfaults.  As part of setup, we call\nis_git_directory, which calls validate_headref, which ends up calling\nget_sha1_hex.  Obviously, we don't have a repository, so the hash\nalgorithm isn't set up yet.  This is an area we'll need to consider\nmaking hash function agnostic, and we may also need to consider\ninserting a hash constant integer into struct object_id if we're going\nto do that.  Alternatively, we could just paper over this issue as a\nspecial case.\n\nClearly we're going to want to expose some sort of lookup functionality\nfor hash algorithms.  We'll need to expose lookup by name (for the\n.git/config file and any command-line options), but we may want other\nfunctions as well.  What functions should those be?  Should we expose\nthe structure or the constant for those lookup functions?  If the\nstructure, we'll probably need to expose the constant in the structure\nas well for easy use.\n\nShould we avoid exposing the array of structure altogether and wrap this\nin a function?\n\nWe could expose a union of hash context structures and take that as the\npointer type for the API calls.  That would probably obviate the need\nfor ctxsz.\n\nWe could expose hex versions of the blob constants if desired.  This\nmight make converting the remaining pieces of code that use them easier.\n\nThere are probably dozens of other things I haven't thought of yet as\nwell.\n\nbrian m. carlson (6):\n  vcs-svn: remove unused prototypes\n  vcs-svn: rename repo functions to \"svn_repo\"\n  setup: expose enumerated repo info\n  Add structure representing hash algorithm\n  Integrate hash algorithm support with repo setup\n  Switch empty tree and blob lookups to use hash abstraction\n\n builtin/am.c        |  2 +-\n builtin/checkout.c  |  2 +-\n builtin/diff.c      |  2 +-\n builtin/pull.c      |  2 +-\n cache.h             | 48 ++++++++++++++++++++++++++++++++++++++++++++----\n diff-lib.c          |  2 +-\n merge-recursive.c   |  2 +-\n notes-merge.c       |  2 +-\n repository.c        |  7 +++++++\n repository.h        |  5 +++++\n sequencer.c         |  6 +++---\n setup.c             | 48 +++++++++++++++++++++++++++---------------------\n sha1_file.c         | 29 +++++++++++++++++++++++++++++\n submodule.c         |  2 +-\n vcs-svn/repo_tree.c |  6 +++---\n vcs-svn/repo_tree.h | 13 +++----------\n vcs-svn/svndump.c   |  8 ++++----\n 17 files changed, 133 insertions(+), 53 deletions(-)\n\n"},{"id":"326846","messageId":"20170821000022.26729-4-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 3/6] setup: expose enumerated repo info","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:19Z","receivedAt":"2017-08-21T00:00:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We enumerate several different items as part of struct\nrepository_format, but then actually set up those values using the\nglobal variables we've initialized from them.  Instead, let's pass a\npointer to the structure down to the code where we enumerate these\nvalues, so we can later on use those values directly to perform setup.\n\nThis technique makes it easier for us to determine additional items\nabout the repository format (such as the hash algorithm) and then use\nthem for setup later on, without needing to add additional global\nvariables.  We can't avoid using the existing global variables since\nthey're intricately intertwined with how things work at the moment, but\nthis improves things for the future.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n setup.c | 46 +++++++++++++++++++++++++---------------------\n 1 file changed, 25 insertions(+), 21 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 860507e1fd..115e70a4e8 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -437,16 +437,15 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \treturn 0;\n }\n \n-static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n+static int check_repository_format_gently(const char *gitdir, struct repository_format *candidate, int *nongit_ok)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n-\tstruct repository_format candidate;\n \tint has_common;\n \n \thas_common = get_common_dir(&sb, gitdir);\n \tstrbuf_addstr(&sb, \"/config\");\n-\tread_repository_format(&candidate, sb.buf);\n+\tread_repository_format(candidate, sb.buf);\n \tstrbuf_release(&sb);\n \n \t/*\n@@ -454,10 +453,10 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n \t * we treat a missing config as a silent \"ok\", even when nongit_ok\n \t * is unset.\n \t */\n-\tif (candidate.version < 0)\n+\tif (candidate->version < 0)\n \t\treturn 0;\n \n-\tif (verify_repository_format(&candidate, &err) < 0) {\n+\tif (verify_repository_format(candidate, &err) < 0) {\n \t\tif (nongit_ok) {\n \t\t\twarning(\"%s\", err.buf);\n \t\t\tstrbuf_release(&err);\n@@ -467,21 +466,21 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n \t\tdie(\"%s\", err.buf);\n \t}\n \n-\trepository_format_precious_objects = candidate.precious_objects;\n-\tstring_list_clear(&candidate.unknown_extensions, 0);\n+\trepository_format_precious_objects = candidate->precious_objects;\n+\tstring_list_clear(&candidate->unknown_extensions, 0);\n \tif (!has_common) {\n-\t\tif (candidate.is_bare != -1) {\n-\t\t\tis_bare_repository_cfg = candidate.is_bare;\n+\t\tif (candidate->is_bare != -1) {\n+\t\t\tis_bare_repository_cfg = candidate->is_bare;\n \t\t\tif (is_bare_repository_cfg == 1)\n \t\t\t\tinside_work_tree = -1;\n \t\t}\n-\t\tif (candidate.work_tree) {\n+\t\tif (candidate->work_tree) {\n \t\t\tfree(git_work_tree_cfg);\n-\t\t\tgit_work_tree_cfg = candidate.work_tree;\n+\t\t\tgit_work_tree_cfg = candidate->work_tree;\n \t\t\tinside_work_tree = -1;\n \t\t}\n \t} else {\n-\t\tfree(candidate.work_tree);\n+\t\tfree(candidate->work_tree);\n \t}\n \n \treturn 0;\n@@ -627,6 +626,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \n static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\t\t\t\t  struct strbuf *cwd,\n+\t\t\t\t\t  struct repository_format *repo_fmt,\n \t\t\t\t\t  int *nongit_ok)\n {\n \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n@@ -652,7 +652,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\tdie(\"Not a git repository: '%s'\", gitdirenv);\n \t}\n \n-\tif (check_repository_format_gently(gitdirenv, nongit_ok)) {\n+\tif (check_repository_format_gently(gitdirenv, repo_fmt, nongit_ok)) {\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n@@ -725,9 +725,10 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \n static const char *setup_discovered_git_dir(const char *gitdir,\n \t\t\t\t\t    struct strbuf *cwd, int offset,\n+\t\t\t\t\t    struct repository_format *repo_fmt,\n \t\t\t\t\t    int *nongit_ok)\n {\n-\tif (check_repository_format_gently(gitdir, nongit_ok))\n+\tif (check_repository_format_gently(gitdir, repo_fmt, nongit_ok))\n \t\treturn NULL;\n \n \t/* --work-tree is set without --git-dir; use discovered one */\n@@ -739,7 +740,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t\t\tgitdir = to_free = real_pathdup(gitdir, 1);\n \t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n-\t\tret = setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n+\t\tret = setup_explicit_git_dir(gitdir, cwd, repo_fmt, nongit_ok);\n \t\tfree(to_free);\n \t\treturn ret;\n \t}\n@@ -771,11 +772,12 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \n /* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */\n static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n+\t\t\t\t      struct repository_format *repo_fmt,\n \t\t\t\t      int *nongit_ok)\n {\n \tint root_len;\n \n-\tif (check_repository_format_gently(\".\", nongit_ok))\n+\tif (check_repository_format_gently(\".\", repo_fmt, nongit_ok))\n \t\treturn NULL;\n \n \tsetenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, \"0\", 1);\n@@ -787,7 +789,7 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n \t\tgitdir = offset == cwd->len ? \".\" : xmemdupz(cwd->buf, offset);\n \t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n-\t\treturn setup_explicit_git_dir(gitdir, cwd, nongit_ok);\n+\t\treturn setup_explicit_git_dir(gitdir, cwd, repo_fmt, nongit_ok);\n \t}\n \n \tinside_git_dir = 1;\n@@ -1028,6 +1030,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \tstatic struct strbuf cwd = STRBUF_INIT;\n \tstruct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n \tconst char *prefix, *env_prefix;\n+\tstruct repository_format repo_fmt;\n \n \t/*\n \t * We may have read an incomplete configuration before\n@@ -1055,18 +1058,18 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\tprefix = NULL;\n \t\tbreak;\n \tcase GIT_DIR_EXPLICIT:\n-\t\tprefix = setup_explicit_git_dir(gitdir.buf, &cwd, nongit_ok);\n+\t\tprefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n \t\tbreak;\n \tcase GIT_DIR_DISCOVERED:\n \t\tif (dir.len < cwd.len && chdir(dir.buf))\n \t\t\tdie(_(\"Cannot change to '%s'\"), dir.buf);\n \t\tprefix = setup_discovered_git_dir(gitdir.buf, &cwd, dir.len,\n-\t\t\t\t\t\t  nongit_ok);\n+\t\t\t\t\t\t  &repo_fmt, nongit_ok);\n \t\tbreak;\n \tcase GIT_DIR_BARE:\n \t\tif (dir.len < cwd.len && chdir(dir.buf))\n \t\t\tdie(_(\"Cannot change to '%s'\"), dir.buf);\n-\t\tprefix = setup_bare_git_dir(&cwd, dir.len, nongit_ok);\n+\t\tprefix = setup_bare_git_dir(&cwd, dir.len, &repo_fmt, nongit_ok);\n \t\tbreak;\n \tcase GIT_DIR_HIT_CEILING:\n \t\tprefix = setup_nongit(cwd.buf, nongit_ok);\n@@ -1183,7 +1186,8 @@ int git_config_perm(const char *var, const char *value)\n \n void check_repository_format(void)\n {\n-\tcheck_repository_format_gently(get_git_dir(), NULL);\n+\tstruct repository_format repo_fmt;\n+\tcheck_repository_format_gently(get_git_dir(), &repo_fmt, NULL);\n \tstartup_info->have_repository = 1;\n }\n \n"},{"id":"326847","messageId":"20170821000022.26729-3-sandals@crustytoothpaste.net","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"[RFC PATCH 2/6] vcs-svn: rename repo functions to \"svn_repo\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-21T00:00:18Z","receivedAt":"2017-08-21T00:00:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There were several functions in the Subversion code that started with\n\"repo_\".  This namespace is also used by the Git struct repository code.\nRename these functions to start with \"svn_repo\" to avoid any future\nconflicts.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n vcs-svn/repo_tree.c | 6 +++---\n vcs-svn/repo_tree.h | 6 +++---\n vcs-svn/svndump.c   | 8 ++++----\n 3 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex 67d27f0b6c..d77cb0ada7 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -8,7 +8,7 @@\n #include \"repo_tree.h\"\n #include \"fast_export.h\"\n \n-const char *repo_read_path(const char *path, uint32_t *mode_out)\n+const char *svn_repo_read_path(const char *path, uint32_t *mode_out)\n {\n \tint err;\n \tstatic struct strbuf buf = STRBUF_INIT;\n@@ -25,7 +25,7 @@ const char *repo_read_path(const char *path, uint32_t *mode_out)\n \treturn buf.buf;\n }\n \n-void repo_copy(uint32_t revision, const char *src, const char *dst)\n+void svn_repo_copy(uint32_t revision, const char *src, const char *dst)\n {\n \tint err;\n \tuint32_t mode;\n@@ -42,7 +42,7 @@ void repo_copy(uint32_t revision, const char *src, const char *dst)\n \tfast_export_modify(dst, mode, data.buf);\n }\n \n-void repo_delete(const char *path)\n+void svn_repo_delete(const char *path)\n {\n \tfast_export_delete(path);\n }\ndiff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h\nindex d0f5690dca..555b64bbb6 100644\n--- a/vcs-svn/repo_tree.h\n+++ b/vcs-svn/repo_tree.h\n@@ -9,8 +9,8 @@ struct strbuf;\n #define REPO_MODE_LNK 0120000\n \n uint32_t next_blob_mark(void);\n-void repo_copy(uint32_t revision, const char *src, const char *dst);\n-const char *repo_read_path(const char *path, uint32_t *mode_out);\n-void repo_delete(const char *path);\n+void svn_repo_copy(uint32_t revision, const char *src, const char *dst);\n+const char *svn_repo_read_path(const char *path, uint32_t *mode_out);\n+void svn_repo_delete(const char *path);\n \n #endif\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 1846685a21..37c4a36b92 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -225,15 +225,15 @@ static void handle_node(void)\n \t\tif (have_text || have_props || node_ctx.srcRev)\n \t\t\tdie(\"invalid dump: deletion node has \"\n \t\t\t\t\"copyfrom info, text, or properties\");\n-\t\trepo_delete(node_ctx.dst.buf);\n+\t\tsvn_repo_delete(node_ctx.dst.buf);\n \t\treturn;\n \t}\n \tif (node_ctx.action == NODEACT_REPLACE) {\n-\t\trepo_delete(node_ctx.dst.buf);\n+\t\tsvn_repo_delete(node_ctx.dst.buf);\n \t\tnode_ctx.action = NODEACT_ADD;\n \t}\n \tif (node_ctx.srcRev) {\n-\t\trepo_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);\n+\t\tsvn_repo_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);\n \t\tif (node_ctx.action == NODEACT_ADD)\n \t\t\tnode_ctx.action = NODEACT_CHANGE;\n \t}\n@@ -249,7 +249,7 @@ static void handle_node(void)\n \t\told_data = NULL;\n \t} else if (node_ctx.action == NODEACT_CHANGE) {\n \t\tuint32_t mode;\n-\t\told_data = repo_read_path(node_ctx.dst.buf, &mode);\n+\t\told_data = svn_repo_read_path(node_ctx.dst.buf, &mode);\n \t\tif (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)\n \t\t\tdie(\"invalid dump: cannot modify a directory into a file\");\n \t\tif (mode != REPO_MODE_DIR && type == REPO_MODE_DIR)\n"},{"id":"326848","messageId":"xmqqefs57ozd.fsf@gitster.mtv.corp.google.com","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 0/6] Hash Abstraction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-21T00:18:14Z","receivedAt":"2017-08-21T00:18:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> brian m. carlson (6):\n>   vcs-svn: remove unused prototypes\n>   vcs-svn: rename repo functions to \"svn_repo\"\n>   setup: expose enumerated repo info\n>   Add structure representing hash algorithm\n>   Integrate hash algorithm support with repo setup\n>   Switch empty tree and blob lookups to use hash abstraction\n\nThe first two are about very quiescent code that can go in and sail\nthru 'next' very quickly without problems, I would think.\n\nAfter the third one alone, the discovered format information may not\nbe fully plumbed through (i.e. after setup finishes there is no way\nfor outside callers to peek at the repo_fmt instance that was on\nstack), but it seems that this is sufficient for the purpose of this\nstep and it looks a very sensible first step.,\n\nAs to patch 4, I tend to agree with the analysis you had in this\ncover letter--it did make me wonder if we want to have union of hash\ncontenxt structures, for example.  But the enumeration of virtual\nfunctions looks about right.\n\nI wondered if we wanted \"binery size\" and \"hex size\" separately,\nbecause the latter will always be twice as big as the former as long\nas the latter's definition is \"hex\", but that is a minor point.\n\nThanks for starting this.\n\n>  builtin/am.c        |  2 +-\n>  builtin/checkout.c  |  2 +-\n>  builtin/diff.c      |  2 +-\n>  builtin/pull.c      |  2 +-\n>  cache.h             | 48 ++++++++++++++++++++++++++++++++++++++++++++----\n>  diff-lib.c          |  2 +-\n>  merge-recursive.c   |  2 +-\n>  notes-merge.c       |  2 +-\n>  repository.c        |  7 +++++++\n>  repository.h        |  5 +++++\n>  sequencer.c         |  6 +++---\n>  setup.c             | 48 +++++++++++++++++++++++++++---------------------\n>  sha1_file.c         | 29 +++++++++++++++++++++++++++++\n>  submodule.c         |  2 +-\n>  vcs-svn/repo_tree.c |  6 +++---\n>  vcs-svn/repo_tree.h | 13 +++----------\n>  vcs-svn/svndump.c   |  8 ++++----\n>  17 files changed, 133 insertions(+), 53 deletions(-)\n"},{"id":"326901","messageId":"CAGZ79kZ1CTza=PEqP=3gTT=81HRAAUfPWQyWKxNXKf5G0e+nNg@mail.gmail.com","threadId":"46635","inReplyTo":"20170821000022.26729-1-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 0/6] Hash Abstraction","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-21T20:48:05Z","receivedAt":"2017-08-21T20:48:12Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Aug 20, 2017 at 5:00 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> = Overview\n>\n> This is an RFC series proposing a basic abstraction for hash functions.\n>\n> As we get closer to converting the remainder of the codebase to use\n> struct object_id, we should think about the design we want our hash\n> function abstraction to take.  This series is a proposal for one idea to\n> start discussion.  Input on any aspect of this proposal is welcome.\n>\n> This series exposes a struct git_hash_algo that contains basic\n> information about a given hash algorithm that distinguishes it from\n> other algorithms: name, lengths, implementing functions, and empty tree\n> and blob constants.  It also exposes an array of hash algorithms, and a\n> constant for indexing them.\n>\n> The series also demonstrates a simple conversion using the abstraction\n> over empty blob and tree values.\n>\n> In order to avoid conflicting with the struct repository work and with\n> the goal of avoiding global variables as much as possible, I've pushed\n> the hash algorithm into struct repository and exposed it via a #define.\n> This necessitiates pulling repository.h into cache.h, which I don't\n> think is fatal.  Doing that, in turn, necessitated some work on the\n> Subversion code to avoid conflicts.\n>\n> It should be fine for Junio to pick up the first two patches from this\n> series, as they're relatively independent and valuable without the rest\n> of the series.  The rest should not be applied immediately, although\n> they do pass the testsuite.\n>\n> I proposed this series now as it will inform the way we go about\n> converting other parts of the codebase, especially some of the pack\n> algorithms.  Because we share some hash computation code between pack\n> checksums and object hashing, we need to decide whether to expose pack\n> checksums as struct object_id, even though they are technically not\n> object IDs.  Furthermore, if we end up needing to stuff an algorithm\n> value into struct object_id, we'll no longer be able to directly\n> reference object IDs in a pack without a copy.\n\nNote that the checksum hash could be different from actual object ids,\nso it may be easiest to introduce a \"new pack format\", which is just\ns/sha1/<new hash>/g for any object ids, though I guess the work needed\nis just the same for converting checksums to the new hash, too.\n\n\n> This series is available from the usual places as branch hash-struct,\n> based against master.\n>\n> = Naming\n>\n> The length names are similar to the current constant names\n> intentionally.  I've used the \"hash_algo\" name for both the integer\n> constant and the pointer to struct, although we could change the latter\n> to \"hash_impl\" or such as people like.\n>\n> I chose to name the define \"current_hash\" and expose no other defines.\n> The name is relatively short since we're going to be typing it a lot.\n> However, if people like, we can capitalize it or expose other defines\n> (say, a GIT_HASH_RAWSZ or GIT_HASH_HEXSZ) instead of or in addition to\n> current_hash, which would make this name less interesting.\n>\n> Feel free to propose alternatives to the naming of anything in this\n> series.\n>\n> = Open Issues\n>\n> I originally decided to convert hex.c as an example, but quickly found\n> out that this caused segfaults.  As part of setup, we call\n> is_git_directory, which calls validate_headref, which ends up calling\n> get_sha1_hex.  Obviously, we don't have a repository, so the hash\n> algorithm isn't set up yet.  This is an area we'll need to consider\n> making hash function agnostic, and we may also need to consider\n> inserting a hash constant integer into struct object_id if we're going\n> to do that.  Alternatively, we could just paper over this issue as a\n> special case.\n\nWe could introduce new files .sha1 or .sha256 in the config dir, whose\nexistence can be checked prior to validating the HEAD.\n\nMaybe the HEAD validation can be non-strict and could check\nif we'd have at least 'n' hex chars and at most 'm' hex chars. 'n' and 'm'\ndepend on the hash functions supported of that version of git. (Maybe\nwe'd validate later again to have the correct hash function length,\nor we could also allow abbreviated hash names for HEAD)\n\n> Clearly we're going to want to expose some sort of lookup functionality\n> for hash algorithms.  We'll need to expose lookup by name (for the\n> .git/config file and any command-line options), but we may want other\n> functions as well.  What functions should those be?  Should we expose\n> the structure or the constant for those lookup functions?  If the\n> structure, we'll probably need to expose the constant in the structure\n> as well for easy use.\n>\n> Should we avoid exposing the array of structure altogether and wrap this\n> in a function?\n>\n> We could expose a union of hash context structures and take that as the\n> pointer type for the API calls.  That would probably obviate the need\n> for ctxsz.\n>\n> We could expose hex versions of the blob constants if desired.  This\n> might make converting the remaining pieces of code that use them easier.\n>\n> There are probably dozens of other things I haven't thought of yet as\n> well.\n>\n> brian m. carlson (6):\n>   vcs-svn: remove unused prototypes\n>   vcs-svn: rename repo functions to \"svn_repo\"\n>   setup: expose enumerated repo info\n>   Add structure representing hash algorithm\n>   Integrate hash algorithm support with repo setup\n>   Switch empty tree and blob lookups to use hash abstraction\n>\n>  builtin/am.c        |  2 +-\n>  builtin/checkout.c  |  2 +-\n>  builtin/diff.c      |  2 +-\n>  builtin/pull.c      |  2 +-\n>  cache.h             | 48 ++++++++++++++++++++++++++++++++++++++++++++----\n>  diff-lib.c          |  2 +-\n>  merge-recursive.c   |  2 +-\n>  notes-merge.c       |  2 +-\n>  repository.c        |  7 +++++++\n>  repository.h        |  5 +++++\n>  sequencer.c         |  6 +++---\n>  setup.c             | 48 +++++++++++++++++++++++++++---------------------\n>  sha1_file.c         | 29 +++++++++++++++++++++++++++++\n>  submodule.c         |  2 +-\n>  vcs-svn/repo_tree.c |  6 +++---\n>  vcs-svn/repo_tree.h | 13 +++----------\n>  vcs-svn/svndump.c   |  8 ++++----\n>  17 files changed, 133 insertions(+), 53 deletions(-)\n>\n"},{"id":"326903","messageId":"CAGZ79ka1X5zJwEzH8Pe2vNX8FaKSP_3m6D0Fva4cpO=QUEaeRw@mail.gmail.com","threadId":"46635","inReplyTo":"20170821000022.26729-5-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 4/6] Add structure representing hash algorithm","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-21T21:08:40Z","receivedAt":"2017-08-21T21:08:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Aug 20, 2017 at 5:00 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> Since in the future we want to support an additional hash algorithm, add\n> a structure that represents a hash algorithm and all the data that must\n> go along with it.  Add a constant to allow easy enumeration of hash\n> algorithms.  Implement function typedefs to create an abstract API that\n> can be used by any hash algorithm, and wrappers for the existing SHA1\n> functions that conform to this API.\n>\n> Don't include an entry in the hash algorithm structure for the null\n> object ID.  As this value is all zeros, any suitably sized all-zero\n> object ID can be used, and there's no need to store a given one on a\n> per-hash basis.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  cache.h     | 36 ++++++++++++++++++++++++++++++++++++\n>  sha1_file.c | 29 +++++++++++++++++++++++++++++\n>  2 files changed, 65 insertions(+)\n>\n> diff --git a/cache.h b/cache.h\n> index 1c69d2a05a..375a7fb15e 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -76,6 +76,42 @@ struct object_id {\n>         unsigned char hash[GIT_MAX_RAWSZ];\n>  };\n>\n> +#define GIT_HASH_SHA1 0\n> +\n> +typedef void (*git_hash_init_fn)(void *ctx);\n> +typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);\n> +typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);\n> +\n> +struct git_hash_algo {\n> +       /* The name of the algorithm, as appears in the config file. */\n> +       const char *name;\n\nand potentially in error messages?\n\n> +\n> +       /* The size of a hash context (e.g. git_SHA_CTX). */\n> +       size_t ctxsz;\n> +\n> +       /* The length of the hash in bytes. */\n> +       size_t rawsz;\n\n[in binary, as opposed to the next entry]\n\n> +\n> +       /* The length of the hash in hex characters. */\n> +       size_t hexsz;\n\nBy having two entries for binary and hex size, we current\nchoice of needing twice as much choice for the human\nrepresentation (as is inherent from the binary <-> hex\nconversion, one char stores 8 or 4 bit); so this is good to\nhave. But is it misnamed? (This abstraction only makes\nsense if we ever plan to have human readable representation\nnot factor 2, e.g. base64 for the non-binary representation.\nBut then the comment is wrong!)\n\nMaybe s/hex characters/string representation suited for humans/ ?\n(Bad naming proposal, but still)\n\n> +       /* The hash initialization function. */\n> +       git_hash_init_fn init_fn;\n> +\n> +       /* The hash update function. */\n> +       git_hash_update_fn update_fn;\n> +\n> +       /* The hash finalization function. */\n> +       git_hash_final_fn final_fn;\n\nI shortly wondered if we want to have just one\npointer to a struct that contains these 3 functions,\nbut that seems overly complex.\n\n> +       /* The OID of the empty tree. */\n> +       const struct object_id *empty_tree;\n> +\n> +       /* The OID of the empty blob. */\n> +       const struct object_id *empty_blob;\n\nIn a more object oriented world, this would\nnot be as micro-optimized(?), but rather be:\n\n    object o = new object()\n    o.getHash()\n\nor such, but we tend to access empty_{blob,tree}\nquite often in the code base. So it seems like a good\nidea to keep this shortcut around.\n\n\n> +};\n> +extern const struct git_hash_algo hash_algos[1];\n> +\n>  #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n>  #define DTYPE(de)      ((de)->d_type)\n>  #else\n> diff --git a/sha1_file.c b/sha1_file.c\n> index b60ae15f70..bd9ff02802 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -41,6 +41,35 @@ const struct object_id empty_blob_oid = {\n>         EMPTY_BLOB_SHA1_BIN_LITERAL\n>  };\n>\n> +static inline void git_hash_sha1_init(void *ctx)\n> +{\n> +       git_SHA1_Init((git_SHA_CTX *)ctx);\n> +}\n> +\n> +static inline void git_hash_sha1_update(void *ctx, const void *data, size_t len)\n> +{\n> +       git_SHA1_Update((git_SHA_CTX *)ctx, data, len);\n> +}\n> +\n> +static inline void git_hash_sha1_final(unsigned char *hash, void *ctx)\n> +{\n> +       git_SHA1_Final(hash, (git_SHA_CTX *)ctx);\n> +}\n> +\n> +const struct git_hash_algo hash_algos[1] = {\n> +       [GIT_HASH_SHA1] = {\n> +               .name = \"sha-1\",\n> +               .ctxsz = sizeof(git_SHA_CTX),\n> +               .rawsz = GIT_SHA1_RAWSZ,\n> +               .hexsz = GIT_SHA1_HEXSZ,\n> +               .init_fn = git_hash_sha1_init,\n> +               .update_fn = git_hash_sha1_update,\n> +               .final_fn = git_hash_sha1_final,\n> +               .empty_tree = &empty_tree_oid,\n> +               .empty_blob = &empty_blob_oid,\n> +       },\n> +};\n\nThis is the same new pattern as in 512f41cfac (clean.c: use\ndesignated initializer, 2017-07-14)\n\nMaybe we want to keep just one test balloon and not mix\nthis one in there, too?\n\n> +\n>  /*\n>   * This is meant to hold a *small* number of objects that you would\n>   * want read_sha1_file() to be able to return, but yet you do not want\n"},{"id":"326904","messageId":"CAGZ79kZrFDNfZYK_CL_rwn6pqB8no3+SFM_4H+RjURRz=R+Fag@mail.gmail.com","threadId":"46635","inReplyTo":"20170821000022.26729-6-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 5/6] Integrate hash algorithm support with repo setup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-21T21:16:29Z","receivedAt":"2017-08-21T21:16:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Aug 20, 2017 at 5:00 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> In future versions of Git, we plan to support an additional hash\n> algorithm.  Integrate the enumeration of hash algorithms with repository\n> setup, and store a pointer to the enumerated data in struct repository.\n> Of course, we currently only support SHA-1, so hard-code this value in\n> read_repository_format.  In the future, we'll enumerate this value from\n> the configuration.\n>\n> Add a constant, current_hash, which points to the hash_algo structure\n> pointer in the repository global.  Include repository.h in cache.h since\n> we now need to have access to these struct and variable definitions.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n\nThe new code seems to fit in nicely with the repository struct.\n\nIn another series I also included repository.h in cache.h\n(not yet sent out), so I think that is a good idea.\n\n> ---\n>  cache.h      | 4 ++++\n>  repository.c | 7 +++++++\n>  repository.h | 5 +++++\n>  setup.c      | 2 ++\n>  4 files changed, 18 insertions(+)\n>\n> diff --git a/cache.h b/cache.h\n> index 375a7fb15e..d759824803 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -13,6 +13,7 @@\n>  #include \"hash.h\"\n>  #include \"path.h\"\n>  #include \"sha1-array.h\"\n> +#include \"repository.h\"\n>\n>  #ifndef platform_SHA_CTX\n>  /*\n> @@ -112,6 +113,8 @@ struct git_hash_algo {\n>  };\n>  extern const struct git_hash_algo hash_algos[1];\n>\n> +#define current_hash the_repository->hash_algo\n> +\n>  #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n>  #define DTYPE(de)      ((de)->d_type)\n>  #else\n> @@ -894,6 +897,7 @@ struct repository_format {\n>         int version;\n>         int precious_objects;\n>         int is_bare;\n> +       int hash_algo;\n>         char *work_tree;\n>         struct string_list unknown_extensions;\n>  };\n> diff --git a/repository.c b/repository.c\n> index 1617467568..37764f627a 100644\n> --- a/repository.c\n> +++ b/repository.c\n> @@ -62,6 +62,11 @@ void repo_set_gitdir(struct repository *repo, const char *path)\n>         repo_setup_env(repo);\n>  }\n>\n> +void repo_set_hash_algo(struct repository *repo, int hash_algo)\n> +{\n> +       repo->hash_algo = &hash_algos[hash_algo];\n> +}\n> +\n>  /*\n>   * Attempt to resolve and set the provided 'gitdir' for repository 'repo'.\n>   * Return 0 upon success and a non-zero value upon failure.\n> @@ -134,6 +139,8 @@ int repo_init(struct repository *repo, const char *gitdir, const char *worktree)\n>         if (read_and_verify_repository_format(&format, repo->commondir))\n>                 goto error;\n>\n> +       repo->hash_algo = &hash_algos[format.hash_algo];\n> +\n>         if (worktree)\n>                 repo_set_worktree(repo, worktree);\n>\n> diff --git a/repository.h b/repository.h\n> index 417787f3ef..f171172150 100644\n> --- a/repository.h\n> +++ b/repository.h\n> @@ -4,6 +4,7 @@\n>  struct config_set;\n>  struct index_state;\n>  struct submodule_cache;\n> +struct git_hash_algo;\n>\n>  struct repository {\n>         /* Environment */\n> @@ -67,6 +68,9 @@ struct repository {\n>          */\n>         struct index_state *index;\n>\n> +       /* Repository's current hash algorithm. */\n> +       const struct git_hash_algo *hash_algo;\n> +\n>         /* Configurations */\n>         /*\n>          * Bit used during initialization to indicate if repository state (like\n> @@ -86,6 +90,7 @@ extern struct repository *the_repository;\n>\n>  extern void repo_set_gitdir(struct repository *repo, const char *path);\n>  extern void repo_set_worktree(struct repository *repo, const char *path);\n> +extern void repo_set_hash_algo(struct repository *repo, int algo);\n>  extern int repo_init(struct repository *repo, const char *gitdir, const char *worktree);\n>  extern int repo_submodule_init(struct repository *submodule,\n>                                struct repository *superproject,\n> diff --git a/setup.c b/setup.c\n> index 115e70a4e8..289e24811c 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -491,6 +491,7 @@ int read_repository_format(struct repository_format *format, const char *path)\n>         memset(format, 0, sizeof(*format));\n>         format->version = -1;\n>         format->is_bare = -1;\n> +       format->hash_algo = GIT_HASH_SHA1;\n>         string_list_init(&format->unknown_extensions, 1);\n>         git_config_from_file(check_repo_format, path, format);\n>         return format->version;\n> @@ -1125,6 +1126,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>                         repo_set_gitdir(the_repository, gitdir);\n>                         setup_git_env();\n>                 }\n> +               repo_set_hash_algo(the_repository, repo_fmt.hash_algo);\n>         }\n>\n>         strbuf_release(&dir);\n"}]}