{"thread":{"id":"46371","subject":"[PATCH 1/2] commit: convert lookup_commit_graft to struct object_id","startedAt":"2017-07-13T00:44:36Z","lastAt":"2017-07-13T23:45:19Z","messageCount":10,"participants":["Stefan Beller","brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"324330","messageId":"20170713004415.5051-1-sbeller@google.com","threadId":"46371","inReplyTo":null,"subject":"[PATCH 1/2] commit: convert lookup_commit_graft to struct object_id","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-13T00:44:14Z","receivedAt":"2017-07-13T00:44:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With this patch, commit.h doesn't contain the string 'sha1' any more.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nBefore diving into the \"RFC object store\" series further, I want to get\nrid of the final sha1s in {commit,tag}.{c,h}.\n\n commit.c  | 6 +++---\n commit.h  | 2 +-\n fsck.c    | 2 +-\n shallow.c | 4 ++--\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex cbfd689939..e0888cf0f7 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -199,11 +199,11 @@ static void prepare_commit_graft(void)\n \tcommit_graft_prepared = 1;\n }\n \n-struct commit_graft *lookup_commit_graft(const unsigned char *sha1)\n+struct commit_graft *lookup_commit_graft(const struct object_id *oid)\n {\n \tint pos;\n \tprepare_commit_graft();\n-\tpos = commit_graft_pos(sha1);\n+\tpos = commit_graft_pos(oid->hash);\n \tif (pos < 0)\n \t\treturn NULL;\n \treturn commit_graft[pos];\n@@ -335,7 +335,7 @@ int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long s\n \tbufptr += tree_entry_len + 1; /* \"tree \" + \"hex sha1\" + \"\\n\" */\n \tpptr = &item->parents;\n \n-\tgraft = lookup_commit_graft(item->object.oid.hash);\n+\tgraft = lookup_commit_graft(&item->object.oid);\n \twhile (bufptr + parent_entry_len < tail && !memcmp(bufptr, \"parent \", 7)) {\n \t\tstruct commit *new_parent;\n \ndiff --git a/commit.h b/commit.h\nindex 4127c298cb..6d857f06c1 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -249,7 +249,7 @@ typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n \n struct commit_graft *read_graft_line(char *buf, int len);\n int register_commit_graft(struct commit_graft *, int);\n-struct commit_graft *lookup_commit_graft(const unsigned char *sha1);\n+struct commit_graft *lookup_commit_graft(const struct object_id *oid);\n \n extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);\n extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos);\ndiff --git a/fsck.c b/fsck.c\nindex b4204d772b..2d2d2e9432 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -736,7 +736,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,\n \t\tbuffer += 41;\n \t\tparent_line_count++;\n \t}\n-\tgraft = lookup_commit_graft(commit->object.oid.hash);\n+\tgraft = lookup_commit_graft(&commit->object.oid);\n \tparent_count = commit_list_count(commit->parents);\n \tif (graft) {\n \t\tif (graft->nr_parent == -1 && !parent_count)\ndiff --git a/shallow.c b/shallow.c\nindex 54359d5490..f5591e56da 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -107,7 +107,7 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n \t\tcur_depth++;\n \t\tif ((depth != INFINITE_DEPTH && cur_depth >= depth) ||\n \t\t    (is_repository_shallow() && !commit->parents &&\n-\t\t     (graft = lookup_commit_graft(commit->object.oid.hash)) != NULL &&\n+\t\t     (graft = lookup_commit_graft(&commit->object.oid)) != NULL &&\n \t\t     graft->nr_parent < 0)) {\n \t\t\tcommit_list_insert(commit, &result);\n \t\t\tcommit->object.flags |= shallow_flag;\n@@ -398,7 +398,7 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa)\n \tfor (i = 0; i < sa->nr; i++) {\n \t\tif (has_object_file(sa->oid + i)) {\n \t\t\tstruct commit_graft *graft;\n-\t\t\tgraft = lookup_commit_graft(sa->oid[i].hash);\n+\t\t\tgraft = lookup_commit_graft(&sa->oid[i]);\n \t\t\tif (graft && graft->nr_parent < 0)\n \t\t\t\tcontinue;\n \t\t\tinfo->ours[info->nr_ours++] = i;\n-- \n2.13.3.650.g5ebc568d5d\n\n"},{"id":"324331","messageId":"20170713004415.5051-2-sbeller@google.com","threadId":"46371","inReplyTo":"20170713004415.5051-1-sbeller@google.com","subject":"[PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-13T00:44:15Z","receivedAt":"2017-07-13T00:45:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/tag.c        |  2 +-\n builtin/verify-tag.c |  9 +++++----\n tag.c                | 10 +++++-----\n tag.h                |  2 +-\n 4 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 01154ea8dc..b25bf8daa2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -111,7 +111,7 @@ static int verify_tag(const char *name, const char *ref,\n \tif (fmt_pretty)\n \t\tflags = GPG_VERIFY_OMIT_STATUS;\n \n-\tif (gpg_verify_tag(oid->hash, name, flags))\n+\tif (gpg_verify_tag(oid, name, flags))\n \t\treturn -1;\n \n \tif (fmt_pretty)\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex f9a5f7535a..ed8329340f 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t}\n \n \twhile (i < argc) {\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id oid;\n \t\tconst char *name = argv[i++];\n-\t\tif (get_sha1(name, sha1)) {\n+\n+\t\tif (get_oid(name, &oid)) {\n \t\t\thad_error = !!error(\"tag '%s' not found.\", name);\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (gpg_verify_tag(sha1, name, flags)) {\n+\t\tif (gpg_verify_tag(&oid, name, flags)) {\n \t\t\thad_error = 1;\n \t\t\tcontinue;\n \t\t}\n \n \t\tif (fmt_pretty)\n-\t\t\tpretty_print_ref(name, sha1, fmt_pretty);\n+\t\t\tpretty_print_ref(name, oid.hash, fmt_pretty);\n \t}\n \treturn had_error;\n }\ndiff --git a/tag.c b/tag.c\nindex 47f60ae151..7e10acfb6e 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -33,7 +33,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \treturn ret;\n }\n \n-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n+int gpg_verify_tag(const struct object_id *oid, const char *name_to_report,\n \t\tunsigned flags)\n {\n \tenum object_type type;\n@@ -41,20 +41,20 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n \tunsigned long size;\n \tint ret;\n \n-\ttype = sha1_object_info(sha1, NULL);\n+\ttype = sha1_object_info(oid->hash, NULL);\n \tif (type != OBJ_TAG)\n \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n \t\t\t\tname_to_report ?\n \t\t\t\tname_to_report :\n-\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n+\t\t\t\tfind_unique_abbrev(oid->hash, DEFAULT_ABBREV),\n \t\t\t\ttypename(type));\n \n-\tbuf = read_sha1_file(sha1, &type, &size);\n+\tbuf = read_sha1_file(oid->hash, &type, &size);\n \tif (!buf)\n \t\treturn error(\"%s: unable to read file.\",\n \t\t\t\tname_to_report ?\n \t\t\t\tname_to_report :\n-\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\t\t\t\tfind_unique_abbrev(oid->hash, DEFAULT_ABBREV));\n \n \tret = run_gpg_verify(buf, size, flags);\n \ndiff --git a/tag.h b/tag.h\nindex fdfcb4a84a..d469534e82 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si\n extern int parse_tag(struct tag *item);\n extern struct object *deref_tag(struct object *, const char *, int);\n extern struct object *deref_tag_noverify(struct object *);\n-extern int gpg_verify_tag(const unsigned char *sha1,\n+extern int gpg_verify_tag(const struct object_id *oid,\n \t\tconst char *name_to_report, unsigned flags);\n \n #endif /* TAG_H */\n-- \n2.13.3.650.g5ebc568d5d\n\n"},{"id":"324333","messageId":"20170713022630.o6ioe6xnoe5wtllf@genre.crustytoothpaste.net","threadId":"46371","inReplyTo":"20170713004415.5051-1-sbeller@google.com","subject":"Re: [PATCH 1/2] commit: convert lookup_commit_graft to struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-13T02:26:30Z","receivedAt":"2017-07-13T02:26:43Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Jul 12, 2017 at 05:44:14PM -0700, Stefan Beller wrote:\n> With this patch, commit.h doesn't contain the string 'sha1' any more.\n\nFrom a relatively quick look, these look sane to me.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"324411","messageId":"xmqq60ewxj5f.fsf@gitster.mtv.corp.google.com","threadId":"46371","inReplyTo":"20170713004415.5051-1-sbeller@google.com","subject":"Re: [PATCH 1/2] commit: convert lookup_commit_graft to struct object_id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T18:54:04Z","receivedAt":"2017-07-13T18:54:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> With this patch, commit.h doesn't contain the string 'sha1' any more.\n\n;-)  Nice.\n\ncommit_graft_pos() still thinks we only deal with SHA-1, but that\nneeds to wait for oid_pos().  The function has only two callers that\ndo not pass X->oid.hash so it may be a good candidate to convert.\n\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n> Before diving into the \"RFC object store\" series further, I want to get\n> rid of the final sha1s in {commit,tag}.{c,h}.\n>\n>  commit.c  | 6 +++---\n>  commit.h  | 2 +-\n>  fsck.c    | 2 +-\n>  shallow.c | 4 ++--\n>  4 files changed, 7 insertions(+), 7 deletions(-)\n>\n> diff --git a/commit.c b/commit.c\n> index cbfd689939..e0888cf0f7 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -199,11 +199,11 @@ static void prepare_commit_graft(void)\n>  \tcommit_graft_prepared = 1;\n>  }\n>  \n> -struct commit_graft *lookup_commit_graft(const unsigned char *sha1)\n> +struct commit_graft *lookup_commit_graft(const struct object_id *oid)\n>  {\n>  \tint pos;\n>  \tprepare_commit_graft();\n> -\tpos = commit_graft_pos(sha1);\n> +\tpos = commit_graft_pos(oid->hash);\n>  \tif (pos < 0)\n>  \t\treturn NULL;\n>  \treturn commit_graft[pos];\n> @@ -335,7 +335,7 @@ int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long s\n>  \tbufptr += tree_entry_len + 1; /* \"tree \" + \"hex sha1\" + \"\\n\" */\n>  \tpptr = &item->parents;\n>  \n> -\tgraft = lookup_commit_graft(item->object.oid.hash);\n> +\tgraft = lookup_commit_graft(&item->object.oid);\n>  \twhile (bufptr + parent_entry_len < tail && !memcmp(bufptr, \"parent \", 7)) {\n>  \t\tstruct commit *new_parent;\n>  \n> diff --git a/commit.h b/commit.h\n> index 4127c298cb..6d857f06c1 100644\n> --- a/commit.h\n> +++ b/commit.h\n> @@ -249,7 +249,7 @@ typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n>  \n>  struct commit_graft *read_graft_line(char *buf, int len);\n>  int register_commit_graft(struct commit_graft *, int);\n> -struct commit_graft *lookup_commit_graft(const unsigned char *sha1);\n> +struct commit_graft *lookup_commit_graft(const struct object_id *oid);\n>  \n>  extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);\n>  extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos);\n> diff --git a/fsck.c b/fsck.c\n> index b4204d772b..2d2d2e9432 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -736,7 +736,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,\n>  \t\tbuffer += 41;\n>  \t\tparent_line_count++;\n>  \t}\n> -\tgraft = lookup_commit_graft(commit->object.oid.hash);\n> +\tgraft = lookup_commit_graft(&commit->object.oid);\n>  \tparent_count = commit_list_count(commit->parents);\n>  \tif (graft) {\n>  \t\tif (graft->nr_parent == -1 && !parent_count)\n> diff --git a/shallow.c b/shallow.c\n> index 54359d5490..f5591e56da 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -107,7 +107,7 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n>  \t\tcur_depth++;\n>  \t\tif ((depth != INFINITE_DEPTH && cur_depth >= depth) ||\n>  \t\t    (is_repository_shallow() && !commit->parents &&\n> -\t\t     (graft = lookup_commit_graft(commit->object.oid.hash)) != NULL &&\n> +\t\t     (graft = lookup_commit_graft(&commit->object.oid)) != NULL &&\n>  \t\t     graft->nr_parent < 0)) {\n>  \t\t\tcommit_list_insert(commit, &result);\n>  \t\t\tcommit->object.flags |= shallow_flag;\n> @@ -398,7 +398,7 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa)\n>  \tfor (i = 0; i < sa->nr; i++) {\n>  \t\tif (has_object_file(sa->oid + i)) {\n>  \t\t\tstruct commit_graft *graft;\n> -\t\t\tgraft = lookup_commit_graft(sa->oid[i].hash);\n> +\t\t\tgraft = lookup_commit_graft(&sa->oid[i]);\n>  \t\t\tif (graft && graft->nr_parent < 0)\n>  \t\t\t\tcontinue;\n>  \t\t\tinfo->ours[info->nr_ours++] = i;\n"},{"id":"324413","messageId":"xmqqzic8w472.fsf@gitster.mtv.corp.google.com","threadId":"46371","inReplyTo":"20170713004415.5051-2-sbeller@google.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T19:02:25Z","receivedAt":"2017-07-13T19:02:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>  \t\tif (fmt_pretty)\n> -\t\t\tpretty_print_ref(name, sha1, fmt_pretty);\n> +\t\t\tpretty_print_ref(name, oid.hash, fmt_pretty);\n\nThe next step would be to have pretty_print_ref() to take an oid; as\nthere are only two callers to it, both of which pass oid->hash to it,\nthat should be a small and conflict-free conversion.\n\n>  \n> -\ttype = sha1_object_info(sha1, NULL);\n> +\ttype = sha1_object_info(oid->hash, NULL);\n\nsha1_object_info() has a handful of callers that do not pass the\npointer to the hash field in an existing oid object, but it does not\nlook too bad as a candidate for conversion.\n\n>  \tif (!buf)\n>  \t\treturn error(\"%s: unable to read file.\",\n>  \t\t\t\tname_to_report ?\n>  \t\t\t\tname_to_report :\n> -\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\t\t\t\tfind_unique_abbrev(oid->hash, DEFAULT_ABBREV));\n\nSo does find_unique_abbrev().\n\nOverall both patches look good.  Thanks.\n"},{"id":"324445","messageId":"xmqq4lugukju.fsf@gitster.mtv.corp.google.com","threadId":"46371","inReplyTo":"20170713004415.5051-2-sbeller@google.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T20:52:05Z","receivedAt":"2017-07-13T20:52:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> index f9a5f7535a..ed8329340f 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>  \t}\n>  \n>  \twhile (i < argc) {\n> -\t\tunsigned char sha1[20];\n> +\t\tstruct object_id oid;\n>  \t\tconst char *name = argv[i++];\n> -\t\tif (get_sha1(name, sha1)) {\n> +\n> +\t\tif (get_oid(name, &oid)) {\n>  \t\t\thad_error = !!error(\"tag '%s' not found.\", name);\n>  \t\t\tcontinue;\n>  \t\t}\n\nThis part is already done, it seems, in bc/object-id topic, even\nthough other parts are not yet done?\n\n>  \n> -\t\tif (gpg_verify_tag(sha1, name, flags)) {\n> +\t\tif (gpg_verify_tag(&oid, name, flags)) {\n>  \t\t\thad_error = 1;\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n>  \t\tif (fmt_pretty)\n> -\t\t\tpretty_print_ref(name, sha1, fmt_pretty);\n> +\t\t\tpretty_print_ref(name, oid.hash, fmt_pretty);\n>  \t}\n>  \treturn had_error;\n>  }\n> diff --git a/tag.c b/tag.c\n> index 47f60ae151..7e10acfb6e 100644\n> --- a/tag.c\n> +++ b/tag.c\n> @@ -33,7 +33,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n>  \treturn ret;\n>  }\n>  \n> -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n> +int gpg_verify_tag(const struct object_id *oid, const char *name_to_report,\n>  \t\tunsigned flags)\n>  {\n>  \tenum object_type type;\n> @@ -41,20 +41,20 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n>  \tunsigned long size;\n>  \tint ret;\n>  \n> -\ttype = sha1_object_info(sha1, NULL);\n> +\ttype = sha1_object_info(oid->hash, NULL);\n>  \tif (type != OBJ_TAG)\n>  \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n>  \t\t\t\tname_to_report ?\n>  \t\t\t\tname_to_report :\n> -\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n> +\t\t\t\tfind_unique_abbrev(oid->hash, DEFAULT_ABBREV),\n>  \t\t\t\ttypename(type));\n>  \n> -\tbuf = read_sha1_file(sha1, &type, &size);\n> +\tbuf = read_sha1_file(oid->hash, &type, &size);\n>  \tif (!buf)\n>  \t\treturn error(\"%s: unable to read file.\",\n>  \t\t\t\tname_to_report ?\n>  \t\t\t\tname_to_report :\n> -\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\t\t\t\tfind_unique_abbrev(oid->hash, DEFAULT_ABBREV));\n>  \n>  \tret = run_gpg_verify(buf, size, flags);\n>  \n> diff --git a/tag.h b/tag.h\n> index fdfcb4a84a..d469534e82 100644\n> --- a/tag.h\n> +++ b/tag.h\n> @@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si\n>  extern int parse_tag(struct tag *item);\n>  extern struct object *deref_tag(struct object *, const char *, int);\n>  extern struct object *deref_tag_noverify(struct object *);\n> -extern int gpg_verify_tag(const unsigned char *sha1,\n> +extern int gpg_verify_tag(const struct object_id *oid,\n>  \t\tconst char *name_to_report, unsigned flags);\n>  \n>  #endif /* TAG_H */\n"},{"id":"324448","messageId":"CAGZ79kb3_K8NxpHuzAdHhSLWUcbw_X3qVVspPeuSNA4sAeT-Kw@mail.gmail.com","threadId":"46371","inReplyTo":"xmqq4lugukju.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-13T21:00:24Z","receivedAt":"2017-07-13T21:00:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jul 13, 2017 at 1:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n>> index f9a5f7535a..ed8329340f 100644\n>> --- a/builtin/verify-tag.c\n>> +++ b/builtin/verify-tag.c\n>> @@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>>       }\n>>\n>>       while (i < argc) {\n>> -             unsigned char sha1[20];\n>> +             struct object_id oid;\n>>               const char *name = argv[i++];\n>> -             if (get_sha1(name, sha1)) {\n>> +\n>> +             if (get_oid(name, &oid)) {\n>>                       had_error = !!error(\"tag '%s' not found.\", name);\n>>                       continue;\n>>               }\n>\n> This part is already done, it seems, in bc/object-id topic, even\n> though other parts are not yet done?\n\nOops. I assumed the latest bc/object-id would have been in master\nalready, but after checking it is not. 967635dc3c2\n(builtin/verify-tag: convert to struct object_id)\nconverts this part, although there are 2 differences:\n* I added a stray newline before get_oid\n* The argument to gpg_verify_tag is a sha1 or oid\n\nSo yes, this produces a merge conflict. :/\n\nThere rest (tag.{c,h}, builtin/tag.c) is not found in brians series.\n"},{"id":"324451","messageId":"xmqqvamwt4ju.fsf@gitster.mtv.corp.google.com","threadId":"46371","inReplyTo":"CAGZ79kb3_K8NxpHuzAdHhSLWUcbw_X3qVVspPeuSNA4sAeT-Kw@mail.gmail.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T21:23:01Z","receivedAt":"2017-07-13T21:23:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> On Thu, Jul 13, 2017 at 1:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n>>> index f9a5f7535a..ed8329340f 100644\n>>> --- a/builtin/verify-tag.c\n>>> +++ b/builtin/verify-tag.c\n>>> @@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>>>       }\n>>>\n>>>       while (i < argc) {\n>>> -             unsigned char sha1[20];\n>>> +             struct object_id oid;\n>>>               const char *name = argv[i++];\n>>> -             if (get_sha1(name, sha1)) {\n>>> +\n>>> +             if (get_oid(name, &oid)) {\n>>>                       had_error = !!error(\"tag '%s' not found.\", name);\n>>>                       continue;\n>>>               }\n>>\n>> This part is already done, it seems, in bc/object-id topic, even\n>> though other parts are not yet done?\n>\n> Oops. I assumed the latest bc/object-id would have been in master\n> already, but after checking it is not. 967635dc3c2\n> (builtin/verify-tag: convert to struct object_id)\n> converts this part, although there are 2 differences:\n> * I added a stray newline before get_oid\n> * The argument to gpg_verify_tag is a sha1 or oid\n>\n> So yes, this produces a merge conflict. :/\n\nThat is OK.  This actually shouldn't create any meaningful conflict.\nBoth try to do the same code, with only a blank-line difference.\n\nAs Brian said bc/object-id would be rerolled, I was wondering if I\nshould queue these two patches (even though I already queued them)\nmyself, or it would be better for you to send them to Brian to make\nit part of his series.\n"},{"id":"324454","messageId":"CAGZ79kZOCtk9YM0=dUic5MHMWLZgb8xeYsFX0VaMNbFrjw=sUA@mail.gmail.com","threadId":"46371","inReplyTo":"xmqqvamwt4ju.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-13T21:49:28Z","receivedAt":"2017-07-13T21:49:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jul 13, 2017 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> On Thu, Jul 13, 2017 at 1:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Stefan Beller <sbeller@google.com> writes:\n>>>\n>>>> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n>>>> index f9a5f7535a..ed8329340f 100644\n>>>> --- a/builtin/verify-tag.c\n>>>> +++ b/builtin/verify-tag.c\n>>>> @@ -56,20 +56,21 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>>>>       }\n>>>>\n>>>>       while (i < argc) {\n>>>> -             unsigned char sha1[20];\n>>>> +             struct object_id oid;\n>>>>               const char *name = argv[i++];\n>>>> -             if (get_sha1(name, sha1)) {\n>>>> +\n>>>> +             if (get_oid(name, &oid)) {\n>>>>                       had_error = !!error(\"tag '%s' not found.\", name);\n>>>>                       continue;\n>>>>               }\n>>>\n>>> This part is already done, it seems, in bc/object-id topic, even\n>>> though other parts are not yet done?\n>>\n>> Oops. I assumed the latest bc/object-id would have been in master\n>> already, but after checking it is not. 967635dc3c2\n>> (builtin/verify-tag: convert to struct object_id)\n>> converts this part, although there are 2 differences:\n>> * I added a stray newline before get_oid\n>> * The argument to gpg_verify_tag is a sha1 or oid\n>>\n>> So yes, this produces a merge conflict. :/\n>\n> That is OK.  This actually shouldn't create any meaningful conflict.\n> Both try to do the same code, with only a blank-line difference.\n>\n> As Brian said bc/object-id would be rerolled, I was wondering if I\n> should queue these two patches (even though I already queued them)\n> myself, or it would be better for you to send them to Brian to make\n> it part of his series.\n\n+cc Brian\n\nSnarkily I wanted to link to an essay \"large patch series\nconsidered harmful\"[1], but could not find any. So a couple\nof bullet points:\n(a) humans dislike larger series, hence fewer reviewers\n(b) the likelihood of a mistake in new code is proportional to its size\n  We can use the number of patches as a proxy for size\n(c) If a mistake is found, the whole series needs rerolling.\n  The effort of rerolling a series can be approximated with\n  its size as well.\n\nFrom combining (b) and (c), we conclude that the effort to\nland a patch series is O(n^2) with n as the number of patches.\nAlso from (a) we conclude that two smaller series containing\nthe same output as one large series, has better code quality.\nSo with that, we conclude that all series shall be as small\nas possible.\n\nSo I'd ask to queue these 2 separately, asking Brian to drop\n\"builtin/verify-tag: convert to struct object_id\"\n\n[1] https://www.cs.princeton.edu/~dpw/papers/hotos.pdf, 2005\n    seems interesting to me in hindsight.\n\nI can also send my patches to Brian, as you (both) like.\n\nThanks,\nStefan\n"},{"id":"324468","messageId":"20170713234509.ik334vp4l5icblkr@genre.crustytoothpaste.net","threadId":"46371","inReplyTo":"CAGZ79kZOCtk9YM0=dUic5MHMWLZgb8xeYsFX0VaMNbFrjw=sUA@mail.gmail.com","subject":"Re: [PATCH 2/2] tag: convert gpg_verify_tag to use struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-13T23:45:09Z","receivedAt":"2017-07-13T23:45:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jul 13, 2017 at 02:49:28PM -0700, Stefan Beller wrote:\n> Snarkily I wanted to link to an essay \"large patch series\n> considered harmful\"[1], but could not find any. So a couple\n> of bullet points:\n> (a) humans dislike larger series, hence fewer reviewers\n> (b) the likelihood of a mistake in new code is proportional to its size\n>   We can use the number of patches as a proxy for size\n> (c) If a mistake is found, the whole series needs rerolling.\n>   The effort of rerolling a series can be approximated with\n>   its size as well.\n> \n> From combining (b) and (c), we conclude that the effort to\n> land a patch series is O(n^2) with n as the number of patches.\n> Also from (a) we conclude that two smaller series containing\n> the same output as one large series, has better code quality.\n> So with that, we conclude that all series shall be as small\n> as possible.\n> \n> So I'd ask to queue these 2 separately, asking Brian to drop\n> \"builtin/verify-tag: convert to struct object_id\"\n> \n> [1] https://www.cs.princeton.edu/~dpw/papers/hotos.pdf, 2005\n>     seems interesting to me in hindsight.\n> \n> I can also send my patches to Brian, as you (both) like.\n\nI'm literally about to send my series out; I rebased and tested it last\nnight.  I don't care whose patch gets applied, but to avoid needing to\nrebase and retest my series, I'm going to send it out as it is.  Junio\ncan apply my series on top of yours, in which case he can drop the\nrelevant patch from my series.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"}]}