{"thread":{"id":"52720","subject":"[PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","startedAt":"2020-01-30T20:33:18Z","lastAt":"2020-02-01T11:08:38Z","messageCount":16,"participants":["Matheus Tavares","Junio C Hamano","brian m. carlson","Jeff King","Matheus Tavares Bernardino","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"390836","messageId":"cover.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":null,"subject":"[PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:16Z","receivedAt":"2020-01-30T20:33:18Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"The motivation for this patchset is another series I'm working on, to\nmake git-grep stop adding submodules' odbs to the alternates list. With\nthat, parse_object() will have to be called with the subdmodules' struct\nrepository. But it seemed that this function doesn't pass on the\nreceived repo to some inner calls, which, instead, always use\nthe_repository. This series seeks to fix these inconsistencies.\n\nNote: I also tried to replace some uses of the_hash_algo with the struct\ngit_hash_algo from the received repository, for consitency. (In\npractice, I'm not sure if this is very useful right now, but maybe it\nwill be relevant for the change to SHA256?) Still, many functions in\nparse_object()'s call chain call oid_to_hex(), which uses the_hash_algo.\nSince changing this would require a much bigger operation, I decided to\nleave it as is, for now.\n\nNote II: Despite receiving a repo through the apply_state struct,\napply.c:apply_binary() call functions which uses the_repository\ninternally. Because of that, I used the_hash_algo in this function in\npatch 6. Should I change it to use apply_state->repo->hash_algo\nanyway?\n\ntravis build: https://travis-ci.org/matheustavares/git/builds/644022000\n\nMatheus Tavares (7):\n  diff: make diff_populate_filespec() honor its repo argument\n  cache-tree: use given repo's hash_algo at verify_one()\n  pack-check: use given repo's hash_algo at verify_packfile()\n  streaming: allow open_istream() to handle any repo\n  sha1-file: pass git_hash_algo to write_object_file_prepare()\n  sha1-file: pass git_hash_algo to hash_object_file()\n  sha1-file: allow check_object_signature() to handle any repo\n\n apply.c                  |  6 +++--\n archive-tar.c            |  6 ++---\n archive-zip.c            |  3 ++-\n builtin/fast-export.c    |  3 ++-\n builtin/index-pack.c     | 10 +++++---\n builtin/mktag.c          |  7 +++--\n builtin/pack-objects.c   |  3 ++-\n builtin/replace.c        |  3 ++-\n builtin/unpack-objects.c |  3 ++-\n cache-tree.c             | 11 +++++---\n cache.h                  |  3 ++-\n convert.c                |  2 +-\n diff.c                   |  2 +-\n diffcore-rename.c        |  4 +--\n dir.c                    |  4 +--\n log-tree.c               |  3 ++-\n object-store.h           |  5 ++--\n object.c                 |  5 ++--\n pack-check.c             | 12 ++++-----\n sha1-file.c              | 55 ++++++++++++++++++++++------------------\n streaming.c              | 28 ++++++++++----------\n streaming.h              |  4 ++-\n 22 files changed, 106 insertions(+), 76 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"390837","messageId":"5a31c7b9e3239f2b7e4841dcc85cbf9cddf545ed.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 1/7] diff: make diff_populate_filespec() honor its repo argument","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:17Z","receivedAt":"2020-01-30T20:33:34Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"diff_populate_filespec() takes a struct repository argument but it\ndoesn't get passed down to read_object_file().\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8e2914c031..0f333c27d6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4024,7 +4024,7 @@ int diff_populate_filespec(struct repository *r,\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t}\n-\t\ts->data = read_object_file(&s->oid, &type, &s->size);\n+\t\ts->data = repo_read_object_file(r, &s->oid, &type, &s->size);\n \t\tif (!s->data)\n \t\t\tdie(\"unable to read %s\", oid_to_hex(&s->oid));\n \t\ts->should_free = 1;\n-- \n2.25.0\n\n"},{"id":"390838","messageId":"8bab40cb3357d64b8d365b8848ded99122056689.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 2/7] cache-tree: use given repo's hash_algo at verify_one()","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:18Z","receivedAt":"2020-01-30T20:33:40Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"verify_one() takes a struct repository argument but uses the_hash_algo\ninternally. Replace it with the provided repo's git_hash_algo, for\nconsistency. For now, this is mainly a cosmetic change, as all callers\nof this function currently only pass the_repository to it.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n cache-tree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 1bd1b23d38..8c51a15751 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -826,7 +826,7 @@ static void verify_one(struct repository *r,\n \t\t\ti++;\n \t\t}\n \t\tstrbuf_addf(&tree_buf, \"%o %.*s%c\", mode, entlen, name, '\\0');\n-\t\tstrbuf_add(&tree_buf, oid->hash, the_hash_algo->rawsz);\n+\t\tstrbuf_add(&tree_buf, oid->hash, r->hash_algo->rawsz);\n \t}\n \thash_object_file(tree_buf.buf, tree_buf.len, tree_type, &new_oid);\n \tif (!oideq(&new_oid, &it->oid))\n-- \n2.25.0\n\n"},{"id":"390839","messageId":"b06ad765589e0384abc5d860fa89781f18ef8dac.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 3/7] pack-check: use given repo's hash_algo at verify_packfile()","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:19Z","receivedAt":"2020-01-30T20:33:47Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"At verify_packfile(), use the git_hash_algo from the provided repository\ninstead of the_hash_algo, for consistency. Like the previous patch, this\nshouldn't bring any behavior changes, since this function is currently\nonly receiving the_repository.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n pack-check.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/pack-check.c b/pack-check.c\nindex 2cc3603189..0fb3b365c7 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -67,23 +67,23 @@ static int verify_packfile(struct repository *r,\n \tif (!is_pack_valid(p))\n \t\treturn error(\"packfile %s cannot be accessed\", p->pack_name);\n \n-\tthe_hash_algo->init_fn(&ctx);\n+\tr->hash_algo->init_fn(&ctx);\n \tdo {\n \t\tunsigned long remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n \t\toffset += remaining;\n \t\tif (!pack_sig_ofs)\n-\t\t\tpack_sig_ofs = p->pack_size - the_hash_algo->rawsz;\n+\t\t\tpack_sig_ofs = p->pack_size - r->hash_algo->rawsz;\n \t\tif (offset > pack_sig_ofs)\n \t\t\tremaining -= (unsigned int)(offset - pack_sig_ofs);\n-\t\tthe_hash_algo->update_fn(&ctx, in, remaining);\n+\t\tr->hash_algo->update_fn(&ctx, in, remaining);\n \t} while (offset < pack_sig_ofs);\n-\tthe_hash_algo->final_fn(hash, &ctx);\n+\tr->hash_algo->final_fn(hash, &ctx);\n \tpack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);\n \tif (!hasheq(hash, pack_sig))\n \t\terr = error(\"%s pack checksum mismatch\",\n \t\t\t    p->pack_name);\n-\tif (!hasheq(index_base + index_size - the_hash_algo->hexsz, pack_sig))\n+\tif (!hasheq(index_base + index_size - r->hash_algo->hexsz, pack_sig))\n \t\terr = error(\"%s pack checksum does not match its index\",\n \t\t\t    p->pack_name);\n \tunuse_pack(w_curs);\n-- \n2.25.0\n\n"},{"id":"390840","messageId":"b76b189efb6d3b6e4d345fd1876a676ed27d1653.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 4/7] streaming: allow open_istream() to handle any repo","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:20Z","receivedAt":"2020-01-30T20:33:52Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Some callers of open_istream() at archive-tar.c and archive-zip.c are\ncapable of working on arbitrary repositories but the repo struct is not\npassed down to open_istream(), which uses the_repository internally. For\nnow, that's not a problem since the said callers are only being called\nwith the_repository. But to be consistent and avoid future problems,\nlet's allow open_istream() to receive a struct repository and use that\ninstead of the_repository. This parameter addition will also be used in\na future patch to make sha1-file.c:check_object_signature() be able to\nwork on arbitrary repos.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n archive-tar.c          |  6 +++---\n archive-zip.c          |  3 ++-\n builtin/index-pack.c   |  3 ++-\n builtin/pack-objects.c |  3 ++-\n sha1-file.c            |  2 +-\n streaming.c            | 28 +++++++++++++++-------------\n streaming.h            |  4 +++-\n 7 files changed, 28 insertions(+), 21 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex e16d3f756d..5a77701a15 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -112,7 +112,7 @@ static void write_trailer(void)\n  * queues up writes, so that all our write(2) calls write exactly one\n  * full block; pads writes to RECORDSIZE\n  */\n-static int stream_blocked(const struct object_id *oid)\n+static int stream_blocked(struct repository *r, const struct object_id *oid)\n {\n \tstruct git_istream *st;\n \tenum object_type type;\n@@ -120,7 +120,7 @@ static int stream_blocked(const struct object_id *oid)\n \tchar buf[BLOCKSIZE];\n \tssize_t readlen;\n \n-\tst = open_istream(oid, &type, &sz, NULL);\n+\tst = open_istream(r, oid, &type, &sz, NULL);\n \tif (!st)\n \t\treturn error(_(\"cannot stream blob %s\"), oid_to_hex(oid));\n \tfor (;;) {\n@@ -324,7 +324,7 @@ static int write_tar_entry(struct archiver_args *args,\n \t\tif (buffer)\n \t\t\twrite_blocked(buffer, size);\n \t\telse\n-\t\t\terr = stream_blocked(oid);\n+\t\t\terr = stream_blocked(args->repo, oid);\n \t}\n \tfree(buffer);\n \treturn err;\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 11f5b1974b..e9f426298b 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -345,7 +345,8 @@ static int write_zip_entry(struct archiver_args *args,\n \n \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n \t\t    size > big_file_threshold) {\n-\t\t\tstream = open_istream(oid, &type, &size, NULL);\n+\t\t\tstream = open_istream(args->repo, oid, &type, &size,\n+\t\t\t\t\t      NULL);\n \t\t\tif (!stream)\n \t\t\t\treturn error(_(\"cannot stream blob %s\"),\n \t\t\t\t\t     oid_to_hex(oid));\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 60a5591039..7a08da8401 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -757,7 +757,8 @@ static int check_collison(struct object_entry *entry)\n \n \tmemset(&data, 0, sizeof(data));\n \tdata.entry = entry;\n-\tdata.st = open_istream(&entry->idx.oid, &type, &size, NULL);\n+\tdata.st = open_istream(the_repository, &entry->idx.oid, &type, &size,\n+\t\t\t       NULL);\n \tif (!data.st)\n \t\treturn -1;\n \tif (size != entry->size || type != entry->type)\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 393c20a2d7..ef17efd94e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -303,7 +303,8 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \tif (!usable_delta) {\n \t\tif (oe_type(entry) == OBJ_BLOB &&\n \t\t    oe_size_greater_than(&to_pack, entry, big_file_threshold) &&\n-\t\t    (st = open_istream(&entry->idx.oid, &type, &size, NULL)) != NULL)\n+\t\t    (st = open_istream(the_repository, &entry->idx.oid, &type,\n+\t\t\t\t       &size, NULL)) != NULL)\n \t\t\tbuf = NULL;\n \t\telse {\n \t\t\tbuf = read_object_file(&entry->idx.oid, &type, &size);\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 03ae9ae93a..13b90b1cb1 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -986,7 +986,7 @@ int check_object_signature(const struct object_id *oid, void *map,\n \t\treturn !oideq(oid, &real_oid) ? -1 : 0;\n \t}\n \n-\tst = open_istream(oid, &obj_type, &size, NULL);\n+\tst = open_istream(the_repository, oid, &obj_type, &size, NULL);\n \tif (!st)\n \t\treturn -1;\n \ndiff --git a/streaming.c b/streaming.c\nindex fcd6303219..800f07a52c 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -16,6 +16,7 @@ enum input_source {\n };\n \n typedef int (*open_istream_fn)(struct git_istream *,\n+\t\t\t       struct repository *,\n \t\t\t       struct object_info *,\n \t\t\t       const struct object_id *,\n \t\t\t       enum object_type *);\n@@ -29,8 +30,8 @@ struct stream_vtbl {\n \n #define open_method_decl(name) \\\n \tint open_istream_ ##name \\\n-\t(struct git_istream *st, struct object_info *oi, \\\n-\t const struct object_id *oid, \\\n+\t(struct git_istream *st, struct repository *r, \\\n+\t struct object_info *oi, const struct object_id *oid, \\\n \t enum object_type *type)\n \n #define close_method_decl(name) \\\n@@ -108,7 +109,8 @@ ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n \treturn st->vtbl->read(st, buf, sz);\n }\n \n-static enum input_source istream_source(const struct object_id *oid,\n+static enum input_source istream_source(struct repository *r,\n+\t\t\t\t\tconst struct object_id *oid,\n \t\t\t\t\tenum object_type *type,\n \t\t\t\t\tstruct object_info *oi)\n {\n@@ -117,7 +119,7 @@ static enum input_source istream_source(const struct object_id *oid,\n \n \toi->typep = type;\n \toi->sizep = &size;\n-\tstatus = oid_object_info_extended(the_repository, oid, oi, 0);\n+\tstatus = oid_object_info_extended(r, oid, oi, 0);\n \tif (status < 0)\n \t\treturn stream_error;\n \n@@ -133,22 +135,23 @@ static enum input_source istream_source(const struct object_id *oid,\n \t}\n }\n \n-struct git_istream *open_istream(const struct object_id *oid,\n+struct git_istream *open_istream(struct repository *r,\n+\t\t\t\t const struct object_id *oid,\n \t\t\t\t enum object_type *type,\n \t\t\t\t unsigned long *size,\n \t\t\t\t struct stream_filter *filter)\n {\n \tstruct git_istream *st;\n \tstruct object_info oi = OBJECT_INFO_INIT;\n-\tconst struct object_id *real = lookup_replace_object(the_repository, oid);\n-\tenum input_source src = istream_source(real, type, &oi);\n+\tconst struct object_id *real = lookup_replace_object(r, oid);\n+\tenum input_source src = istream_source(r, real, type, &oi);\n \n \tif (src < 0)\n \t\treturn NULL;\n \n \tst = xmalloc(sizeof(*st));\n-\tif (open_istream_tbl[src](st, &oi, real, type)) {\n-\t\tif (open_istream_incore(st, &oi, real, type)) {\n+\tif (open_istream_tbl[src](st, r, &oi, real, type)) {\n+\t\tif (open_istream_incore(st, r, &oi, real, type)) {\n \t\t\tfree(st);\n \t\t\treturn NULL;\n \t\t}\n@@ -338,8 +341,7 @@ static struct stream_vtbl loose_vtbl = {\n \n static open_method_decl(loose)\n {\n-\tst->u.loose.mapped = map_loose_object(the_repository,\n-\t\t\t\t\t      oid, &st->u.loose.mapsize);\n+\tst->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize);\n \tif (!st->u.loose.mapped)\n \t\treturn -1;\n \tif ((unpack_loose_header(&st->z,\n@@ -499,7 +501,7 @@ static struct stream_vtbl incore_vtbl = {\n \n static open_method_decl(incore)\n {\n-\tst->u.incore.buf = read_object_file_extended(the_repository, oid, type, &st->size, 0);\n+\tst->u.incore.buf = read_object_file_extended(r, oid, type, &st->size, 0);\n \tst->u.incore.read_ptr = 0;\n \tst->vtbl = &incore_vtbl;\n \n@@ -520,7 +522,7 @@ int stream_blob_to_fd(int fd, const struct object_id *oid, struct stream_filter\n \tssize_t kept = 0;\n \tint result = -1;\n \n-\tst = open_istream(oid, &type, &sz, filter);\n+\tst = open_istream(the_repository, oid, &type, &sz, filter);\n \tif (!st) {\n \t\tif (filter)\n \t\t\tfree_stream_filter(filter);\ndiff --git a/streaming.h b/streaming.h\nindex f465a3cd31..5e4e6acfd0 100644\n--- a/streaming.h\n+++ b/streaming.h\n@@ -8,7 +8,9 @@\n /* opaque */\n struct git_istream;\n \n-struct git_istream *open_istream(const struct object_id *, enum object_type *, unsigned long *, struct stream_filter *);\n+struct git_istream *open_istream(struct repository *, const struct object_id *,\n+\t\t\t\t enum object_type *, unsigned long *,\n+\t\t\t\t struct stream_filter *);\n int close_istream(struct git_istream *);\n ssize_t read_istream(struct git_istream *, void *, size_t);\n \n-- \n2.25.0\n\n"},{"id":"390841","messageId":"f256d2058736091c3d9662788762849e8df794b0.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 5/7] sha1-file: pass git_hash_algo to write_object_file_prepare()","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:21Z","receivedAt":"2020-01-30T20:33:59Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Allow write_object_file_prepare() to receive arbitrary 'struct\ngit_hash_algo's instead of always using the_hash_algo. The added\nparameter will be used in the next commit to make hash_object_file() be\nable to work with arbitrary git_hash_algo's, as well.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n sha1-file.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 13b90b1cb1..e789bfd153 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r,\n \t}\n }\n \n-static void write_object_file_prepare(const void *buf, unsigned long len,\n+static void write_object_file_prepare(const struct git_hash_algo *algo,\n+\t\t\t\t      const void *buf, unsigned long len,\n \t\t\t\t      const char *type, struct object_id *oid,\n \t\t\t\t      char *hdr, int *hdrlen)\n {\n@@ -1657,10 +1658,10 @@ static void write_object_file_prepare(const void *buf, unsigned long len,\n \t*hdrlen = xsnprintf(hdr, *hdrlen, \"%s %\"PRIuMAX , type, (uintmax_t)len)+1;\n \n \t/* Sha1.. */\n-\tthe_hash_algo->init_fn(&c);\n-\tthe_hash_algo->update_fn(&c, hdr, *hdrlen);\n-\tthe_hash_algo->update_fn(&c, buf, len);\n-\tthe_hash_algo->final_fn(oid->hash, &c);\n+\talgo->init_fn(&c);\n+\talgo->update_fn(&c, hdr, *hdrlen);\n+\talgo->update_fn(&c, buf, len);\n+\talgo->final_fn(oid->hash, &c);\n }\n \n /*\n@@ -1718,7 +1719,8 @@ int hash_object_file(const void *buf, unsigned long len, const char *type,\n {\n \tchar hdr[MAX_HEADER_LEN];\n \tint hdrlen = sizeof(hdr);\n-\twrite_object_file_prepare(buf, len, type, oid, hdr, &hdrlen);\n+\twrite_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr,\n+\t\t\t\t  &hdrlen);\n \treturn 0;\n }\n \n@@ -1876,7 +1878,8 @@ int write_object_file(const void *buf, unsigned long len, const char *type,\n \t/* Normally if we have it in the pack then we do not bother writing\n \t * it out into .git/objects/??/?{38} file.\n \t */\n-\twrite_object_file_prepare(buf, len, type, oid, hdr, &hdrlen);\n+\twrite_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr,\n+\t\t\t\t  &hdrlen);\n \tif (freshen_packed_object(oid) || freshen_loose_object(oid))\n \t\treturn 0;\n \treturn write_loose_object(oid, hdr, hdrlen, buf, len, 0);\n@@ -1892,7 +1895,8 @@ int hash_object_file_literally(const void *buf, unsigned long len,\n \t/* type string, SP, %lu of the length plus NUL must fit this */\n \thdrlen = strlen(type) + MAX_HEADER_LEN;\n \theader = xmalloc(hdrlen);\n-\twrite_object_file_prepare(buf, len, type, oid, header, &hdrlen);\n+\twrite_object_file_prepare(the_hash_algo, buf, len, type, oid, header,\n+\t\t\t\t  &hdrlen);\n \n \tif (!(flags & HASH_WRITE_OBJECT))\n \t\tgoto cleanup;\n-- \n2.25.0\n\n"},{"id":"390842","messageId":"40b95d0a979eb4865572611bfb6d1bfe2be1de57.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 6/7] sha1-file: pass git_hash_algo to hash_object_file()","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:22Z","receivedAt":"2020-01-30T20:34:07Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Allow hash_object_file() to work on arbitrary repos by introducing a\ngit_hash_algo parameter. Change callers which have a struct repository\npointer in their scope to pass on the git_hash_algo from the said repo.\nFor all other callers, pass on the_hash_algo, which was already being\nused internally at hash_object_file(). This functionality will be used\nin the following patch to make check_object_signature() be able to work\non arbitrary repos (which, in turn, will be used to fix an\ninconsistency at object.c:parse_object()).\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n apply.c                  |  6 ++++--\n builtin/index-pack.c     |  2 +-\n builtin/replace.c        |  3 ++-\n builtin/unpack-objects.c |  3 ++-\n cache-tree.c             |  9 ++++++---\n convert.c                |  2 +-\n diffcore-rename.c        |  4 ++--\n dir.c                    |  4 ++--\n log-tree.c               |  3 ++-\n object-store.h           |  5 +++--\n sha1-file.c              | 20 +++++++++++---------\n 11 files changed, 36 insertions(+), 25 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex fab44322c5..bdc008fae2 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -3157,7 +3157,8 @@ static int apply_binary(struct apply_state *state,\n \t\t * See if the old one matches what the patch\n \t\t * applies to.\n \t\t */\n-\t\thash_object_file(img->buf, img->len, blob_type, &oid);\n+\t\thash_object_file(the_hash_algo, img->buf, img->len, blob_type,\n+\t\t\t\t &oid);\n \t\tif (strcmp(oid_to_hex(&oid), patch->old_oid_prefix))\n \t\t\treturn error(_(\"the patch applies to '%s' (%s), \"\n \t\t\t\t       \"which does not match the \"\n@@ -3202,7 +3203,8 @@ static int apply_binary(struct apply_state *state,\n \t\t\t\t     name);\n \n \t\t/* verify that the result matches */\n-\t\thash_object_file(img->buf, img->len, blob_type, &oid);\n+\t\thash_object_file(the_hash_algo, img->buf, img->len, blob_type,\n+\t\t\t\t &oid);\n \t\tif (strcmp(oid_to_hex(&oid), patch->new_oid_prefix))\n \t\t\treturn error(_(\"binary patch to '%s' creates incorrect result (expecting %s, got %s)\"),\n \t\t\t\tname, patch->new_oid_prefix, oid_to_hex(&oid));\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 7a08da8401..0183610a76 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -949,7 +949,7 @@ static void resolve_delta(struct object_entry *delta_obj,\n \tfree(delta_data);\n \tif (!result->data)\n \t\tbad_object(delta_obj->idx.offset, _(\"failed to apply delta\"));\n-\thash_object_file(result->data, result->size,\n+\thash_object_file(the_hash_algo, result->data, result->size,\n \t\t\t type_name(delta_obj->real_type), &delta_obj->idx.oid);\n \tsha1_object(result->data, NULL, result->size, delta_obj->real_type,\n \t\t    &delta_obj->idx.oid);\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex bd92dc63b9..b36d17a657 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -409,7 +409,8 @@ static int check_one_mergetag(struct commit *commit,\n \tstruct tag *tag;\n \tint i;\n \n-\thash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &tag_oid);\n+\thash_object_file(the_hash_algo, extra->value, extra->len,\n+\t\t\t type_name(OBJ_TAG), &tag_oid);\n \ttag = lookup_tag(the_repository, &tag_oid);\n \tif (!tag)\n \t\treturn error(_(\"bad mergetag in commit '%s'\"), ref);\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 9100964667..dd4a75e030 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -265,7 +265,8 @@ static void write_object(unsigned nr, enum object_type type,\n \t} else {\n \t\tstruct object *obj;\n \t\tint eaten;\n-\t\thash_object_file(buf, size, type_name(type), &obj_list[nr].oid);\n+\t\thash_object_file(the_hash_algo, buf, size, type_name(type),\n+\t\t\t\t &obj_list[nr].oid);\n \t\tadded_object(nr, type, buf, size);\n \t\tobj = parse_object_buffer(the_repository, &obj_list[nr].oid,\n \t\t\t\t\t  type, size, buf,\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 8c51a15751..a537a806c1 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -407,13 +407,15 @@ static int update_one(struct cache_tree *it,\n \n \tif (repair) {\n \t\tstruct object_id oid;\n-\t\thash_object_file(buffer.buf, buffer.len, tree_type, &oid);\n+\t\thash_object_file(the_hash_algo, buffer.buf, buffer.len,\n+\t\t\t\t tree_type, &oid);\n \t\tif (has_object_file_with_flags(&oid, OBJECT_INFO_SKIP_FETCH_OBJECT))\n \t\t\toidcpy(&it->oid, &oid);\n \t\telse\n \t\t\tto_invalidate = 1;\n \t} else if (dryrun) {\n-\t\thash_object_file(buffer.buf, buffer.len, tree_type, &it->oid);\n+\t\thash_object_file(the_hash_algo, buffer.buf, buffer.len,\n+\t\t\t\t tree_type, &it->oid);\n \t} else if (write_object_file(buffer.buf, buffer.len, tree_type,\n \t\t\t\t     &it->oid)) {\n \t\tstrbuf_release(&buffer);\n@@ -828,7 +830,8 @@ static void verify_one(struct repository *r,\n \t\tstrbuf_addf(&tree_buf, \"%o %.*s%c\", mode, entlen, name, '\\0');\n \t\tstrbuf_add(&tree_buf, oid->hash, r->hash_algo->rawsz);\n \t}\n-\thash_object_file(tree_buf.buf, tree_buf.len, tree_type, &new_oid);\n+\thash_object_file(r->hash_algo, tree_buf.buf, tree_buf.len, tree_type,\n+\t\t\t &new_oid);\n \tif (!oideq(&new_oid, &it->oid))\n \t\tBUG(\"cache-tree for path %.*s does not match. \"\n \t\t    \"Expected %s got %s\", len, path->buf,\ndiff --git a/convert.c b/convert.c\nindex 797e0bd0b1..33507ee0a9 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1146,7 +1146,7 @@ static int ident_to_worktree(const char *src, size_t len,\n \t/* are we \"faking\" in place editing ? */\n \tif (src == buf->buf)\n \t\tto_free = strbuf_detach(buf, NULL);\n-\thash_object_file(src, len, \"blob\", &oid);\n+\thash_object_file(the_hash_algo, src, len, \"blob\", &oid);\n \n \tstrbuf_grow(buf, len + cnt * (the_hash_algo->hexsz + 3));\n \tfor (;;) {\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 531d7adeaf..e189f407af 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -263,8 +263,8 @@ static unsigned int hash_filespec(struct repository *r,\n \tif (!filespec->oid_valid) {\n \t\tif (diff_populate_filespec(r, filespec, 0))\n \t\t\treturn 0;\n-\t\thash_object_file(filespec->data, filespec->size, \"blob\",\n-\t\t\t\t &filespec->oid);\n+\t\thash_object_file(r->hash_algo, filespec->data, filespec->size,\n+\t\t\t\t \"blob\", &filespec->oid);\n \t}\n \treturn oidhash(&filespec->oid);\n }\ndiff --git a/dir.c b/dir.c\nindex 7d255227b1..512c5d7ea6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1002,8 +1002,8 @@ static int add_patterns(const char *fname, const char *base, int baselen,\n \t\t\t\toidcpy(&oid_stat->oid,\n \t\t\t\t       &istate->cache[pos]->oid);\n \t\t\telse\n-\t\t\t\thash_object_file(buf, size, \"blob\",\n-\t\t\t\t\t\t &oid_stat->oid);\n+\t\t\t\thash_object_file(the_hash_algo, buf, size,\n+\t\t\t\t\t\t \"blob\", &oid_stat->oid);\n \t\t\tfill_stat_data(&oid_stat->stat, &st);\n \t\t\toid_stat->valid = 1;\n \t\t}\ndiff --git a/log-tree.c b/log-tree.c\nindex 4e32638de8..cae38dcc66 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -501,7 +501,8 @@ static int show_one_mergetag(struct commit *commit,\n \tint status, nth;\n \tsize_t payload_size, gpg_message_offset;\n \n-\thash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);\n+\thash_object_file(the_hash_algo, extra->value, extra->len,\n+\t\t\t type_name(OBJ_TAG), &oid);\n \ttag = lookup_tag(the_repository, &oid);\n \tif (!tag)\n \t\treturn -1; /* error message already given */\ndiff --git a/object-store.h b/object-store.h\nindex 61b8b13e3b..cf97cbc292 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -198,8 +198,9 @@ static inline void *repo_read_object_file(struct repository *r,\n /* Read and unpack an object file into memory, write memory to an object file */\n int oid_object_info(struct repository *r, const struct object_id *, unsigned long *);\n \n-int hash_object_file(const void *buf, unsigned long len,\n-\t\t     const char *type, struct object_id *oid);\n+int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n+\t\t     unsigned long len, const char *type,\n+\t\t     struct object_id *oid);\n \n int write_object_file(const void *buf, unsigned long len,\n \t\t      const char *type, struct object_id *oid);\ndiff --git a/sha1-file.c b/sha1-file.c\nindex e789bfd153..d545e8b882 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -982,7 +982,7 @@ int check_object_signature(const struct object_id *oid, void *map,\n \tint hdrlen;\n \n \tif (map) {\n-\t\thash_object_file(map, size, type, &real_oid);\n+\t\thash_object_file(the_hash_algo, map, size, type, &real_oid);\n \t\treturn !oideq(oid, &real_oid) ? -1 : 0;\n \t}\n \n@@ -1543,7 +1543,7 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n {\n \tstruct cached_object *co;\n \n-\thash_object_file(buf, len, type_name(type), oid);\n+\thash_object_file(the_hash_algo, buf, len, type_name(type), oid);\n \tif (has_object_file(oid) || find_cached_object(oid))\n \t\treturn 0;\n \tALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);\n@@ -1714,13 +1714,13 @@ static int write_buffer(int fd, const void *buf, size_t len)\n \treturn 0;\n }\n \n-int hash_object_file(const void *buf, unsigned long len, const char *type,\n+int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n+\t\t     unsigned long len, const char *type,\n \t\t     struct object_id *oid)\n {\n \tchar hdr[MAX_HEADER_LEN];\n \tint hdrlen = sizeof(hdr);\n-\twrite_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr,\n-\t\t\t\t  &hdrlen);\n+\twrite_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen);\n \treturn 0;\n }\n \n@@ -2006,7 +2006,8 @@ static int index_mem(struct index_state *istate,\n \tif (write_object)\n \t\tret = write_object_file(buf, size, type_name(type), oid);\n \telse\n-\t\tret = hash_object_file(buf, size, type_name(type), oid);\n+\t\tret = hash_object_file(the_hash_algo, buf, size,\n+\t\t\t\t       type_name(type), oid);\n \tif (re_allocated)\n \t\tfree(buf);\n \treturn ret;\n@@ -2032,8 +2033,8 @@ static int index_stream_convert_blob(struct index_state *istate,\n \t\tret = write_object_file(sbuf.buf, sbuf.len, type_name(OBJ_BLOB),\n \t\t\t\t\toid);\n \telse\n-\t\tret = hash_object_file(sbuf.buf, sbuf.len, type_name(OBJ_BLOB),\n-\t\t\t\t       oid);\n+\t\tret = hash_object_file(the_hash_algo, sbuf.buf, sbuf.len,\n+\t\t\t\t       type_name(OBJ_BLOB), oid);\n \tstrbuf_release(&sbuf);\n \treturn ret;\n }\n@@ -2151,7 +2152,8 @@ int index_path(struct index_state *istate, struct object_id *oid,\n \t\tif (strbuf_readlink(&sb, path, st->st_size))\n \t\t\treturn error_errno(\"readlink(\\\"%s\\\")\", path);\n \t\tif (!(flags & HASH_WRITE_OBJECT))\n-\t\t\thash_object_file(sb.buf, sb.len, blob_type, oid);\n+\t\t\thash_object_file(the_hash_algo, sb.buf, sb.len,\n+\t\t\t\t\t blob_type, oid);\n \t\telse if (write_object_file(sb.buf, sb.len, blob_type, oid))\n \t\t\trc = error(_(\"%s: failed to insert into database\"), path);\n \t\tstrbuf_release(&sb);\n-- \n2.25.0\n\n"},{"id":"390843","messageId":"ab935929bd934ca7a52e69d532d098dfec3508c5.1580413221.git.matheus.bernardino@usp.br","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"[PATCH 7/7] sha1-file: allow check_object_signature() to handle any repo","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-01-30T20:32:23Z","receivedAt":"2020-01-30T20:34:17Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Some callers of check_object_signature() can work on arbitrary\nrepositories, but the repo does not get passed to this function.\nInstead, the_repository is always used internally. To fix possible\ninconsistencies, allow the function to receive a struct repository and\nmake those callers pass on the repo being handled.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n builtin/fast-export.c |  3 ++-\n builtin/index-pack.c  |  5 +++--\n builtin/mktag.c       |  7 +++++--\n cache.h               |  3 ++-\n object.c              |  5 +++--\n pack-check.c          |  2 +-\n sha1-file.c           | 21 +++++++++++----------\n 7 files changed, 27 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex dbec4df92b..25386b34d3 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -293,7 +293,8 @@ static void export_blob(const struct object_id *oid)\n \t\tbuf = read_object_file(oid, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"could not read blob %s\", oid_to_hex(oid));\n-\t\tif (check_object_signature(oid, buf, size, type_name(type)) < 0)\n+\t\tif (check_object_signature(the_repository, oid, buf, size,\n+\t\t\t\t\t   type_name(type)) < 0)\n \t\t\tdie(\"oid mismatch in blob %s\", oid_to_hex(oid));\n \t\tobject = parse_object_buffer(the_repository, oid, type,\n \t\t\t\t\t     size, buf, &eaten);\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0183610a76..acdda17d84 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1384,8 +1384,9 @@ static void fix_unresolved_deltas(struct hashfile *f)\n \t\tif (!base_obj->data)\n \t\t\tcontinue;\n \n-\t\tif (check_object_signature(&d->oid, base_obj->data,\n-\t\t\t\tbase_obj->size, type_name(type)))\n+\t\tif (check_object_signature(the_repository, &d->oid,\n+\t\t\t\t\t   base_obj->data, base_obj->size,\n+\t\t\t\t\t   type_name(type)))\n \t\t\tdie(_(\"local object %s is corrupt\"), oid_to_hex(&d->oid));\n \t\tbase_obj->obj = append_obj_to_pack(f, d->oid.hash,\n \t\t\t\t\tbase_obj->data, base_obj->size, type);\ndiff --git a/builtin/mktag.c b/builtin/mktag.c\nindex 6fb7dc8578..4982d3a93e 100644\n--- a/builtin/mktag.c\n+++ b/builtin/mktag.c\n@@ -29,8 +29,11 @@ static int verify_object(const struct object_id *oid, const char *expected_type)\n \tconst struct object_id *repl = lookup_replace_object(the_repository, oid);\n \n \tif (buffer) {\n-\t\tif (type == type_from_string(expected_type))\n-\t\t\tret = check_object_signature(repl, buffer, size, expected_type);\n+\t\tif (type == type_from_string(expected_type)) {\n+\t\t\tret = check_object_signature(the_repository, repl,\n+\t\t\t\t\t\t     buffer, size,\n+\t\t\t\t\t\t     expected_type);\n+\t\t}\n \t\tfree(buffer);\n \t}\n \treturn ret;\ndiff --git a/cache.h b/cache.h\nindex cbfaead23a..40ae160991 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1363,7 +1363,8 @@ int git_open_cloexec(const char *name, int flags);\n int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);\n int parse_loose_header(const char *hdr, unsigned long *sizep);\n \n-int check_object_signature(const struct object_id *oid, void *buf, unsigned long size, const char *type);\n+int check_object_signature(struct repository *r, const struct object_id *oid,\n+\t\t\t   void *buf, unsigned long size, const char *type);\n \n int finalize_object_file(const char *tmpfile, const char *filename);\n \ndiff --git a/object.c b/object.c\nindex 142ef69399..81f5820fc3 100644\n--- a/object.c\n+++ b/object.c\n@@ -262,7 +262,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)\n \tif ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||\n \t    (!obj && repo_has_object_file(r, oid) &&\n \t     oid_object_info(r, oid, NULL) == OBJ_BLOB)) {\n-\t\tif (check_object_signature(repl, NULL, 0, NULL) < 0) {\n+\t\tif (check_object_signature(r, repl, NULL, 0, NULL) < 0) {\n \t\t\terror(_(\"hash mismatch %s\"), oid_to_hex(oid));\n \t\t\treturn NULL;\n \t\t}\n@@ -272,7 +272,8 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)\n \n \tbuffer = repo_read_object_file(r, oid, &type, &size);\n \tif (buffer) {\n-\t\tif (check_object_signature(repl, buffer, size, type_name(type)) < 0) {\n+\t\tif (check_object_signature(r, repl, buffer, size,\n+\t\t\t\t\t   type_name(type)) < 0) {\n \t\t\tfree(buffer);\n \t\t\terror(_(\"hash mismatch %s\"), oid_to_hex(repl));\n \t\t\treturn NULL;\ndiff --git a/pack-check.c b/pack-check.c\nindex 0fb3b365c7..e4ef71c673 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -144,7 +144,7 @@ static int verify_packfile(struct repository *r,\n \t\t\terr = error(\"cannot unpack %s from %s at offset %\"PRIuMAX\"\",\n \t\t\t\t    oid_to_hex(entries[i].oid.oid), p->pack_name,\n \t\t\t\t    (uintmax_t)entries[i].offset);\n-\t\telse if (check_object_signature(entries[i].oid.oid, data, size, type_name(type)))\n+\t\telse if (check_object_signature(r, entries[i].oid.oid, data, size, type_name(type)))\n \t\t\terr = error(\"packed %s from %s is corrupt\",\n \t\t\t\t    oid_to_hex(entries[i].oid.oid), p->pack_name);\n \t\telse if (fn) {\ndiff --git a/sha1-file.c b/sha1-file.c\nindex d545e8b882..e2f608ac08 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -971,8 +971,8 @@ void *xmmap(void *start, size_t length,\n  * With \"map\" == NULL, try reading the object named with \"oid\" using\n  * the streaming interface and rehash it to do the same.\n  */\n-int check_object_signature(const struct object_id *oid, void *map,\n-\t\t\t   unsigned long size, const char *type)\n+int check_object_signature(struct repository *r, const struct object_id *oid,\n+\t\t\t   void *map, unsigned long size, const char *type)\n {\n \tstruct object_id real_oid;\n \tenum object_type obj_type;\n@@ -982,11 +982,11 @@ int check_object_signature(const struct object_id *oid, void *map,\n \tint hdrlen;\n \n \tif (map) {\n-\t\thash_object_file(the_hash_algo, map, size, type, &real_oid);\n+\t\thash_object_file(r->hash_algo, map, size, type, &real_oid);\n \t\treturn !oideq(oid, &real_oid) ? -1 : 0;\n \t}\n \n-\tst = open_istream(the_repository, oid, &obj_type, &size, NULL);\n+\tst = open_istream(r, oid, &obj_type, &size, NULL);\n \tif (!st)\n \t\treturn -1;\n \n@@ -994,8 +994,8 @@ int check_object_signature(const struct object_id *oid, void *map,\n \thdrlen = xsnprintf(hdr, sizeof(hdr), \"%s %\"PRIuMAX , type_name(obj_type), (uintmax_t)size) + 1;\n \n \t/* Sha1.. */\n-\tthe_hash_algo->init_fn(&c);\n-\tthe_hash_algo->update_fn(&c, hdr, hdrlen);\n+\tr->hash_algo->init_fn(&c);\n+\tr->hash_algo->update_fn(&c, hdr, hdrlen);\n \tfor (;;) {\n \t\tchar buf[1024 * 16];\n \t\tssize_t readlen = read_istream(st, buf, sizeof(buf));\n@@ -1006,9 +1006,9 @@ int check_object_signature(const struct object_id *oid, void *map,\n \t\t}\n \t\tif (!readlen)\n \t\t\tbreak;\n-\t\tthe_hash_algo->update_fn(&c, buf, readlen);\n+\t\tr->hash_algo->update_fn(&c, buf, readlen);\n \t}\n-\tthe_hash_algo->final_fn(real_oid.hash, &c);\n+\tr->hash_algo->final_fn(real_oid.hash, &c);\n \tclose_istream(st);\n \treturn !oideq(oid, &real_oid) ? -1 : 0;\n }\n@@ -2454,8 +2454,9 @@ int read_loose_object(const char *path,\n \t\t\tgit_inflate_end(&stream);\n \t\t\tgoto out;\n \t\t}\n-\t\tif (check_object_signature(expected_oid, *contents,\n-\t\t\t\t\t *size, type_name(*type))) {\n+\t\tif (check_object_signature(the_repository, expected_oid,\n+\t\t\t\t\t   *contents, *size,\n+\t\t\t\t\t   type_name(*type))) {\n \t\t\terror(_(\"hash mismatch for %s (expected %s)\"), path,\n \t\t\t      oid_to_hex(expected_oid));\n \t\t\tfree(*contents);\n-- \n2.25.0\n\n"},{"id":"390845","messageId":"xmqqmua4bpqt.fsf@gitster-ct.c.googlers.com","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"Re: [PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-30T21:26:02Z","receivedAt":"2020-01-30T21:26:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares <matheus.bernardino@usp.br> writes:\n\n> Note: I also tried to replace some uses of the_hash_algo with the struct\n> git_hash_algo from the received repository, for consitency. (In\n> practice, I'm not sure if this is very useful right now, but maybe it\n> will be relevant for the change to SHA256?) Still, many functions in\n> parse_object()'s call chain call oid_to_hex(), which uses the_hash_algo.\n> Since changing this would require a much bigger operation, I decided to\n> leave it as is, for now.\n\nPassing a repo instance throughout the callchain and redusing the\nuse of the_repo would be a worthwhile longer term clean-up.  \n\nMoving off of the_hash_algo to repo->hash_algo is, too, and it is\nvery much relevant to the \"newhash\" work Brian (CCed) has been\nworking on.  You two do not want to step on each other's toes.\nPlease coordinate.\n\nThanks.\n"},{"id":"390875","messageId":"20200130234651.GO4113372@camp.crustytoothpaste.net","threadId":"52720","inReplyTo":"xmqqmua4bpqt.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-01-30T23:46:51Z","receivedAt":"2020-01-30T23:47:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-01-30 at 21:26:02, Junio C Hamano wrote:\n> Matheus Tavares <matheus.bernardino@usp.br> writes:\n> \n> > Note: I also tried to replace some uses of the_hash_algo with the struct\n> > git_hash_algo from the received repository, for consitency. (In\n> > practice, I'm not sure if this is very useful right now, but maybe it\n> > will be relevant for the change to SHA256?) Still, many functions in\n> > parse_object()'s call chain call oid_to_hex(), which uses the_hash_algo.\n> > Since changing this would require a much bigger operation, I decided to\n> > leave it as is, for now.\n> \n> Passing a repo instance throughout the callchain and redusing the\n> use of the_repo would be a worthwhile longer term clean-up.\n\nYes, I think this would be a nice change to see.\n\n> Moving off of the_hash_algo to repo->hash_algo is, too, and it is\n> very much relevant to the \"newhash\" work Brian (CCed) has been\n> working on.  You two do not want to step on each other's toes.\n> Please coordinate.\n\nThis series is fine from my end.  I think it provides a nice improvement\nthat I can take advantage of in the future, so I don't see a reason to\nhold it up on my account.  The few patches I have in this area would be\neasy to rebase or adjust.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"390886","messageId":"20200131090329.GB2857810@coredump.intra.peff.net","threadId":"52720","inReplyTo":"cover.1580413221.git.matheus.bernardino@usp.br","subject":"Re: [PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-31T09:03:29Z","receivedAt":"2020-01-31T09:03:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 30, 2020 at 05:32:16PM -0300, Matheus Tavares wrote:\n\n> The motivation for this patchset is another series I'm working on, to\n> make git-grep stop adding submodules' odbs to the alternates list. With\n> that, parse_object() will have to be called with the subdmodules' struct\n> repository. But it seemed that this function doesn't pass on the\n> received repo to some inner calls, which, instead, always use\n> the_repository. This series seeks to fix these inconsistencies.\n\nI read over the patches and they all seem sensible. There may be spots\nyou missed where these functions are subtly using the_repository under\nthe hood (or where you used the_repository but could have used some\nrepository pointer tucked away in a struct). But even if that is the\ncase, it seems clear that all of the changes are going in the correct\ndirection, and we can continue to iterate on top.\n\n> Note: I also tried to replace some uses of the_hash_algo with the struct\n> git_hash_algo from the received repository, for consitency. (In\n> practice, I'm not sure if this is very useful right now, but maybe it\n> will be relevant for the change to SHA256?) Still, many functions in\n> parse_object()'s call chain call oid_to_hex(), which uses the_hash_algo.\n> Since changing this would require a much bigger operation, I decided to\n> leave it as is, for now.\n\nI'm not sure I look forward to a day where oid_to_hex() needs to take an\nextra parameter, just because it will make it more annoying to use. But\nwe'll have to go there eventually unless we plan to forbid mixing of\nrepositories with different hashes within the same process.\n\nThe hash transition document says:\n\n  To convert recorded submodule pointers, you need to have the converted\n  submodule repository in place. The translation table of the submodule\n  can be used to look up the new hash.\n\nwhich I take to mean that the local copy of the submodule needs to match\nthe superproject hash (this says nothing about what the upstream owner\nof the submodule wants to do; we'd be translating on the fly to the new\nhash in the local repo). So using the_hash_algo would work either way.\n\nI don't think we're particularly interested in supporting multiple\nunrelated repositories within the same process. While that would be\nconvenient for some cases (e.g., you have a million repositories and\nwant to look at all of their trees without creating a million\nprocesses), I don't think it's a goal anyone is really working towards\nwith this \"struct repository\" layer.\n\n> Note II: Despite receiving a repo through the apply_state struct,\n> apply.c:apply_binary() call functions which uses the_repository\n> internally. Because of that, I used the_hash_algo in this function in\n> patch 6. Should I change it to use apply_state->repo->hash_algo\n> anyway?\n\nI think this follows my \"it's OK for now because we're going in the\nright direction from above\". I.e., we probably do eventually want to use\napply_state->repo->hash_algo, just like we probably do eventually want\nall those functions to take a repository argument. But by even using\nthe_hash_algo, you've at least marked the spot for later fixing (the\nvery final step of this long process will be eradicating all of\nthe_hash_algo and the_repository from the bottom of the call-stack on\nup).\n\n-Peff\n"},{"id":"390933","messageId":"xmqqsgjva22g.fsf@gitster-ct.c.googlers.com","threadId":"52720","inReplyTo":"b76b189efb6d3b6e4d345fd1876a676ed27d1653.1580413221.git.matheus.bernardino@usp.br","subject":"Re: [PATCH 4/7] streaming: allow open_istream() to handle any repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-31T18:55:03Z","receivedAt":"2020-01-31T18:55:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares <matheus.bernardino@usp.br> writes:\n\n> Some callers of open_istream() at archive-tar.c and archive-zip.c are\n> capable of working on arbitrary repositories but the repo struct is not\n> passed down to open_istream(), which uses the_repository internally. For\n> now, that's not a problem since the said callers are only being called\n> with the_repository. But to be consistent and avoid future problems,\n> let's allow open_istream() to receive a struct repository and use that\n> instead of the_repository. This parameter addition will also be used in\n> a future patch to make sha1-file.c:check_object_signature() be able to\n> work on arbitrary repos.\n>\n> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n> ---\n\nNicely done.  I am happy to see this change as the inventor of the\nstreaming interface ;-)\n\n>  archive-tar.c          |  6 +++---\n>  archive-zip.c          |  3 ++-\n>  builtin/index-pack.c   |  3 ++-\n>  builtin/pack-objects.c |  3 ++-\n>  sha1-file.c            |  2 +-\n>  streaming.c            | 28 +++++++++++++++-------------\n>  streaming.h            |  4 +++-\n>  7 files changed, 28 insertions(+), 21 deletions(-)\n>\n> diff --git a/archive-tar.c b/archive-tar.c\n> index e16d3f756d..5a77701a15 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -112,7 +112,7 @@ static void write_trailer(void)\n>   * queues up writes, so that all our write(2) calls write exactly one\n>   * full block; pads writes to RECORDSIZE\n>   */\n> -static int stream_blocked(const struct object_id *oid)\n> +static int stream_blocked(struct repository *r, const struct object_id *oid)\n>  {\n>  \tstruct git_istream *st;\n>  \tenum object_type type;\n> @@ -120,7 +120,7 @@ static int stream_blocked(const struct object_id *oid)\n>  \tchar buf[BLOCKSIZE];\n>  \tssize_t readlen;\n>  \n> -\tst = open_istream(oid, &type, &sz, NULL);\n> +\tst = open_istream(r, oid, &type, &sz, NULL);\n>  \tif (!st)\n>  \t\treturn error(_(\"cannot stream blob %s\"), oid_to_hex(oid));\n>  \tfor (;;) {\n> @@ -324,7 +324,7 @@ static int write_tar_entry(struct archiver_args *args,\n>  \t\tif (buffer)\n>  \t\t\twrite_blocked(buffer, size);\n>  \t\telse\n> -\t\t\terr = stream_blocked(oid);\n> +\t\t\terr = stream_blocked(args->repo, oid);\n>  \t}\n>  \tfree(buffer);\n>  \treturn err;\n> diff --git a/archive-zip.c b/archive-zip.c\n> index 11f5b1974b..e9f426298b 100644\n> --- a/archive-zip.c\n> +++ b/archive-zip.c\n> @@ -345,7 +345,8 @@ static int write_zip_entry(struct archiver_args *args,\n>  \n>  \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n>  \t\t    size > big_file_threshold) {\n> -\t\t\tstream = open_istream(oid, &type, &size, NULL);\n> +\t\t\tstream = open_istream(args->repo, oid, &type, &size,\n> +\t\t\t\t\t      NULL);\n>  \t\t\tif (!stream)\n>  \t\t\t\treturn error(_(\"cannot stream blob %s\"),\n>  \t\t\t\t\t     oid_to_hex(oid));\n> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n> index 60a5591039..7a08da8401 100644\n> --- a/builtin/index-pack.c\n> +++ b/builtin/index-pack.c\n> @@ -757,7 +757,8 @@ static int check_collison(struct object_entry *entry)\n>  \n>  \tmemset(&data, 0, sizeof(data));\n>  \tdata.entry = entry;\n> -\tdata.st = open_istream(&entry->idx.oid, &type, &size, NULL);\n> +\tdata.st = open_istream(the_repository, &entry->idx.oid, &type, &size,\n> +\t\t\t       NULL);\n>  \tif (!data.st)\n>  \t\treturn -1;\n>  \tif (size != entry->size || type != entry->type)\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 393c20a2d7..ef17efd94e 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -303,7 +303,8 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n>  \tif (!usable_delta) {\n>  \t\tif (oe_type(entry) == OBJ_BLOB &&\n>  \t\t    oe_size_greater_than(&to_pack, entry, big_file_threshold) &&\n> -\t\t    (st = open_istream(&entry->idx.oid, &type, &size, NULL)) != NULL)\n> +\t\t    (st = open_istream(the_repository, &entry->idx.oid, &type,\n> +\t\t\t\t       &size, NULL)) != NULL)\n>  \t\t\tbuf = NULL;\n>  \t\telse {\n>  \t\t\tbuf = read_object_file(&entry->idx.oid, &type, &size);\n> diff --git a/sha1-file.c b/sha1-file.c\n> index 03ae9ae93a..13b90b1cb1 100644\n> --- a/sha1-file.c\n> +++ b/sha1-file.c\n> @@ -986,7 +986,7 @@ int check_object_signature(const struct object_id *oid, void *map,\n>  \t\treturn !oideq(oid, &real_oid) ? -1 : 0;\n>  \t}\n>  \n> -\tst = open_istream(oid, &obj_type, &size, NULL);\n> +\tst = open_istream(the_repository, oid, &obj_type, &size, NULL);\n>  \tif (!st)\n>  \t\treturn -1;\n>  \n> diff --git a/streaming.c b/streaming.c\n> index fcd6303219..800f07a52c 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -16,6 +16,7 @@ enum input_source {\n>  };\n>  \n>  typedef int (*open_istream_fn)(struct git_istream *,\n> +\t\t\t       struct repository *,\n>  \t\t\t       struct object_info *,\n>  \t\t\t       const struct object_id *,\n>  \t\t\t       enum object_type *);\n> @@ -29,8 +30,8 @@ struct stream_vtbl {\n>  \n>  #define open_method_decl(name) \\\n>  \tint open_istream_ ##name \\\n> -\t(struct git_istream *st, struct object_info *oi, \\\n> -\t const struct object_id *oid, \\\n> +\t(struct git_istream *st, struct repository *r, \\\n> +\t struct object_info *oi, const struct object_id *oid, \\\n>  \t enum object_type *type)\n>  \n>  #define close_method_decl(name) \\\n> @@ -108,7 +109,8 @@ ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n>  \treturn st->vtbl->read(st, buf, sz);\n>  }\n>  \n> -static enum input_source istream_source(const struct object_id *oid,\n> +static enum input_source istream_source(struct repository *r,\n> +\t\t\t\t\tconst struct object_id *oid,\n>  \t\t\t\t\tenum object_type *type,\n>  \t\t\t\t\tstruct object_info *oi)\n>  {\n> @@ -117,7 +119,7 @@ static enum input_source istream_source(const struct object_id *oid,\n>  \n>  \toi->typep = type;\n>  \toi->sizep = &size;\n> -\tstatus = oid_object_info_extended(the_repository, oid, oi, 0);\n> +\tstatus = oid_object_info_extended(r, oid, oi, 0);\n>  \tif (status < 0)\n>  \t\treturn stream_error;\n>  \n> @@ -133,22 +135,23 @@ static enum input_source istream_source(const struct object_id *oid,\n>  \t}\n>  }\n>  \n> -struct git_istream *open_istream(const struct object_id *oid,\n> +struct git_istream *open_istream(struct repository *r,\n> +\t\t\t\t const struct object_id *oid,\n>  \t\t\t\t enum object_type *type,\n>  \t\t\t\t unsigned long *size,\n>  \t\t\t\t struct stream_filter *filter)\n>  {\n>  \tstruct git_istream *st;\n>  \tstruct object_info oi = OBJECT_INFO_INIT;\n> -\tconst struct object_id *real = lookup_replace_object(the_repository, oid);\n> -\tenum input_source src = istream_source(real, type, &oi);\n> +\tconst struct object_id *real = lookup_replace_object(r, oid);\n> +\tenum input_source src = istream_source(r, real, type, &oi);\n>  \n>  \tif (src < 0)\n>  \t\treturn NULL;\n>  \n>  \tst = xmalloc(sizeof(*st));\n> -\tif (open_istream_tbl[src](st, &oi, real, type)) {\n> -\t\tif (open_istream_incore(st, &oi, real, type)) {\n> +\tif (open_istream_tbl[src](st, r, &oi, real, type)) {\n> +\t\tif (open_istream_incore(st, r, &oi, real, type)) {\n>  \t\t\tfree(st);\n>  \t\t\treturn NULL;\n>  \t\t}\n> @@ -338,8 +341,7 @@ static struct stream_vtbl loose_vtbl = {\n>  \n>  static open_method_decl(loose)\n>  {\n> -\tst->u.loose.mapped = map_loose_object(the_repository,\n> -\t\t\t\t\t      oid, &st->u.loose.mapsize);\n> +\tst->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize);\n>  \tif (!st->u.loose.mapped)\n>  \t\treturn -1;\n>  \tif ((unpack_loose_header(&st->z,\n> @@ -499,7 +501,7 @@ static struct stream_vtbl incore_vtbl = {\n>  \n>  static open_method_decl(incore)\n>  {\n> -\tst->u.incore.buf = read_object_file_extended(the_repository, oid, type, &st->size, 0);\n> +\tst->u.incore.buf = read_object_file_extended(r, oid, type, &st->size, 0);\n>  \tst->u.incore.read_ptr = 0;\n>  \tst->vtbl = &incore_vtbl;\n>  \n> @@ -520,7 +522,7 @@ int stream_blob_to_fd(int fd, const struct object_id *oid, struct stream_filter\n>  \tssize_t kept = 0;\n>  \tint result = -1;\n>  \n> -\tst = open_istream(oid, &type, &sz, filter);\n> +\tst = open_istream(the_repository, oid, &type, &sz, filter);\n>  \tif (!st) {\n>  \t\tif (filter)\n>  \t\t\tfree_stream_filter(filter);\n> diff --git a/streaming.h b/streaming.h\n> index f465a3cd31..5e4e6acfd0 100644\n> --- a/streaming.h\n> +++ b/streaming.h\n> @@ -8,7 +8,9 @@\n>  /* opaque */\n>  struct git_istream;\n>  \n> -struct git_istream *open_istream(const struct object_id *, enum object_type *, unsigned long *, struct stream_filter *);\n> +struct git_istream *open_istream(struct repository *, const struct object_id *,\n> +\t\t\t\t enum object_type *, unsigned long *,\n> +\t\t\t\t struct stream_filter *);\n>  int close_istream(struct git_istream *);\n>  ssize_t read_istream(struct git_istream *, void *, size_t);\n"},{"id":"390981","messageId":"CAHd-oW4WBc0AmcZwmOBNNMVQW=JZ-pVhXf8ebL9exgPp13VvOQ@mail.gmail.com","threadId":"52720","inReplyTo":"20200131090329.GB2857810@coredump.intra.peff.net","subject":"Re: [PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-02-01T04:44:04Z","receivedAt":"2020-02-01T04:44:19Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Hi, Peff\n\nThanks for the great feedback.\n\nOn Fri, Jan 31, 2020 at 6:03 AM Jeff King <peff@peff.net> wrote:\n>\n[...]\n> The hash transition document says:\n>\n>   To convert recorded submodule pointers, you need to have the converted\n>   submodule repository in place. The translation table of the submodule\n>   can be used to look up the new hash.\n>\n> which I take to mean that the local copy of the submodule needs to match\n> the superproject hash (this says nothing about what the upstream owner\n> of the submodule wants to do; we'd be translating on the fly to the new\n> hash in the local repo). So using the_hash_algo would work either way.\n>\n> I don't think we're particularly interested in supporting multiple\n> unrelated repositories within the same process. While that would be\n> convenient for some cases (e.g., you have a million repositories and\n> want to look at all of their trees without creating a million\n> processes), I don't think it's a goal anyone is really working towards\n> with this \"struct repository\" layer.\n\nThanks for these explanations. One thing that left me thinking,\nthough, is about changing the_hash_algo to r->hash_algo (not only in\noid_to_hex()). I previously thought this would eventually be required\nfor the hash transition. But if I understood your comment correctly,\nit might not be necessary since the submodule should always match the\nsuperproject hash. And, as you mentioned, supporting multiple\nunrelated repos [in a single process] is not one of the main goals, as\nwell. So wouldn't keep using the_hash_algo be OK?\n"},{"id":"390983","messageId":"20200201100337.u4ikzmrprca4ltvb@tb-raspi4","threadId":"52720","inReplyTo":"f256d2058736091c3d9662788762849e8df794b0.1580413221.git.matheus.bernardino@usp.br","subject":"Re: [PATCH 5/7] sha1-file: pass git_hash_algo to write_object_file_prepare()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2020-02-01T10:03:37Z","receivedAt":"2020-02-01T10:04:05Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Thu, Jan 30, 2020 at 05:32:21PM -0300, Matheus Tavares wrote:\n> Allow write_object_file_prepare() to receive arbitrary 'struct\n> git_hash_algo's instead of always using the_hash_algo. The added\n> parameter will be used in the next commit to make hash_object_file() be\n> able to work with arbitrary git_hash_algo's, as well.\n>\n> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n> ---\n>  sha1-file.c | 20 ++++++++++++--------\n>  1 file changed, 12 insertions(+), 8 deletions(-)\n>\n> diff --git a/sha1-file.c b/sha1-file.c\n> index 13b90b1cb1..e789bfd153 100644\n> --- a/sha1-file.c\n> +++ b/sha1-file.c\n> @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r,\n>  \t}\n>  }\n>\n> -static void write_object_file_prepare(const void *buf, unsigned long len,\n> +static void write_object_file_prepare(const struct git_hash_algo *algo,\n> +\t\t\t\t      const void *buf, unsigned long len,\n>  \t\t\t\t      const char *type, struct object_id *oid,\n>  \t\t\t\t      char *hdr, int *hdrlen)\n>  {\n\nOne minor minor suggestion/nit, may be my own type of style only.\n\nWhen adding a parameter to a function, we typically add it at the end of\nthe parameter list, rather than at the beginning.\nThe other (unwritten) convention, when dealing with \"buffer pointer/len\",\nseams to be that buffer-ptr is the first paramter and len is is the second.\n\nHowever, this is my personal note, other opinions and comments are welcome.\nWhat do others think ?\n\n\n"},{"id":"390985","messageId":"20200201110401.GA1864948@coredump.intra.peff.net","threadId":"52720","inReplyTo":"CAHd-oW4WBc0AmcZwmOBNNMVQW=JZ-pVhXf8ebL9exgPp13VvOQ@mail.gmail.com","subject":"Re: [PATCH 0/7] fix inconsistent uses of the_repo in parse_object()'s call chain","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-01T11:04:01Z","receivedAt":"2020-02-01T11:04:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 01, 2020 at 01:44:04AM -0300, Matheus Tavares Bernardino wrote:\n\n> > I don't think we're particularly interested in supporting multiple\n> > unrelated repositories within the same process. While that would be\n> > convenient for some cases (e.g., you have a million repositories and\n> > want to look at all of their trees without creating a million\n> > processes), I don't think it's a goal anyone is really working towards\n> > with this \"struct repository\" layer.\n> \n> Thanks for these explanations. One thing that left me thinking,\n> though, is about changing the_hash_algo to r->hash_algo (not only in\n> oid_to_hex()). I previously thought this would eventually be required\n> for the hash transition. But if I understood your comment correctly,\n> it might not be necessary since the submodule should always match the\n> superproject hash. And, as you mentioned, supporting multiple\n> unrelated repos [in a single process] is not one of the main goals, as\n> well. So wouldn't keep using the_hash_algo be OK?\n\nYes, by the reasoning in my message, it wouldn't be a big problem. But I\nliterally hadn't thought about it until writing that, so I may either be\nmissing something, or there may be plans from other folks working on the\nhash transition that contradict that.\n\nEven if we don't care too much about it for now, though, it still seems\nlike a step in the right direction.\n\n-Peff\n"},{"id":"390986","messageId":"20200201110836.GB1864948@coredump.intra.peff.net","threadId":"52720","inReplyTo":"20200201100337.u4ikzmrprca4ltvb@tb-raspi4","subject":"Re: [PATCH 5/7] sha1-file: pass git_hash_algo to write_object_file_prepare()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-01T11:08:36Z","receivedAt":"2020-02-01T11:08:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 01, 2020 at 11:03:37AM +0100, Torsten Bögershausen wrote:\n\n> > diff --git a/sha1-file.c b/sha1-file.c\n> > index 13b90b1cb1..e789bfd153 100644\n> > --- a/sha1-file.c\n> > +++ b/sha1-file.c\n> > @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r,\n> >  \t}\n> >  }\n> >\n> > -static void write_object_file_prepare(const void *buf, unsigned long len,\n> > +static void write_object_file_prepare(const struct git_hash_algo *algo,\n> > +\t\t\t\t      const void *buf, unsigned long len,\n> >  \t\t\t\t      const char *type, struct object_id *oid,\n> >  \t\t\t\t      char *hdr, int *hdrlen)\n> >  {\n> \n> One minor minor suggestion/nit, may be my own type of style only.\n> \n> When adding a parameter to a function, we typically add it at the end of\n> the parameter list, rather than at the beginning.\n> The other (unwritten) convention, when dealing with \"buffer pointer/len\",\n> seams to be that buffer-ptr is the first paramter and len is is the second.\n\nI'd usually add new options at the end, too, all things being equal. But\nin this case, I see:\n\n  - some of these are input parameters and some are output (I think oid,\n    hdr, and hdrlen are the latter); it makes sense to keep them grouped\n    that way\n\n  - in object-oriented functions, we'd often take the object first\n    (e.g., strbuf_add). In non-object-oriented functions, we often take\n    a \"context\" argument first that shapes how the function works (e.g.,\n    all of the pp_* functions that take a pretty_print_context). I'd\n    think of the algorithm a little bit as a \"context\" argument, though\n    I admit it's mostly a gut feeling and quite subjective.\n\n    But if you believe that line of reasoning, then it doesn't seem out\n    of place as the first argument.\n\n-Peff\n"}]}