{"thread":{"id":"45106","subject":"fuzzy patch application","startedAt":"2017-02-10T19:21:04Z","lastAt":"2017-02-11T03:10:05Z","messageCount":9,"participants":["Nick Desaulniers","Junio C Hamano","Jeff King","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"311293","messageId":"CAKwvOdn9j=_Ob=xq4ucN6Ar1G537zNiU9ox4iF6o1qO7kPY41A@mail.gmail.com","threadId":"45106","inReplyTo":null,"subject":"fuzzy patch application","fromName":"Nick Desaulniers","fromEmail":"ndesaulniers@google.com","sentAt":"2017-02-10T19:20:59Z","receivedAt":"2017-02-10T19:21:04Z","isPatch":false,"sender":{"key":"ndesaulniers@google.com","avatar":null},"body":"I frequently need to backport patches from the Linux kernel to older\nkernel versions (Android Security). My usual workflow for simple\npatches is:\n\n1. try `git am patch.txt`.\n2. if that fails try `git apply -v patch.txt` then add commit message\nmanually.\n3. if that fails try `patch -p1 < patch.txt` then add commit message\nmanually.\n4. if that fails, manually fix bug based on patch.\n\nMy question is, why does `patch` seem to do a better job at applying\npatches than `git am`? It's almost like the `git` tools don't try to fuzz\nthe offsets. Is there a way to tell git to try fuzzing offsets when\napplying patches?\n\nFrom the output of `patch` I ran recently (for a patch that\n`git apply` would not apply):\n\npatching file \nHunk #1 succeeded at 113 (offset -1 lines).\nHunk #2 succeeded at 435 (offset -3 lines).\nHunk #3 succeeded at 693 (offset 2 lines).\nHunk #4 succeeded at 1535 (offset -41 lines).\nHunk #5 succeeded at 1551 (offset -41 lines).\nHunk #6 succeeded at 1574 with fuzz 2 (offset -42 lines).\nHunk #7 succeeded at 1614 (offset -42 lines).\n\nIn fact, `git apply -v` errors for hunk #6.\n\nI would guess maybe that there's an upper limit on the offset searched?\nAlso, I'm not sure what the `fuzz 2` part means exactly, but it seems like\n`git apply` chokes when fuzzing is needed to properly apply a patch.\n\nhttp://stackoverflow.com/q/6336440/1027966\n\n-- \nThanks,\n~Nick Desaulniers\n"},{"id":"311296","messageId":"xmqq37flltuc.fsf@gitster.mtv.corp.google.com","threadId":"45106","inReplyTo":"CAKwvOdn9j=_Ob=xq4ucN6Ar1G537zNiU9ox4iF6o1qO7kPY41A@mail.gmail.com","subject":"Re: fuzzy patch application","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-10T19:38:19Z","receivedAt":"2017-02-10T19:38:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Desaulniers writes:\n\n> I frequently need to backport patches from the Linux kernel to older\n> kernel versions (Android Security)....\n> ...\n> My question is, why does `patch` seem to do a better job at applying\n> patches than `git am`? It's almost like the `git` tools don't try to fuzz\n> the offsets.\n\nYou diagnosed correctly. We do allow offsets but by default no fuzz\nand that is a deliberate design decision made in very early days.\nYou can pass option to reduce context, but that is not the default.\n"},{"id":"311308","messageId":"20170210205749.c72ajnld7zjfjkwj@sigill.intra.peff.net","threadId":"45106","inReplyTo":"CAKwvOdn9j=_Ob=xq4ucN6Ar1G537zNiU9ox4iF6o1qO7kPY41A@mail.gmail.com","subject":"Re: fuzzy patch application","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-10T20:57:50Z","receivedAt":"2017-02-10T20:57:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 10, 2017 at 11:20:59AM -0800, Nick Desaulniers wrote:\n\n> I frequently need to backport patches from the Linux kernel to older\n> kernel versions (Android Security). My usual workflow for simple\n> patches is:\n> \n> 1. try `git am patch.txt`.\n\nThis is not exactly an answer to your question, but \"git am -3\" is often\na better solution than trying to fuzz patches. It assumes the patches\nare Git patches (and record their origin blobs), and that you have that\nblob (which should be true if the patches are based on the normal kernel\nhistory, and you just fetch that history into your repository).\n\nI've found that this often manages to apply patches that \"git apply\"\nwill not by itself. And I also find the resulting conflicts to be much\neasier to deal with than patch's \".rej\" files.\n\n-Peff\n"},{"id":"311317","messageId":"CAGZ79kaG=oqDM=1+rz_zk6Qn-7wAszxPnBtqrkAJS29_qT7SoA@mail.gmail.com","threadId":"45106","inReplyTo":"20170210205749.c72ajnld7zjfjkwj@sigill.intra.peff.net","subject":"Re: fuzzy patch application","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-10T21:37:12Z","receivedAt":"2017-02-10T21:37:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 10, 2017 at 12:57 PM, Jeff King wrote:\n> On Fri, Feb 10, 2017 at 11:20:59AM -0800, Nick Desaulniers wrote:\n>\n>> I frequently need to backport patches from the Linux kernel to older\n>> kernel versions (Android Security). My usual workflow for simple\n>> patches is:\n>>\n>> 1. try `git am patch.txt`.\n>\n> This is not exactly an answer to your question, but \"git am -3\" is often\n> a better solution than trying to fuzz patches. It assumes the patches\n> are Git patches (and record their origin blobs), and that you have that\n> blob (which should be true if the patches are based on the normal kernel\n> history, and you just fetch that history into your repository).\n>\n> I've found that this often manages to apply patches that \"git apply\"\n> will not by itself. And I also find the resulting conflicts to be much\n> easier to deal with than patch's \".rej\" files.\n>\n> -Peff\n\nI have been told this a couple of times before; do we want to make -3\nthe default (in 2.13 then) ?\n"},{"id":"311319","messageId":"20170210214850.2ok62xdmemgotwnt@sigill.intra.peff.net","threadId":"45106","inReplyTo":"CAGZ79kaG=oqDM=1+rz_zk6Qn-7wAszxPnBtqrkAJS29_qT7SoA@mail.gmail.com","subject":"Re: fuzzy patch application","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-10T21:48:51Z","receivedAt":"2017-02-10T21:48:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 10, 2017 at 01:37:12PM -0800, Stefan Beller wrote:\n\n> > This is not exactly an answer to your question, but \"git am -3\" is often\n> > a better solution than trying to fuzz patches. It assumes the patches\n> > are Git patches (and record their origin blobs), and that you have that\n> > blob (which should be true if the patches are based on the normal kernel\n> > history, and you just fetch that history into your repository).\n> >\n> > I've found that this often manages to apply patches that \"git apply\"\n> > will not by itself. And I also find the resulting conflicts to be much\n> > easier to deal with than patch's \".rej\" files.\n> \n> I have been told this a couple of times before; do we want to make -3\n> the default (in 2.13 then) ?\n\nI dunno. I always use it, but I'm not sure if there are any downsides,\naside from a little extra processing time. It does have some\nincompatibilities with other options. And I think it kicks in rename\ndetection (but I might be mis-remembering another feature). That could\nbe surprising, I guess.\n\nThe original dates all the way back to 47f0b6d5d (Fall back to three-way\nmerge when applying a patch., 2005-10-06), but I don't see any rationale\nfor not making it the default. Junio probably could give a better\nanswer.\n\n-Peff\n"},{"id":"311322","messageId":"xmqqy3xdisos.fsf@gitster.mtv.corp.google.com","threadId":"45106","inReplyTo":"20170210214850.2ok62xdmemgotwnt@sigill.intra.peff.net","subject":"Re: fuzzy patch application","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-10T22:31:31Z","receivedAt":"2017-02-10T22:31:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I dunno. I always use it, but I'm not sure if there are any downsides,\n> aside from a little extra processing time. It does have some\n> incompatibilities with other options. And I think it kicks in rename\n> detection (but I might be mis-remembering another feature). That could\n> be surprising, I guess.\n>\n> The original dates all the way back to 47f0b6d5d (Fall back to three-way\n> merge when applying a patch., 2005-10-06), but I don't see any rationale\n> for not making it the default. Junio probably could give a better\n> answer.\n\nNothing deep. Just being cautious by not to enable extra frills by\ndefault, making it strictly o-p-t i-n. As that was a strict fall\nback (i.e. there was no \"if we are going to fall back, we need to\nspend extra cycles to do this extra preparation before we attempt\nthe regular application\"), extra-processing cost was not a concern,\nat least back I wrote it the first time. I do not offhand know if\nthat still holds in the current one that was rewritten in C.\n\nMaking \"am -3\" default may also scare users who are not exactly\ncomfortable with reading \"git diff\" output during a conflicted merge\nand resolving a conflict, but other than that there shouldn't be any\ndownside.\n\n"},{"id":"311326","messageId":"xmqqlgtdiroz.fsf@gitster.mtv.corp.google.com","threadId":"45106","inReplyTo":"xmqqy3xdisos.fsf@gitster.mtv.corp.google.com","subject":"Re: fuzzy patch application","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-10T22:53:00Z","receivedAt":"2017-02-10T22:53:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Making \"am -3\" default may also scare users who are not exactly\n> comfortable with reading \"git diff\" output during a conflicted merge\n> and resolving a conflict, but other than that there shouldn't be any\n> downside.\n\nAnother obvious downside is that there are those of us who prefer to\nrun \"am\" without \"-3\" first to notice that the patch we expect to\napply cleanly does apply cleanly, which gives us a chance to catch\nmistakes. I personally feel that as long as there is a configuration\nthat makes -3 a personal default (with --no-3way override from the\ncommand line), it is a better design not to enable \"-3\" by default\nfor everybody. New people can first learn using both forms and then\nafter they got comfortable with resolving merges, they can choose to\nflip the default for themselves.\n"},{"id":"311327","messageId":"CAKwvOdmvYiu4ApxL7jVh_0WZUMVeC2jJ4Q_Zs5Z5-J6GNH8DdQ@mail.gmail.com","threadId":"45106","inReplyTo":"xmqqlgtdiroz.fsf@gitster.mtv.corp.google.com","subject":"Re: fuzzy patch application","fromName":"Nick Desaulniers","fromEmail":"ndesaulniers@google.com","sentAt":"2017-02-10T22:56:11Z","receivedAt":"2017-02-10T22:56:35Z","isPatch":false,"sender":{"key":"ndesaulniers@google.com","avatar":null},"body":"It's not my call about the defaults, but users can be surprised by\nsuch changes.\n\nFor the dangers related to fuzzing, is there more info? I found\nand old post on this from Linus calling fuzzing dangerous but\nfrom what I could tell about my patch that wouldn't apply\nwithout fuzzing, the only difference in bad hunks was\nwhitespace that had diverged somehow.\n\nOn Fri, Feb 10, 2017 at 2:53 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Making \"am -3\" default may also scare users who are not exactly\n>> comfortable with reading \"git diff\" output during a conflicted merge\n>> and resolving a conflict, but other than that there shouldn't be any\n>> downside.\n>\n> Another obvious downside is that there are those of us who prefer to\n> run \"am\" without \"-3\" first to notice that the patch we expect to\n> apply cleanly does apply cleanly, which gives us a chance to catch\n> mistakes. I personally feel that as long as there is a configuration\n> that makes -3 a personal default (with --no-3way override from the\n> command line), it is a better design not to enable \"-3\" by default\n> for everybody. New people can first learn using both forms and then\n> after they got comfortable with resolving merges, they can choose to\n> flip the default for themselves.\n\n\n\n-- \nThanks,\n~Nick Desaulniers\n"},{"id":"311330","messageId":"xmqqd1epifsr.fsf@gitster.mtv.corp.google.com","threadId":"45106","inReplyTo":"CAKwvOdmvYiu4ApxL7jVh_0WZUMVeC2jJ4Q_Zs5Z5-J6GNH8DdQ@mail.gmail.com","subject":"Re: fuzzy patch application","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-11T03:09:56Z","receivedAt":"2017-02-11T03:10:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Desaulniers writes:\n\n> For the dangers related to fuzzing, is there more info? I found\n> and old post on this from Linus calling fuzzing dangerous but\n> from what I could tell about my patch that wouldn't apply\n> without fuzzing, the only difference in bad hunks was\n> whitespace that had diverged somehow.\n\nIf the \"old post\" is the one he explains why he chose not to allow\nfuzz by default, I think you got all what you need. Basically, he\nwanted you and his users to make sure that the patch they are having\ntrouble with applying can be due to only insignificant difference\nand it is safe to apply with reduced context, instead of blindly\naccepting a fuzzy patch application.\n"}]}