{"thread":{"id":"19746","subject":"Question about fixing windows bug reading graft data","startedAt":"2009-06-08T17:19:37Z","lastAt":"2009-07-27T19:31:35Z","messageCount":4,"participants":["Kelly F. Hickel","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"115821","messageId":"63BEA5E623E09F4D92233FB12A9F794303117E06@emailmn.mqsoftware.com","threadId":"19746","inReplyTo":null,"subject":"Question about fixing windows bug reading graft data","fromName":"Kelly F. Hickel","fromEmail":"kfh@mqsoftware.com","sentAt":"2009-06-08T17:19:37Z","receivedAt":"2009-06-08T17:19:37Z","isPatch":false,"sender":{"key":"kfh@mqsoftware.com","avatar":null},"body":"Hi All,\n\tRan into a bug trying to use grafts on windows with cygwin git\nversion 1.6.1.2.  I've verified that the bug is still there in the\nlatest source, and was going to submit a patch, but then I noticed that\nthere seem to be more occurrences in commit.c, and wondered if there was\na better way to fix it than what I had first come up with.\n\nThe bug, is that in in commit.c, the code strips '\\n', but not '\\r', so\nthe code says the graft data is bad:\nstruct commit_graft *read_graft_line(char *buf, int len) {\n        /* The format is just \"Commit Parent1 Parent2 ...\\n\" */\n        int i;\n        struct commit_graft *graft = NULL;\n\n        if (buf[len-1] == '\\n')\n                buf[--len] = 0;\n        if (buf[0] == '#' || buf[0] == '\\0')\n                return NULL;\n        if ((len + 1) % 41) {\n        bad_graft_data:\n                error(\"bad graft data: %s\", buf);\n                free(graft);\n                return NULL;\n        }\n\nMy first plan was to fix it the way that xdiff-interface.c handles it,\nassuming that was \"the Git way\" to deal with CRLF:\n        /* Exclude terminating newline (and cr) from matching */\n        if (len > 0 && line[len-1] == '\\n') {\n                if (len > 1 && line[len-2] == '\\r')\n                        len -= 2;\n                else\n                        len--;\n        }\n\nBut I noticed that there seemed to be several checks for '\\n' in\ncommit.c that didn't check for '\\r', and wondered if there was a reason,\nor if there'd be a better way to handle it.....\n\n\n\n--\n \nKelly F. Hickel\nSenior Product Architect\nMQSoftware, Inc.\n952-345-8677 Office\n952-345-8721 Fax\nkfh@mqsoftware.com\nwww.mqsoftware.com\nCertified IBM SOA Specialty\nYour Full Service Provider for IBM WebSphere Learn more at\nwww.mqsoftware.com \n"},{"id":"118884","messageId":"63BEA5E623E09F4D92233FB12A9F7943033B2744@emailmn.mqsoftware.com","threadId":"19746","inReplyTo":"63BEA5E623E09F4D92233FB12A9F794303117E06@emailmn.mqsoftware.com","subject":"RE: Question about fixing windows bug reading graft data","fromName":"Kelly F. Hickel","fromEmail":"kfh@mqsoftware.com","sentAt":"2009-07-27T17:33:50Z","receivedAt":"2009-07-27T17:33:50Z","isPatch":false,"sender":{"key":"kfh@mqsoftware.com","avatar":null},"body":"OK, so the 10th copy of the msysGit Herald post has shamed me out of\nhiding!\nI posted the below awhile back, and since I volunteered to fix something\n(if given a few pointers), I felt I had \"Done My Duty\" to the Git world!\n\nBut now Dscho has made me rip the blinders from my eyes, to try once\nagain to offer to fix this bug (even though I found it in Cygwin Git and\ndon't use msysgit, but hey, Git is Git, right!?!?!)....\n\nSo, here I am, gonna put myself out there, willing to suffer ridicule,\netc!\n\nAny guidance on \"the Git way\" to properly deal with \\r in a meta\nfile????  Show me the light!\n\n\n--\n\nKelly F. Hickel\nSenior Product Architect\nMQSoftware, Inc.\n952-345-8677 Office\n952-345-8721 Fax\nkfh@mqsoftware.com\nwww.mqsoftware.com\nCertified IBM SOA Specialty\nYour Full Service Provider for IBM WebSphere\nLearn more at www.mqsoftware.com \n\n\n> -----Original Message-----\n> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n> Behalf Of Kelly F. Hickel\n> Sent: Monday, June 08, 2009 12:20 PM\n> To: git@vger.kernel.org\n> Subject: Question about fixing windows bug reading graft data\n> \n> Hi All,\n> \tRan into a bug trying to use grafts on windows with cygwin git\n> version 1.6.1.2.  I've verified that the bug is still there in the\n> latest source, and was going to submit a patch, but then I noticed\nthat\n> there seem to be more occurrences in commit.c, and wondered if there\n> was\n> a better way to fix it than what I had first come up with.\n> \n> The bug, is that in in commit.c, the code strips '\\n', but not '\\r',\nso\n> the code says the graft data is bad:\n> struct commit_graft *read_graft_line(char *buf, int len) {\n>         /* The format is just \"Commit Parent1 Parent2 ...\\n\" */\n>         int i;\n>         struct commit_graft *graft = NULL;\n> \n>         if (buf[len-1] == '\\n')\n>                 buf[--len] = 0;\n>         if (buf[0] == '#' || buf[0] == '\\0')\n>                 return NULL;\n>         if ((len + 1) % 41) {\n>         bad_graft_data:\n>                 error(\"bad graft data: %s\", buf);\n>                 free(graft);\n>                 return NULL;\n>         }\n> \n> My first plan was to fix it the way that xdiff-interface.c handles it,\n> assuming that was \"the Git way\" to deal with CRLF:\n>         /* Exclude terminating newline (and cr) from matching */\n>         if (len > 0 && line[len-1] == '\\n') {\n>                 if (len > 1 && line[len-2] == '\\r')\n>                         len -= 2;\n>                 else\n>                         len--;\n>         }\n> \n> But I noticed that there seemed to be several checks for '\\n' in\n> commit.c that didn't check for '\\r', and wondered if there was a\n> reason,\n> or if there'd be a better way to handle it.....\n> \n> \n> \n> --\n> \n> Kelly F. Hickel\n> Senior Product Architect\n> MQSoftware, Inc.\n> 952-345-8677 Office\n> 952-345-8721 Fax\n> kfh@mqsoftware.com\n> www.mqsoftware.com\n> Certified IBM SOA Specialty\n> Your Full Service Provider for IBM WebSphere Learn more at\n> www.mqsoftware.com\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"118885","messageId":"alpine.DEB.1.00.0907271948130.6883@intel-tinevez-2-302","threadId":"19746","inReplyTo":"63BEA5E623E09F4D92233FB12A9F7943033B2744@emailmn.mqsoftware.com","subject":"RE: Question about fixing windows bug reading graft data","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-07-27T17:55:09Z","receivedAt":"2009-07-27T17:55:09Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Jul 2009, Kelly F. Hickel wrote:\n\n> OK, so the 10th copy of the msysGit Herald post has shamed me out of\n> hiding!\n\n;-)\n\n> > The bug, is that in in commit.c, the code strips '\\n', but not '\\r', \n> > so the code says the graft data is bad:\n> >\n> > struct commit_graft *read_graft_line(char *buf, int len) {\n> >         /* The format is just \"Commit Parent1 Parent2 ...\\n\" */\n> >         int i;\n> >         struct commit_graft *graft = NULL;\n> > \n> >         if (buf[len-1] == '\\n')\n> >                 buf[--len] = 0;\n> >         if (buf[0] == '#' || buf[0] == '\\0')\n> >                 return NULL;\n> >         if ((len + 1) % 41) {\n> >         bad_graft_data:\n> >                 error(\"bad graft data: %s\", buf);\n> >                 free(graft);\n> >                 return NULL;\n> >         }\n> > \n> > My first plan was to fix it the way that xdiff-interface.c handles it,\n> > assuming that was \"the Git way\" to deal with CRLF:\n> >         /* Exclude terminating newline (and cr) from matching */\n> >         if (len > 0 && line[len-1] == '\\n') {\n> >                 if (len > 1 && line[len-2] == '\\r')\n> >                         len -= 2;\n> >                 else\n> >                         len--;\n> >         }\n> > \n> > But I noticed that there seemed to be several checks for '\\n' in \n> > commit.c that didn't check for '\\r', and wondered if there was a \n> > reason, or if there'd be a better way to handle it.....\n\nI think that you really only have to handle text files read from the file \nsystem.  That is not the case for commit object parsers: commit _objects_ \nare required to have LF line endings.\n\nBut a few files come to mind which might have CR/LF line endings and need \nto be interpreted correctly by Git: \"grafts\", as you pointed out, but also \nthe refs and of course the config.\n\nIt would probably be a good idea to have something like\n\n\tstatic inline fix_line_ending(char *line, int len)\n\t{\n\t\tif (len > 0 && line[len-1] == '\\n')\n\t\t\tline[len-1 - (len > 1 && line[len-2] == '\\r')] = '\\0';\n\t}\n\nin cache.h, and use it in said places.\n\nOf course, the hassle is to find all those places ;-)\n\nThanks,\nDscho\n"},{"id":"118897","messageId":"alpine.DEB.1.00.0907272129550.8306@pacific.mpi-cbg.de","threadId":"19746","inReplyTo":"alpine.DEB.1.00.0907271948130.6883@intel-tinevez-2-302","subject":"RE: Question about fixing windows bug reading graft data","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-07-27T19:31:35Z","receivedAt":"2009-07-27T19:31:35Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Jul 2009, Johannes Schindelin wrote:\n\n> But a few files come to mind which might have CR/LF line endings and \n> need to be interpreted correctly by Git: \"grafts\", as you pointed out, \n> but also the refs and of course the config.\n\nForgot at least one: objects/info/alternates.\n\nAnd this one was a very real issue on my side, when I installed an \nalternate on somebody's Windows computer, and it did not work.  Should \nhave used vi to begin with... :-)\n\nCiao,\nDscho\n"}]}