{"thread":{"id":"11875","subject":"[PATCH 2/9] Add flag to make unpack_trees() not print errors.","startedAt":"2008-02-04T18:35:40Z","lastAt":"2008-02-06T01:11:14Z","messageCount":5,"participants":["Daniel Barkalow","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"67400","messageId":"alpine.LNX.1.00.0802041335390.13593@iabervon.org","threadId":"11875","inReplyTo":null,"subject":"[PATCH 2/9] Add flag to make unpack_trees() not print errors.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-04T18:35:40Z","receivedAt":"2008-02-04T18:35:40Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This will allow builtin-checkout to suppress merge errors if it's\ngoing to try more merging methods.\n\nAdditionally, if unpack_trees() returns with an error, but without\nprinting anything, it will roll back any changes to the index (by\nrereading the index, currently). This obviously could be done by the\ncaller, but chances are that the caller would forget and debugging\nthis is difficult.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n unpack-trees.c |   43 +++++++++++++++++++++++++++++--------------\n unpack-trees.h |    1 +\n 2 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9e6587f..45f40c2 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -356,12 +356,23 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\tposns[i] = create_tree_entry_list(t+i);\n \n \t\tif (unpack_trees_rec(posns, len, o->prefix ? o->prefix : \"\",\n-\t\t\t\t     o, &df_conflict_list))\n+\t\t\t\t     o, &df_conflict_list)) {\n+\t\t\tif (o->gently) {\n+\t\t\t\tdiscard_cache();\n+\t\t\t\tread_cache();\n+\t\t\t}\n \t\t\treturn -1;\n+\t\t}\n \t}\n \n-\tif (o->trivial_merges_only && o->nontrivial_merge)\n-\t\treturn error(\"Merge requires file-level merging\");\n+\tif (o->trivial_merges_only && o->nontrivial_merge) {\n+\t\tif (o->gently) {\n+\t\t\tdiscard_cache();\n+\t\t\tread_cache();\n+\t\t}\n+\t\treturn o->gently ? -1 :\n+\t\t\terror(\"Merge requires file-level merging\");\n+\t}\n \n \tcheck_updates(active_cache, active_nr, o);\n \treturn 0;\n@@ -415,7 +426,8 @@ static int verify_uptodate(struct cache_entry *ce,\n \t}\n \tif (errno == ENOENT)\n \t\treturn 0;\n-\treturn error(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n+\treturn o->gently ? -1 :\n+\t\terror(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n }\n \n static void invalidate_ce_path(struct cache_entry *ce)\n@@ -501,8 +513,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t\td.exclude_per_dir = o->dir->exclude_per_dir;\n \ti = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);\n \tif (i)\n-\t\treturn error(\"Updating '%s' would lose untracked files in it\",\n-\t\t\t     ce->name);\n+\t\treturn o->gently ? -1 :\n+\t\t\terror(\"Updating '%s' would lose untracked files in it\",\n+\t\t\t      ce->name);\n \tfree(pathbuf);\n \treturn cnt;\n }\n@@ -574,8 +587,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,\n \t\t\t\treturn 0;\n \t\t}\n \n-\t\treturn error(\"Untracked working tree file '%s' \"\n-\t\t\t     \"would be %s by merge.\", ce->name, action);\n+\t\treturn o->gently ? -1 :\n+\t\t\terror(\"Untracked working tree file '%s' \"\n+\t\t\t      \"would be %s by merge.\", ce->name, action);\n \t}\n \treturn 0;\n }\n@@ -707,7 +721,7 @@ int threeway_merge(struct cache_entry **stages,\n \t/* #14, #14ALT, #2ALT */\n \tif (remote && !df_conflict_head && head_match && !remote_match) {\n \t\tif (index && !same(index, remote) && !same(index, head))\n-\t\t\treturn reject_merge(index);\n+\t\t\treturn o->gently ? -1 : reject_merge(index);\n \t\treturn merged_entry(remote, index, o);\n \t}\n \t/*\n@@ -715,7 +729,7 @@ int threeway_merge(struct cache_entry **stages,\n \t * make sure that it matches head.\n \t */\n \tif (index && !same(index, head)) {\n-\t\treturn reject_merge(index);\n+\t\treturn o->gently ? -1 : reject_merge(index);\n \t}\n \n \tif (head) {\n@@ -866,11 +880,11 @@ int twoway_merge(struct cache_entry **src,\n \t\t\t/* all other failures */\n \t\t\tremove_entry(remove);\n \t\t\tif (oldtree)\n-\t\t\t\treturn reject_merge(oldtree);\n+\t\t\t\treturn o->gently ? -1 : reject_merge(oldtree);\n \t\t\tif (current)\n-\t\t\t\treturn reject_merge(current);\n+\t\t\t\treturn o->gently ? -1 : reject_merge(current);\n \t\t\tif (newtree)\n-\t\t\t\treturn reject_merge(newtree);\n+\t\t\t\treturn o->gently ? -1 : reject_merge(newtree);\n \t\t\treturn -1;\n \t\t}\n \t}\n@@ -897,7 +911,8 @@ int bind_merge(struct cache_entry **src,\n \t\treturn error(\"Cannot do a bind merge of %d trees\\n\",\n \t\t\t     o->merge_size);\n \tif (a && old)\n-\t\treturn error(\"Entry '%s' overlaps.  Cannot bind.\", a->name);\n+\t\treturn o->gently ? -1 :\n+\t\t\terror(\"Entry '%s' overlaps.  Cannot bind.\", a->name);\n \tif (!a)\n \t\treturn keep_entry(old, o);\n \telse\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 197a004..83d1229 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -16,6 +16,7 @@ struct unpack_trees_options {\n \tint trivial_merges_only;\n \tint verbose_update;\n \tint aggressive;\n+\tint gently;\n \tconst char *prefix;\n \tint pos;\n \tstruct dir_struct *dir;\n-- \n1.5.4\n"},{"id":"67462","messageId":"alpine.LSU.1.00.0802050112380.8543@racer.site","threadId":"11875","inReplyTo":"alpine.LNX.1.00.0802041335390.13593@iabervon.org","subject":"Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-05T01:16:34Z","receivedAt":"2008-02-05T01:16:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, Daniel Barkalow wrote:\n\n> This will allow builtin-checkout to suppress merge errors if it's going \n> to try more merging methods.\n\nI saw one error() in twoway_merge(), one in bind_merge(), and one in \nonway_merge() that were not guarded by o->gently.\n\nAlso, I'd call it \"silent\", not \"gently\".\n\n> Additionally, if unpack_trees() returns with an error, but without \n> printing anything, it will roll back any changes to the index (by \n> rereading the index, currently). This obviously could be done by the \n> caller, but chances are that the caller would forget and debugging this \n> is difficult.\n\nGranted, it is easy to forget.  But maybe the caller does not need the \nindex?  Or maybe it wants a different one?  I'd prefer the caller to clean \nup, if necessary.\n\nCiao,\nDscho\n"},{"id":"67548","messageId":"alpine.LNX.1.00.0802051439200.13593@iabervon.org","threadId":"11875","inReplyTo":"alpine.LSU.1.00.0802050112380.8543@racer.site","subject":"Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-05T20:38:30Z","receivedAt":"2008-02-05T20:38:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n> \n> > This will allow builtin-checkout to suppress merge errors if it's going \n> > to try more merging methods.\n> \n> I saw one error() in twoway_merge(), one in bind_merge(), and one in \n> onway_merge() that were not guarded by o->gently.\n> \n> Also, I'd call it \"silent\", not \"gently\".\n> \n> > Additionally, if unpack_trees() returns with an error, but without \n> > printing anything, it will roll back any changes to the index (by \n> > rereading the index, currently). This obviously could be done by the \n> > caller, but chances are that the caller would forget and debugging this \n> > is difficult.\n> \n> Granted, it is easy to forget.  But maybe the caller does not need the \n> index?  Or maybe it wants a different one?  I'd prefer the caller to clean \n> up, if necessary.\n\nThat's what makes it \"gently\" instead of just \"silent\"; it has no effect \nif it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack \ninto a separate index, which should actually be faster (since it doesn't \nhave to keep shifting the entries in the index it's working on) and make \nthis moot. In any case, it only rolls it back with the option that's only \nused by a caller that wants the index unchanged on failure, currently. If \na caller turns out to just want a return code and not care about the index \nor the error message, and the code hasn't been reworked, we can add a \nseparate flag then.\n\nI'd done some analysis at the time that suggested that, if you didn't want \nto give a message on failure, you must want to do something else to the \nindex to replace what hadn't worked, so you must want the index reset, but \nI've forgotten why I was so sure at the time, aside from that nobody's \nwanted it before now.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"67587","messageId":"7v7ihj7yv8.fsf@gitster.siamese.dyndns.org","threadId":"11875","inReplyTo":"alpine.LNX.1.00.0802051439200.13593@iabervon.org","subject":"Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T23:43:55Z","receivedAt":"2008-02-05T23:43:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> ...\n>> > Additionally, if unpack_trees() returns with an error, but without \n>> > printing anything, it will roll back any changes to the index (by \n>> > rereading the index, currently). This obviously could be done by the \n>> > caller, but chances are that the caller would forget and debugging this \n>> > is difficult.\n>> \n>> Granted, it is easy to forget.  But maybe the caller does not need the \n>> index?  Or maybe it wants a different one?  I'd prefer the caller to clean \n>> up, if necessary.\n>\n> That's what makes it \"gently\" instead of just \"silent\"; it has no effect \n> if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack \n> into a separate index, which should actually be faster (since it doesn't \n> have to keep shifting the entries in the index it's working on) and make \n> this moot.\n\nAbsolutely.  That is the original motivation I did the_index\nthing for.\n\nBut \"re-reading\" may not be quite nice.  It would defeat the\noptimization introduced by the change to use CE_UPTODATE flag to\navoid unnecessary lstat(2) calls.\n"},{"id":"67598","messageId":"alpine.LNX.1.00.0802052006450.13593@iabervon.org","threadId":"11875","inReplyTo":"7v7ihj7yv8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-06T01:11:14Z","receivedAt":"2008-02-06T01:11:14Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 5 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> > ...\n> >> > Additionally, if unpack_trees() returns with an error, but without \n> >> > printing anything, it will roll back any changes to the index (by \n> >> > rereading the index, currently). This obviously could be done by the \n> >> > caller, but chances are that the caller would forget and debugging this \n> >> > is difficult.\n> >> \n> >> Granted, it is easy to forget.  But maybe the caller does not need the \n> >> index?  Or maybe it wants a different one?  I'd prefer the caller to clean \n> >> up, if necessary.\n> >\n> > That's what makes it \"gently\" instead of just \"silent\"; it has no effect \n> > if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack \n> > into a separate index, which should actually be faster (since it doesn't \n> > have to keep shifting the entries in the index it's working on) and make \n> > this moot.\n> \n> Absolutely.  That is the original motivation I did the_index\n> thing for.\n> \n> But \"re-reading\" may not be quite nice.  It would defeat the\n> optimization introduced by the change to use CE_UPTODATE flag to\n> avoid unnecessary lstat(2) calls.\n\nFor now, I want to re-read the index, because I need the changes undone \nand I don't have any way of reverting them. That's another reason to have \nunpack_trees return with the index unchanged: it may have a better way of \ngetting there than the caller does, such as having never changed that \nmemory in the first place. I'll add doing it efficiently to my list of \nthings to do, but I want to keep it from being necessary for correctness \nof this series.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}