{"thread":{"id":"28141","subject":"[PATCH] log: decorate grafted commits with \"grafted\"","startedAt":"2011-08-17T15:02:06Z","lastAt":"2011-08-19T12:43:50Z","messageCount":14,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"173677","messageId":"1313593326-12077-1-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":null,"subject":"[PATCH] log: decorate grafted commits with \"grafted\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-17T15:02:06Z","receivedAt":"2011-08-17T15:02:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This would help decide whether to deepen some more on shallow\nrepositories, or the branch really has really ends there.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n I briefly considered wrapping _() around \"grafted\" text, but\n decided that making it a keyword might help searching\n\n log-tree.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex e945701..c469341 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -18,6 +18,7 @@ enum decoration_type {\n \tDECORATION_REF_TAG,\n \tDECORATION_REF_STASH,\n \tDECORATION_REF_HEAD,\n+\tDECORATION_GRAFTED\n };\n \n static char decoration_colors[][COLOR_MAXLEN] = {\n@@ -27,6 +28,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_BOLD_YELLOW,\t/* REF_TAG */\n \tGIT_COLOR_BOLD_MAGENTA,\t/* REF_STASH */\n \tGIT_COLOR_BOLD_CYAN,\t/* REF_HEAD */\n+\tGIT_COLOR_BOLD_BLUE,\t/* GRAFTED */\n };\n \n static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)\n@@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)\n \tlog.parent = NULL;\n \topt->loginfo = &log;\n \n+\tif (!commit->parents && lookup_commit_graft(commit->object.sha1))\n+\t\tadd_name_decoration(DECORATION_GRAFTED, \"grafted\",\n+\t\t\t\t    &commit->object);\n \tshown = log_tree_diff(opt, commit, &log);\n \tif (!shown && opt->loginfo && opt->always_show_header) {\n \t\tlog.parent = NULL;\n-- \n1.7.4.74.g639db\n"},{"id":"173687","messageId":"7v8vqreuwk.fsf@alter.siamese.dyndns.org","threadId":"28141","inReplyTo":"1313593326-12077-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] log: decorate grafted commits with \"grafted\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-17T18:48:43Z","receivedAt":"2011-08-17T18:48:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> @@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)\n>  \tlog.parent = NULL;\n>  \topt->loginfo = &log;\n>  \n> +\tif (!commit->parents && lookup_commit_graft(commit->object.sha1))\n> +\t\tadd_name_decoration(DECORATION_GRAFTED, \"grafted\",\n> +\t\t\t\t    &commit->object);\n\nI am not very enthused about this change.\n\nWe have already looked up the commit when we parsed it, and then we again\nhave to call lookup_commit_graft() which would yield false for most of the\ncommits?\n\nDoes this work with replacements and shallows, by the way?\n"},{"id":"173711","messageId":"CACsJy8DvbN6w_ZBpjPdjpdsyYPa=O2G8gpcu2Oxx-mbvz_+4=A@mail.gmail.com","threadId":"28141","inReplyTo":"7v8vqreuwk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log: decorate grafted commits with \"grafted\"","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T02:02:50Z","receivedAt":"2011-08-18T02:02:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/8/18 Junio C Hamano <gitster@pobox.com>:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> @@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)\n>>       log.parent = NULL;\n>>       opt->loginfo = &log;\n>>\n>> +     if (!commit->parents && lookup_commit_graft(commit->object.sha1))\n>> +             add_name_decoration(DECORATION_GRAFTED, \"grafted\",\n>> +                                 &commit->object);\n>\n> I am not very enthused about this change.\n>\n> We have already looked up the commit when we parsed it, and then we again\n> have to call lookup_commit_graft() which would yield false for most of the\n> commits?\n\nwhich is why there's \"!commit->parents\" check. I made this patch with\nshallow clone in mind. You probably have seen that if grafts are used\nto extend history instead of cutting it, it won't show.\n\n> Does this work with replacements and shallows, by the way?\n\nShallows, yes (that was my aim). Replacements, no. A better way would\nbe go over commit_graft[] and replace_object[] arrays, decorate all\ngrafted/replaced commits, instead of checking in log_tree_commit().\n-- \nDuy\n"},{"id":"173748","messageId":"1313670578-2939-1-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313593326-12077-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 0/5] Decorate grafts and replaces","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:33Z","receivedAt":"2011-08-18T12:29:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I originally wanted to decorate shallow cut points. Now it decorates\nall grafts (normal as well as shallow cuts) and replaced commits/tags.\n\nThe first patch is an independent fix. The last one, decorating\nreplaces, can also be considered a fix (decorating \"refs/replace/SHA1\"\non random commits does not sound right, but maybe it's just me).\n\nNguyễn Thái Ngọc Duy (5):\n  decoration: do not mis-decorate refs with same prefix\n  Add for_each_commit_graft() to iterate all grafts\n  Move write_shallow_commits to fetch-pack.c\n  log: decorate grafted commits with \"grafted\"\n  log: decorate \"replaced\" on to replaced commits\n\n builtin/fetch-pack.c |   30 ++++++++++++++++++++++++++++++\n commit.c             |   20 +++++---------------\n commit.h             |    3 ++-\n log-tree.c           |   34 ++++++++++++++++++++++++++++++----\n 4 files changed, 67 insertions(+), 20 deletions(-)\n\n-- \n1.7.4.74.g639db\n"},{"id":"173749","messageId":"1313670578-2939-2-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313670578-2939-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/5] decoration: do not mis-decorate refs with same prefix","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:34Z","receivedAt":"2011-08-18T12:29:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"We definitely do not want to decorate refs/headsandtails the same as\nrefs/heads/*, for example.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n log-tree.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex e945701..344f734 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -95,11 +95,11 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in\n \tif (!obj)\n \t\treturn 0;\n \n-\tif (!prefixcmp(refname, \"refs/heads\"))\n+\tif (!prefixcmp(refname, \"refs/heads/\"))\n \t\ttype = DECORATION_REF_LOCAL;\n-\telse if (!prefixcmp(refname, \"refs/remotes\"))\n+\telse if (!prefixcmp(refname, \"refs/remotes/\"))\n \t\ttype = DECORATION_REF_REMOTE;\n-\telse if (!prefixcmp(refname, \"refs/tags\"))\n+\telse if (!prefixcmp(refname, \"refs/tags/\"))\n \t\ttype = DECORATION_REF_TAG;\n \telse if (!prefixcmp(refname, \"refs/stash\"))\n \t\ttype = DECORATION_REF_STASH;\n-- \n1.7.4.74.g639db\n"},{"id":"173750","messageId":"1313670578-2939-3-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313670578-2939-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/5] Add for_each_commit_graft() to iterate all grafts","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:35Z","receivedAt":"2011-08-18T12:29:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n commit.c |    8 ++++++++\n commit.h |    2 ++\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex dc22695..efd647d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -226,6 +226,14 @@ struct commit_graft *lookup_commit_graft(const unsigned char *sha1)\n \treturn commit_graft[pos];\n }\n \n+int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)\n+{\n+\tint i, ret;\n+\tfor (i = ret = 0; i < commit_graft_nr && !ret; i++)\n+\t\tret = fn(commit_graft[i], cb_data);\n+\treturn ret;\n+}\n+\n int write_shallow_commits(struct strbuf *out, int use_pack_protocol)\n {\n \tint i, count = 0;\ndiff --git a/commit.h b/commit.h\nindex 0e36fd0..9030d42 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -143,6 +143,7 @@ struct commit_graft {\n \tint nr_parent; /* < 0 if shallow commit */\n \tunsigned char parent[FLEX_ARRAY][20]; /* more */\n };\n+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@@ -155,6 +156,7 @@ extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n extern int register_shallow(const unsigned char *sha1);\n extern int unregister_shallow(const unsigned char *sha1);\n extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);\n+extern int for_each_commit_graft(each_commit_graft_fn, void *);\n extern int is_repository_shallow(void);\n extern struct commit_list *get_shallow_commits(struct object_array *heads,\n \t\tint depth, int shallow_flag, int not_shallow_flag);\n-- \n1.7.4.74.g639db\n"},{"id":"173751","messageId":"1313670578-2939-4-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313670578-2939-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/5] Move write_shallow_commits to fetch-pack.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:36Z","receivedAt":"2011-08-18T12:29:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This function produces network traffic and should be in fetch-pack. It\nhas been in commit.c because it needs to iterate (private) graft\nlist. It can now do so using for_each_commit_graft().\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/fetch-pack.c |   30 ++++++++++++++++++++++++++++++\n commit.c             |   18 ------------------\n commit.h             |    1 -\n 3 files changed, 30 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 4367984..cb5b20a 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -185,6 +185,36 @@ static void consume_shallow_list(int fd)\n \t}\n }\n \n+struct write_shallow_data {\n+\tstruct strbuf *out;\n+\tint use_pack_protocol;\n+\tint count;\n+};\n+\n+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n+{\n+\tstruct write_shallow_data *data = cb_data;\n+\tconst char *hex = sha1_to_hex(graft->sha1);\n+\tdata->count++;\n+\tif (data->use_pack_protocol)\n+\t\tpacket_buf_write(data->out, \"shallow %s\", hex);\n+\telse {\n+\t\tstrbuf_addstr(data->out, hex);\n+\t\tstrbuf_addch(data->out, '\\n');\n+\t}\n+\treturn 0;\n+}\n+\n+static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)\n+{\n+\tstruct write_shallow_data data;\n+\tdata.out = out;\n+\tdata.use_pack_protocol = use_pack_protocol;\n+\tdata.count = 0;\n+\tfor_each_commit_graft(write_one_shallow, &data);\n+\treturn data.count;\n+}\n+\n static enum ack_type get_ack(int fd, unsigned char *result_sha1)\n {\n \tstatic char line[1000];\ndiff --git a/commit.c b/commit.c\nindex efd647d..661ff0d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -234,24 +234,6 @@ int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)\n \treturn ret;\n }\n \n-int write_shallow_commits(struct strbuf *out, int use_pack_protocol)\n-{\n-\tint i, count = 0;\n-\tfor (i = 0; i < commit_graft_nr; i++)\n-\t\tif (commit_graft[i]->nr_parent < 0) {\n-\t\t\tconst char *hex =\n-\t\t\t\tsha1_to_hex(commit_graft[i]->sha1);\n-\t\t\tcount++;\n-\t\t\tif (use_pack_protocol)\n-\t\t\t\tpacket_buf_write(out, \"shallow %s\", hex);\n-\t\t\telse {\n-\t\t\t\tstrbuf_addstr(out, hex);\n-\t\t\t\tstrbuf_addch(out, '\\n');\n-\t\t\t}\n-\t\t}\n-\treturn count;\n-}\n-\n int unregister_shallow(const unsigned char *sha1)\n {\n \tint pos = commit_graft_pos(sha1);\ndiff --git a/commit.h b/commit.h\nindex 9030d42..82d5aeb 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -155,7 +155,6 @@ extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n \n extern int register_shallow(const unsigned char *sha1);\n extern int unregister_shallow(const unsigned char *sha1);\n-extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);\n extern int for_each_commit_graft(each_commit_graft_fn, void *);\n extern int is_repository_shallow(void);\n extern struct commit_list *get_shallow_commits(struct object_array *heads,\n-- \n1.7.4.74.g639db\n"},{"id":"173753","messageId":"1313670578-2939-5-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313670578-2939-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/5] log: decorate grafted commits with \"grafted\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:37Z","receivedAt":"2011-08-18T12:29:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"In shallow repositories, this may help detect whether a branch ends,\nor it is deeper than current depth.\n\nIt also show graft points that extend a branch.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n log-tree.c |   12 ++++++++++++\n 1 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 344f734..5605244 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -18,6 +18,7 @@ enum decoration_type {\n \tDECORATION_REF_TAG,\n \tDECORATION_REF_STASH,\n \tDECORATION_REF_HEAD,\n+\tDECORATION_GRAFTED,\n };\n \n static char decoration_colors[][COLOR_MAXLEN] = {\n@@ -27,6 +28,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_BOLD_YELLOW,\t/* REF_TAG */\n \tGIT_COLOR_BOLD_MAGENTA,\t/* REF_STASH */\n \tGIT_COLOR_BOLD_CYAN,\t/* REF_HEAD */\n+\tGIT_COLOR_BOLD_BLUE,\t/* GRAFTED */\n };\n \n static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)\n@@ -118,6 +120,15 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in\n \treturn 0;\n }\n \n+static int add_graft_decoration(const struct commit_graft *graft, void *cb_data)\n+{\n+\tstruct commit *commit = lookup_commit(graft->sha1);\n+\tif (!commit)\n+\t\treturn 0;\n+\tadd_name_decoration(DECORATION_GRAFTED, \"grafted\", &commit->object);\n+\treturn 0;\n+}\n+\n void load_ref_decorations(int flags)\n {\n \tstatic int loaded;\n@@ -125,6 +136,7 @@ void load_ref_decorations(int flags)\n \t\tloaded = 1;\n \t\tfor_each_ref(add_ref_decoration, &flags);\n \t\thead_ref(add_ref_decoration, &flags);\n+\t\tfor_each_commit_graft(add_graft_decoration, NULL);\n \t}\n }\n \n-- \n1.7.4.74.g639db\n"},{"id":"173752","messageId":"1313670578-2939-6-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"1313670578-2939-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 5/5] log: decorate \"replaced\" on to replaced commits","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-18T12:29:38Z","receivedAt":"2011-08-18T12:29:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Old code also decorates \"new\" commits with \"refs/replace/SHA1\". This\nis now gone, but I guess no one will miss it.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n log-tree.c |   16 +++++++++++++++-\n 1 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 5605244..b982b9f 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -92,8 +92,22 @@ static void add_name_decoration(enum decoration_type type, const char *name, str\n \n static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n-\tstruct object *obj = parse_object(sha1);\n+\tstruct object *obj;\n \tenum decoration_type type = DECORATION_NONE;\n+\n+\tif (!prefixcmp(refname, \"refs/replace/\")) {\n+\t\tunsigned char original_sha1[20];\n+\t\tif (get_sha1_hex(refname + 13, original_sha1)) {\n+\t\t\twarning(\"invalid replace ref %s\", refname);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tobj = parse_object(original_sha1);\n+\t\tif (obj && (obj->type == OBJ_COMMIT || obj->type == OBJ_TAG))\n+\t\t\tadd_name_decoration(DECORATION_GRAFTED, \"replaced\", obj);\n+\t\treturn 0;\n+\t}\n+\n+\tobj = parse_object(sha1);\n \tif (!obj)\n \t\treturn 0;\n \n-- \n1.7.4.74.g639db\n"},{"id":"173775","messageId":"7vzkj6a439.fsf@alter.siamese.dyndns.org","threadId":"28141","inReplyTo":"1313670578-2939-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/5] decoration: do not mis-decorate refs with same prefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T17:58:36Z","receivedAt":"2011-08-18T17:58:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Good eyes; thanks.\n"},{"id":"173776","messageId":"7vty9ea437.fsf@alter.siamese.dyndns.org","threadId":"28141","inReplyTo":"1313670578-2939-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/5] Move write_shallow_commits to fetch-pack.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T18:01:26Z","receivedAt":"2011-08-18T18:01:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nice; thanks.\n"},{"id":"173778","messageId":"7vobzma435.fsf@alter.siamese.dyndns.org","threadId":"28141","inReplyTo":"1313670578-2939-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/5] log: decorate grafted commits with \"grafted\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T18:10:20Z","receivedAt":"2011-08-18T18:10:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I think this makes a lot more sense than the other option of looking up\ngraft points every time, as we expect only a handful of grafts, if any,\nexist in a repository.\n\nIf you have, on the other hand, tons of historical graft points, and you\nare viewing only near the tip (e.g. \"log -20\"), this implimentation still\ncalls lookup_commit() for all those historical graft points that are\nlikely to be scattered further back in the packfile, and the performance\nmay suffer.  But that is what it means when we say that we are optimized\nfor use case where only a handful of grafts exist, so it is not something\nthat we worry about too much.\n"},{"id":"173774","messageId":"7vippua433.fsf@alter.siamese.dyndns.org","threadId":"28141","inReplyTo":"1313670578-2939-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 5/5] log: decorate \"replaced\" on to replaced commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T18:16:55Z","receivedAt":"2011-08-18T18:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Old code also decorates \"new\" commits with \"refs/replace/SHA1\". This\n> is now gone, but I guess no one will miss it.\n\nMakes sense but...\n\n> +\tif (!prefixcmp(refname, \"refs/replace/\")) {\n> +\t\tunsigned char original_sha1[20];\n> +\t\tif (get_sha1_hex(refname + 13, original_sha1)) {\n> +\t\t\twarning(\"invalid replace ref %s\", refname);\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tobj = parse_object(original_sha1);\n> +\t\tif (obj && (obj->type == OBJ_COMMIT || obj->type == OBJ_TAG))\n\n... is it necessary to check and limit the types like this here?\n\nIf the argument is \"we know only commits and tags are listed and blobs and\ntrees are not shown with --decorate option\" and \"excluding the decoration\nthat we know will never be used will avoid bloating the decorate hashtable\nwith unused cruft\", then add_name_decoration() should be doing the check\nfor all of its callers, not just this one, no?\n\n> +\t\t\tadd_name_decoration(DECORATION_GRAFTED, \"replaced\", obj);\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tobj = parse_object(sha1);\n>  \tif (!obj)\n>  \t\treturn 0;\n"},{"id":"173855","messageId":"1313757830-30640-1-git-send-email-pclouds@gmail.com","threadId":"28141","inReplyTo":"7vippua433.fsf@alter.siamese.dyndns.org","subject":"[PATCH] log: decorate \"replaced\" on to replaced commits","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-19T12:43:50Z","receivedAt":"2011-08-19T12:43:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Old code also decorates \"new\" commits with \"refs/replace/SHA1\". This\nis now gone, but I guess no one will miss it.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n 2011/8/19 Junio C Hamano <gitster@pobox.com>:\n > If the argument is \"we know only commits and tags are listed and blobs and\n > trees are not shown with --decorate option\" and \"excluding the decoration\n > that we know will never be used will avoid bloating the decorate hashtable\n > with unused cruft\", then add_name_decoration() should be doing the check\n > for all of its callers, not just this one, no?\n\n Makes sense. Moreover replacing blobs and trees are generally not safe.\n If people do that, they may have more issues to worry about than this.\n Let's keep it simple. We can fix add_name_decoration() later if it becomes\n a real problem.\n\n log-tree.c |   16 +++++++++++++++-\n 1 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex e945701..d73e69c 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -92,8 +92,22 @@ static void add_name_decoration(enum decoration_type type, const char *name, str\n \n static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n-\tstruct object *obj = parse_object(sha1);\n+\tstruct object *obj;\n \tenum decoration_type type = DECORATION_NONE;\n+\n+\tif (!prefixcmp(refname, \"refs/replace/\")) {\n+\t\tunsigned char original_sha1[20];\n+\t\tif (get_sha1_hex(refname + 13, original_sha1)) {\n+\t\t\twarning(\"invalid replace ref %s\", refname);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tobj = parse_object(original_sha1);\n+\t\tif (obj)\n+\t\t\tadd_name_decoration(DECORATION_GRAFTED, \"replaced\", obj);\n+\t\treturn 0;\n+\t}\n+\n+\tobj = parse_object(sha1);\n \tif (!obj)\n \t\treturn 0;\n \n-- \n1.7.4.74.g639db\n"}]}