{"thread":{"id":"6069","subject":"[PATCH 2/3] Introduce a global level warn() function.","startedAt":"2006-12-22T00:48:32Z","lastAt":"2006-12-22T08:22:46Z","messageCount":9,"participants":["Shawn O. Pearce","Junio C Hamano","Shawn Pearce","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"29982","messageId":"20061222004832.GB14789@spearce.org","threadId":"6069","inReplyTo":"be6b1443171482e1930bd7744a0218db0c03d611.1166748450.git.spearce@spearce.org","subject":"[PATCH 2/3] Introduce a global level warn() function.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T00:48:32Z","receivedAt":"2006-12-22T00:48:32Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Like the existing error() function the new warn() function can be\nused to describe a situation that probably should not be occuring,\nbut which the user (and Git) can continue to work around without\nrunning into too many problems.\n\nAn example situation is a bad commit SHA1 found in a reflog.\nAttempting to read this record out of the reflog isn't really an\nerror as we have skipped over it in the past.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n git-compat-util.h |    2 ++\n usage.c           |   19 +++++++++++++++++++\n 2 files changed, 21 insertions(+), 0 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 41fa7f6..304cdbf 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -71,10 +71,12 @@\n extern void usage(const char *err) NORETURN;\n extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));\n extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));\n+extern void warn(const char *err, ...) __attribute__((format (printf, 1, 2)));\n \n extern void set_usage_routine(void (*routine)(const char *err) NORETURN);\n extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);\n extern void set_error_routine(void (*routine)(const char *err, va_list params));\n+extern void set_warn_routine(void (*routine)(const char *warn, va_list params));\n \n #ifdef NO_MMAP\n \ndiff --git a/usage.c b/usage.c\nindex 52c2e96..4dc5c77 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -29,12 +29,17 @@ static void error_builtin(const char *err, va_list params)\n \treport(\"error: \", err, params);\n }\n \n+static void warn_builtin(const char *warn, va_list params)\n+{\n+\treport(\"warning: \", warn, params);\n+}\n \n /* If we are in a dlopen()ed .so write to a global variable would segfault\n  * (ugh), so keep things static. */\n static void (*usage_routine)(const char *err) NORETURN = usage_builtin;\n static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;\n static void (*error_routine)(const char *err, va_list params) = error_builtin;\n+static void (*warn_routine)(const char *err, va_list params) = warn_builtin;\n \n void set_usage_routine(void (*routine)(const char *err) NORETURN)\n {\n@@ -51,6 +56,11 @@ void set_error_routine(void (*routine)(const char *err, va_list params))\n \terror_routine = routine;\n }\n \n+void set_warn_routine(void (*routine)(const char *warn, va_list params))\n+{\n+\twarn_routine = routine;\n+}\n+\n \n void usage(const char *err)\n {\n@@ -75,3 +85,12 @@ int error(const char *err, ...)\n \tva_end(params);\n \treturn -1;\n }\n+\n+void warn(const char *warn, ...)\n+{\n+\tva_list params;\n+\n+\tva_start(params, warn);\n+\twarn_routine(warn, params);\n+\tva_end(params);\n+}\n-- \n1.4.4.3.ga78a1\n"},{"id":"29984","messageId":"20061222004906.GC14789@spearce.org","threadId":"6069","inReplyTo":"be6b1443171482e1930bd7744a0218db0c03d611.1166748450.git.spearce@spearce.org","subject":"[PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T00:49:06Z","receivedAt":"2006-12-22T00:49:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If the user has been using reflog for a long time (e.g. since its\nintroduction) then it is very likely that an existing branch's\nreflog may still mention commits which have long since been pruned\nout of the repository.\n\nRather than aborting with a very useless error message during\ngit-repack, pack as many valid commits as we can get from the\nreflog and let the user know that the branch's reflog contains\nalready pruned commits.  A future 'git reflog expire' (or whatever\nit finally winds up being called) can then be performed to expunge\nthose reflog entries.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n revision.c |   29 +++++++++++++++++++----------\n 1 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex de2cbd8..52330cc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -466,6 +466,7 @@ static void limit_list(struct rev_info *revs)\n \n struct all_refs_cb {\n \tint all_flags;\n+\tint warned_bad_reflog;\n \tstruct rev_info *all_revs;\n \tconst char *name_for_errormsg;\n };\n@@ -487,25 +488,33 @@ static void handle_all(struct rev_info *revs, unsigned flags)\n \tfor_each_ref(handle_one_ref, &cb);\n }\n \n-static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1, char *detail, void *cb_data)\n+static int handle_one_reflog_commit(unsigned char *sha1, void *cb_data)\n {\n \tstruct all_refs_cb *cb = cb_data;\n-\tstruct object *object;\n-\n-\tif (!is_null_sha1(osha1)) {\n-\t\tobject = get_reference(cb->all_revs, cb->name_for_errormsg,\n-\t\t\t\t       osha1, cb->all_flags);\n-\t\tadd_pending_object(cb->all_revs, object, \"\");\n+\tif (!is_null_sha1(sha1)) {\n+\t\tstruct object *o = parse_object(sha1);\n+\t\tif (o) {\n+\t\t\to->flags |= cb->all_flags;\n+\t\t\tadd_pending_object(cb->all_revs, o, \"\");\n+\t\t} else if (!cb->warned_bad_reflog) {\n+\t\t\twarn(\"reflog of '%s' references pruned commits\",\n+\t\t\t\tcb->name_for_errormsg);\n+\t\t\tcb->warned_bad_reflog = 1;\n+\t\t}\n \t}\n-\tobject = get_reference(cb->all_revs, cb->name_for_errormsg,\n-\t\t\t       nsha1, cb->all_flags);\n-\tadd_pending_object(cb->all_revs, object, \"\");\n+}\n+\n+static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1, char *detail, void *cb_data)\n+{\n+\thandle_one_reflog_commit(osha1, cb_data);\n+\thandle_one_reflog_commit(nsha1, cb_data);\n \treturn 0;\n }\n \n static int handle_one_reflog(const char *path, const unsigned char *sha1, int flag, void *cb_data)\n {\n \tstruct all_refs_cb *cb = cb_data;\n+\tcb->warned_bad_reflog = 0;\n \tcb->name_for_errormsg = path;\n \tfor_each_reflog_ent(path, handle_one_reflog_ent, cb_data);\n \treturn 0;\n-- \n1.4.4.3.ga78a1\n"},{"id":"29985","messageId":"7vmz5g92h7.fsf@assigned-by-dhcp.cox.net","threadId":"6069","inReplyTo":"20061222004906.GC14789@spearce.org","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-22T00:52:04Z","receivedAt":"2006-12-22T00:52:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> If the user has been using reflog for a long time (e.g. since its\n> introduction) then it is very likely that an existing branch's\n> reflog may still mention commits which have long since been pruned\n> out of the repository.\n\nI've thought about this issue when I did the repack/prune; my\ntake on this was you should prune reflog first then repack.\n"},{"id":"29989","messageId":"20061222005606.GA14773@spearce.org","threadId":"6069","inReplyTo":"20061222004906.GC14789@spearce.org","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T00:56:06Z","receivedAt":"2006-12-22T00:56:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> wrote:\n> If the user has been using reflog for a long time (e.g. since its\n> introduction) then it is very likely that an existing branch's\n> reflog may still mention commits which have long since been pruned\n> out of the repository.\n> \n> Rather than aborting with a very useless error message during\n> git-repack, pack as many valid commits as we can get from the\n> reflog and let the user know that the branch's reflog contains\n> already pruned commits.  A future 'git reflog expire' (or whatever\n> it finally winds up being called) can then be performed to expunge\n> those reflog entries.\n\nIf its not obvious from the patch, this doesn't entirely fix the\nproblem.\n\nJust because the commit has not been pruned does not mean that a blob\nor tree referenced by that commit has not been pruned.  Right now\nI am missing 1 blob and cannot repack because of it.\n\nAt least with this series of 3 patches the error message resulting\nfrom this one missing blob is clearer.\n\n-- \nShawn.\n"},{"id":"29990","messageId":"20061222010018.GB14773@spearce.org","threadId":"6069","inReplyTo":"7vmz5g92h7.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T01:00:18Z","receivedAt":"2006-12-22T01:00:18Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> \"Shawn O. Pearce\" <spearce@spearce.org> writes:\n> \n> > If the user has been using reflog for a long time (e.g. since its\n> > introduction) then it is very likely that an existing branch's\n> > reflog may still mention commits which have long since been pruned\n> > out of the repository.\n> \n> I've thought about this issue when I did the repack/prune; my\n> take on this was you should prune reflog first then repack.\n\nOK, but we should suggest that to the user rather than just\ncryptically saying 'fatal: bad object refs/heads/build'.\n\n-- \nShawn.\n"},{"id":"29993","messageId":"emfbk4$f45$1@sea.gmane.org","threadId":"6069","inReplyTo":"20061222010018.GB14773@spearce.org","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-22T01:18:33Z","receivedAt":"2006-12-22T01:18:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Shawn Pearce wrote:\n\n> Junio C Hamano <junkio@cox.net> wrote:\n>> \"Shawn O. Pearce\" <spearce@spearce.org> writes:\n>> \n>>> If the user has been using reflog for a long time (e.g. since its\n>>> introduction) then it is very likely that an existing branch's\n>>> reflog may still mention commits which have long since been pruned\n>>> out of the repository.\n>> \n>> I've thought about this issue when I did the repack/prune; my\n>> take on this was you should prune reflog first then repack.\n> \n> OK, but we should suggest that to the user rather than just\n> cryptically saying 'fatal: bad object refs/heads/build'.\n\nI still think it is a good idea to allow user (experienced user)\nto set to not consider reflog for saving. Especially that there\nexist repositories which have reflogs with long pruned commits,\nand it would be nice to preserve the reflog info, even if some of\ninformation is not available.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"29994","messageId":"20061222012344.GD14773@spearce.org","threadId":"6069","inReplyTo":"emfbk4$f45$1@sea.gmane.org","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T01:23:44Z","receivedAt":"2006-12-22T01:23:44Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> wrote:\n> I still think it is a good idea to allow user (experienced user)\n> to set to not consider reflog for saving. Especially that there\n> exist repositories which have reflogs with long pruned commits,\n> and it would be nice to preserve the reflog info, even if some of\n> information is not available.\n\nIn my humble opinion there is *no* value in a reflog whose commits\nare \"corrupt\" (due to missing commit object or missing tree/blob\nit references) as you cannot get that commit back.  And its hard\nto get any other data from it.\n\nMy local git.git repository got killed during a repack because\nI was missing 1 blob (an old version of builtin-blame.c) from\nJunio's pu branch.  Apparently I had a reflog on that tracking\nbranch which went back pretty far and although the commits in that\nlog were still available, the blob wasn't.\n\n-- \nShawn.\n"},{"id":"30022","messageId":"7vodpw5ors.fsf@assigned-by-dhcp.cox.net","threadId":"6069","inReplyTo":"20061222005606.GA14773@spearce.org","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-22T08:16:23Z","receivedAt":"2006-12-22T08:16:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> Just because the commit has not been pruned does not mean that a blob\n> or tree referenced by that commit has not been pruned.\n\nTrue.  How about this?\n\n---\n\n builtin-reflog.c |   43 ++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 40 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-reflog.c b/builtin-reflog.c\nindex d4f7353..4097c32 100644\n--- a/builtin-reflog.c\n+++ b/builtin-reflog.c\n@@ -3,7 +3,7 @@\n #include \"commit.h\"\n #include \"refs.h\"\n #include \"dir.h\"\n-#include <time.h>\n+#include \"tree-walk.h\"\n \n struct expire_reflog_cb {\n \tFILE *newlog;\n@@ -13,13 +13,50 @@ struct expire_reflog_cb {\n \tunsigned long expire_unreachable;\n };\n \n+static int tree_is_complete(const unsigned char *sha1)\n+{\n+\tstruct tree_desc desc;\n+\tvoid *buf;\n+\tchar type[20];\n+\n+\tbuf = read_sha1_file(sha1, type, &desc.size);\n+\tif (!buf)\n+\t\treturn 0;\n+\tdesc.buf = buf;\n+\twhile (desc.size) {\n+\t\tconst unsigned char *elem;\n+\t\tconst char *name;\n+\t\tunsigned mode;\n+\n+\t\telem = tree_entry_extract(&desc, &name, &mode);\n+\t\tif (!has_sha1_file(elem) ||\n+\t\t    (S_ISDIR(mode) && !tree_is_complete(elem))) {\n+\t\t\tfree(buf);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tupdate_tree_entry(&desc);\n+\t}\n+\tfree(buf);\n+\treturn 1;\n+}\n+\n static int keep_entry(struct commit **it, unsigned char *sha1)\n {\n+\tstruct commit *commit;\n+\n \t*it = NULL;\n \tif (is_null_sha1(sha1))\n \t\treturn 1;\n-\t*it = lookup_commit_reference_gently(sha1, 1);\n-\treturn (*it != NULL);\n+\tcommit = lookup_commit_reference_gently(sha1, 1);\n+\tif (!commit)\n+\t\treturn 0;\n+\n+\t/* Make sure everything in this commit exists. */\n+\tparse_object(commit->object.sha1);\n+\tif (!tree_is_complete(commit->tree->object.sha1))\n+\t\treturn 0;\n+\t*it = commit;\n+\treturn 1;\n }\n \n static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n"},{"id":"30025","messageId":"20061222082246.GC15548@spearce.org","threadId":"6069","inReplyTo":"7vodpw5ors.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] Don't crash during repack of a reflog with pruned commits.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-22T08:22:46Z","receivedAt":"2006-12-22T08:22:46Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Shawn Pearce <spearce@spearce.org> writes:\n> \n> > Just because the commit has not been pruned does not mean that a blob\n> > or tree referenced by that commit has not been pruned.\n> \n> True.  How about this?\n\nYea, that's a good way to deal with it.  Ack'd.\n\n-- \nShawn.\n"}]}