{"thread":{"id":"23460","subject":"[PATCH] am -3: recover the diagnostic messages for corrupt patches","startedAt":"2010-04-14T20:33:30Z","lastAt":"2010-04-15T03:28:49Z","messageCount":2,"participants":["Junio C Hamano","Stephen Boyd"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"139509","messageId":"7vpr21pyed.fsf@alter.siamese.dyndns.org","threadId":"23460","inReplyTo":null,"subject":"[PATCH] am -3: recover the diagnostic messages for corrupt patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-14T20:33:30Z","receivedAt":"2010-04-14T20:33:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git am -3\" first tries to apply the patch without any extra trick, and\napplies it to a synthesized tree for 3-way merge after the first attempt\nfails.  \"git apply\" exits with status 1 for a patch that is well-formed\nbut is not applicable (and it dies on other errors with non-zereo, non-1\nstatus) and has an optimization to fall back to the 3-way merge only in\nthe case.\n\nAn earlier patch 3ddd170 (am: suppress apply errors when using 3-way,\n2009-06-16) squelched diagnostic messages from the first attempt, not to\nbe shown to the end user.  This worked reasonably well if the reason the\nfirst application failed was because the patch was made against a wrong\nversion.\n\nWhen the patch is corrupt (e.g. line-wrapped or leading whitespaces got\ndropped), however, because the second patch application is not even\nattempted, the error message from the first application is never shown\nand is forever lost.  This message is necessary to locate where the patch\nis corrupt and fix it up.\n\nWe could fix this issue by reverting 3dd170, or keeping the error message\nto somewhere and showing it, but because this is an error codepath, the\neasiest is to disable the optimization.  The second patch application is\nattempted even when the input is corrupt, and it will notice, diagnose,\nand stop with an error message.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-am.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex e26c54a..4e0e406 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -534,7 +534,7 @@ do\n \t\t;;\n \tesac\n \n-\tif test $apply_status = 1 && test \"$threeway\" = t\n+\tif test $apply_status != 0 && test \"$threeway\" = t\n \tthen\n \t\tif (fall_back_3way)\n \t\tthen\n-- \n1.7.1.rc1.252.gce30c\n"},{"id":"139545","messageId":"4BC687F1.4010608@gmail.com","threadId":"23460","inReplyTo":"7vpr21pyed.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] am -3: recover the diagnostic messages for corrupt patches","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2010-04-15T03:28:49Z","receivedAt":"2010-04-15T03:28:49Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"On 04/14/2010 01:33 PM, Junio C Hamano wrote:\n[...]\n> We could fix this issue by reverting 3dd170, or keeping the error message\n> to somewhere and showing it, but because this is an error codepath, the\n> easiest is to disable the optimization.  The second patch application is\n> attempted even when the input is corrupt, and it will notice, diagnose,\n> and stop with an error message.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nLooks fine to me. Two invocations on an error path where user\nintervention is most likely required shouldn't be too bad like you say.\n\nThis reminds me, a while ago I looked into pushing 3way into git-apply\n(that was a GSoC project suggestion). We could introduce an internal\n\"I'm attempting a 3way if this fails\" option for git-apply and then it\ncould be smart and output the errors if the patch is malformed.\nOtherwise it would be silent like it should be on 3way apply for the\nfirst time failures. Obviously this will have to happen for 3way to be\npushed into git-apply at all.\n\nSounds a little too smart to me though (this is the stupid content\ntracker after all) so let's just do what you have ;-)\n"}]}