{"thread":{"id":"12577","subject":"[PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","startedAt":"2008-03-08T18:27:09Z","lastAt":"2008-03-09T11:27:00Z","messageCount":5,"participants":["Pierre Habouzit","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71426","messageId":"20080308182709.GA3109@artemis.madism.org","threadId":"12577","inReplyTo":null,"subject":"[PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-08T18:27:09Z","receivedAt":"2008-03-08T18:27:09Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"The previous parser wasn't able to grok:\n* empty lines;\n* annotated patch levels (trailing -pNNN annotations);\n* trailing comments.\n\nNow it even uses the patch level hints as a git apply argument.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n\n  I'd even argue that it's maint material btw :)\n\n git-quiltimport.sh |   19 +++++++++++++++++--\n 1 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/git-quiltimport.sh b/git-quiltimport.sh\nindex 233e5ea..0d8a765 100755\n--- a/git-quiltimport.sh\n+++ b/git-quiltimport.sh\n@@ -63,7 +63,22 @@ tmp_info=\"$tmp_dir/info\"\n commit=$(git rev-parse HEAD)\n \n mkdir $tmp_dir || exit 2\n-for patch_name in $(grep -v '^#' < \"$QUILT_PATCHES/series\" ); do\n+cat \"$QUILT_PATCHES/series\" | while read patch_name level garbage; do\n+\tcase \"$patch_name\" in ''|'#'*) continue;; esac\n+\tcase \"$level\" in\n+\t\t-p*);;\n+\t\t''|'#'*)\n+\t\t\tlevel=;;\n+\t\t*)\n+\t\t\techo \"unable to parse patch level, ignoring it.\"\n+\t\t\tlevel=;;\n+\tesac\n+\tcase \"$garbage\" in\n+\t\t''|'#'*);;\n+\t\t*)\n+\t\t\techo \"trailing garbage found: $garbage, please fix your series file\"\n+\t\t\texit 1;;\n+\tesac\n \tif ! [ -f \"$QUILT_PATCHES/$patch_name\" ] ; then\n \t\techo \"$patch_name doesn't exist. Skipping.\"\n \t\tcontinue\n@@ -113,7 +128,7 @@ for patch_name in $(grep -v '^#' < \"$QUILT_PATCHES/series\" ); do\n \tfi\n \n \tif [ -z \"$dry_run\" ] ; then\n-\t\tgit apply --index -C1 \"$tmp_patch\" &&\n+\t\tgit apply --index -C1 $level \"$tmp_patch\" &&\n \t\ttree=$(git write-tree) &&\n \t\tcommit=$( (echo \"$SUBJECT\"; echo; cat \"$tmp_msg\") | git commit-tree $tree -p $commit) &&\n \t\tgit update-ref -m \"quiltimport: $patch_name\" HEAD $commit || exit 4\n-- \n1.5.4.3.593.g07854.dirty\n\n"},{"id":"71487","messageId":"7vk5kcsh06.fsf@gitster.siamese.dyndns.org","threadId":"12577","inReplyTo":"20080308182709.GA3109@artemis.madism.org","subject":"Re: [PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-09T03:39:21Z","receivedAt":"2008-03-09T03:39:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>   I'd even argue that it's maint material btw :)\n\nI don't know.\n\nYou made the loop a subshell but I think you can redirect into the while\nloop without an extra \"cat\" process.\n\nCan a patch name contain $IFS whitespace characters?  If so, this patch\nwould regress them.  Otherwise it looks fine.\n"},{"id":"71499","messageId":"20080309095918.GA23924@artemis.madism.org","threadId":"12577","inReplyTo":"7vk5kcsh06.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-09T09:59:19Z","receivedAt":"2008-03-09T09:59:19Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Mar 09, 2008 at 03:39:21AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> >   I'd even argue that it's maint material btw :)\n> \n> I don't know.\n\nWell not groking the -pNNN makes it useless for me in its current form, 80% of\nthe quilt series I use have them (and right now it says: I cannot apply patch\n'foo-bar.patch -p1').\n\n> You made the loop a subshell but I think you can redirect into the while\n> loop without an extra \"cat\" process.\n\nWell probably yeah, I could use an exec -- < series or an alike trick\nfor sure.\n\n> Can a patch name contain $IFS whitespace characters?\n\n  I tested and quilt says that:\n\n$ quilt new a\\ b.patch\nPatch a is now on top\n\nAnd when I quilt refresh it, it creates a patch named 'a'.  (and yes, I thought\nof this and tested it prior to sending the patch).\n\n> If so, this patch would regress them.  Otherwise it looks fine.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"71500","messageId":"7vpru4p5g1.fsf@gitster.siamese.dyndns.org","threadId":"12577","inReplyTo":"20080309095918.GA23924@artemis.madism.org","subject":"Re: [PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-09T10:17:18Z","receivedAt":"2008-03-09T10:17:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>> You made the loop a subshell but I think you can redirect into the while\n>> loop without an extra \"cat\" process.\n>\n> Well probably yeah, I could use an exec -- < series or an alike trick\n> for sure.\n\nIt seems many people forget that it is perfectly fine to redirect into\nwhile loop itself, like\n\n\twhile read foo bar\n        do\n        \tdo a lot of stuff\n\tdone <input-file\n\n>> Can a patch name contain $IFS whitespace characters?\n>\n>   I tested and quilt says that:\n>\n> $ quilt new a\\ b.patch\n> Patch a is now on top\n>\n> And when I quilt refresh it, it creates a patch named 'a'.  (and yes, I thought\n> of this and tested it prior to sending the patch).\n>\n>> If so, this patch would regress them.  Otherwise it looks fine.\n\nOk, somehow in an earlier patch to quiltimport I picked up an incorrect\nnotion that they could contain a blank, but then that's fine.\n\nWill queue, but I am very well into my merge binge for tonight (and it\nhasn't finished even though it is past 3 AM, sheesh), so it will be in the\nnext round.\n"},{"id":"71509","messageId":"20080309112700.GB23924@artemis.madism.org","threadId":"12577","inReplyTo":"7vpru4p5g1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-quiltimport: better parser to grok \"enhanced\" series files.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-09T11:27:00Z","receivedAt":"2008-03-09T11:27:00Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Mar 09, 2008 at 10:17:18AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> >> You made the loop a subshell but I think you can redirect into the while\n> >> loop without an extra \"cat\" process.\n> >\n> > Well probably yeah, I could use an exec -- < series or an alike trick\n> > for sure.\n> \n> It seems many people forget that it is perfectly fine to redirect into\n> while loop itself, like\n> \n> \twhile read foo bar\n>         do\n>         \tdo a lot of stuff\n> \tdone <input-file\n\nOh right, well, I assume you already squashed that on top of my patch :)\n\n> Ok, somehow in an earlier patch to quiltimport I picked up an incorrect\n> notion that they could contain a blank, but then that's fine.\n\n  Well, maybe they could, but quilt doesn't get it right.\n\n> Will queue, but I am very well into my merge binge for tonight (and it\n> hasn't finished even though it is past 3 AM, sheesh), so it will be in the\n> next round.\n\n  That's obviously fine by me, thanks :)\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"}]}