{"thread":{"id":"4659","subject":"[PATCH 0/2] resurrect format-patch's patch id checking","startedAt":"2006-06-25T01:50:28Z","lastAt":"2006-06-27T20:38:04Z","messageCount":20,"participants":["Johannes Schindelin","Junio C Hamano","Martin Langhoff","Martin Langhoff (CatalystIT)"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"22499","messageId":"Pine.LNX.4.63.0606250349280.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":null,"subject":"[PATCH 0/2] resurrect format-patch's patch id checking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-25T01:50:28Z","receivedAt":"2006-06-25T01:50:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWith these patches, you can say\n\n\tgit format-patch --ignore-if-in-upstream upstream\n\nto get a series of only the patches which are in HEAD, but not upstream.\n\nThe first patch adds diff_flush_patch_id() to calculate the patch id, after\na diff queue was set up. (Earlier, I sent out a version which also adds\na command line option to the diff family, but I'll postpone that until\nTimo's patches went in).\n\nThe second patch adds the actual patch id checking. There are two tricky\nthings involved:\n\n\t- I add a pseudo-object for each patch of the upstream, which has\n\t the patch id as sha1. This is no real object, but since we rely\n\t on the hashes being unique for all practical purposes, and since\n\t it has parsed == 0, it should be no problem.\n\n\t- To add the patch ids of the upstream, the revision walker must\n\t be called twice.\n\n\t So, if format-patch was called with a range \"a..b\" (a single\n\t revision \"a\" is handled as \"a..HEAD\" by format-patch), a revision\n\t walker is set up for \"b..a\", and the patch ids are calculated and\n\t stored. This is done by toggling the UNINTERESTING bits of both\n\t pending objects.\n\n\t After that, the flags of all objects are reset to 0, so that the\n\t revisions can be walked again. The flags of the two pending objects\n\t are then reset to their original state.\n\nNote that \"--numbered\" still works.\n\nWARNING: since it is quite late in this part of the world, _and_ I am known\nto produce not-always-optimal patches which could eat your children, please\ngive this a good beating.\n\nCiao,\nDscho\n"},{"id":"22608","messageId":"Pine.LNX.4.63.0606261728340.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606250349280.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T15:33:40Z","receivedAt":"2006-06-26T15:33:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt is cleaner, and it describes better what is the idea behind the code.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tThis goes on top of the --ignore-if-in-upstream patch.\n\n\tOn Sun, 25 Jun 2006, Johannes Schindelin wrote:\n\n\t> - To add the patch ids of the upstream, the revision walker must\n\t> be called twice.\n\t> \n\t> So, if format-patch was called with a range \"a..b\" (a single\n\t> revision \"a\" is handled as \"a..HEAD\" by format-patch), a \n\t> revision walker is set up for \"b..a\", and the patch ids are \n\t> calculated and stored. This is done by toggling the \n\t> UNINTERESTING bits of both pending objects.\n\t> \n\t> After that, the flags of all objects are reset to 0, so that the\n\t> revisions can be walked again. The flags of the two pending \n\t> objects are then reset to their original state.\n\n\tIt is not clean to reset the flags of all objects to 0. Instead, \n\tthe commits are walked directly. Not that it matters in that\n\tparticular case (the only read objects _are_ these commits).\n\n builtin-log.c | 12 ++----------\n 1 files changed, 2 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex e78a9a4..e2cd975 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -160,15 +160,6 @@ static void reopen_stdout(struct commit \n \tfreopen(filename, \"w\", stdout);\n }\n \n-static void reset_all_objects_flags()\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < obj_allocs; i++)\n-\t\tif (objs[i])\n-\t\t\tobjs[i]->flags = 0;\n-}\n-\n static int get_patch_id(struct commit *commit, struct diff_options *options,\n \t\tunsigned char *sha1)\n {\n@@ -220,7 +211,8 @@ static void get_patch_ids(struct rev_inf\n \t}\n \n \t/* reset for next revision walk */\n-\treset_all_objects_flags();\n+\tclear_commit_marks((struct commit *)o1, SEEN | UNINTERESTING);\n+\tclear_commit_marks((struct commit *)o2, SEEN | UNINTERESTING);\n \to1->flags = flags1;\n \to2->flags = flags2;\n }\n-- \n1.4.1.rc1.gc792\n"},{"id":"22609","messageId":"7vhd27j3v8.fsf@assigned-by-dhcp.cox.net","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606261728340.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-26T16:09:15Z","receivedAt":"2006-06-26T16:09:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> \tIt is not clean to reset the flags of all objects to 0. Instead, \n> \tthe commits are walked directly. Not that it matters in that\n> \tparticular case (the only read objects _are_ these commits).\n\nI think this makes sense, but the clear-commit-marks function\nitself looks fishy. I suspect a parent that has not been parsed\ncould be already marked in which case the current code would\nleave it marked. Don't we need this perhaps?\n\ndiff --git a/commit.c b/commit.c\nindex 946615d..69fbc41 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -397,8 +397,7 @@ void clear_commit_marks(struct commit *c\n \tcommit->object.flags &= ~mark;\n \twhile (parents) {\n \t\tstruct commit *parent = parents->item;\n-\t\tif (parent && parent->object.parsed &&\n-\t\t (parent->object.flags & mark))\n+\t\tif (parent && (parent->object.flags & mark))\n \t\t\tclear_commit_marks(parent, mark);\n \t\tparents = parents->next;\n \t}\n"},{"id":"22613","messageId":"46a038f90606261520k7df8cb3ci7a4a609644e0be12@mail.gmail.com","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606261728340.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-26T22:20:51Z","receivedAt":"2006-06-26T22:20:51Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"git-format-patch is rather broken on pu right now (pre these patches).\nIn my test git repo, git-format-patch.sh called with 2 parameters,\nlike\n\n git-format-patch \n\nwhen HEAD != myhead does the right thing, but git format-patch\ndoesn't, it seems to be messing up and not finding the merge base\ncorrectly:\n\n 0001-Initial-revision-of-git-the-information-manager-from-hell.txt\n\nAnd it errors out with ignore-if-in-upstream:\n\n $ ./git format-patch --ignore-if-in-upstream -o .patches origin master\n fatal: Not a range.\n\nI'll retest later with these patches and post again.\n\n\nmartin\n"},{"id":"22624","messageId":"Pine.LNX.4.63.0606270038200.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"46a038f90606261520k7df8cb3ci7a4a609644e0be12@mail.gmail.com","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T22:39:42Z","receivedAt":"2006-06-26T22:39:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jun 2006, Martin Langhoff wrote:\n\n> And it errors out with ignore-if-in-upstream:\n> \n> $ ./git format-patch --ignore-if-in-upstream -o .patches origin master\n> fatal: Not a range.\n\nCould you test with \"origin..master\" instead of \"origin master\"?\n\nThanks,\nDscho\n"},{"id":"22618","messageId":"Pine.LNX.4.63.0606270042420.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"7vhd27j3v8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T22:44:12Z","receivedAt":"2006-06-26T22:44:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Jun 2006, Junio C Hamano wrote:\n\n> diff --git a/commit.c b/commit.c\n> index 946615d..69fbc41 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -397,8 +397,7 @@ void clear_commit_marks(struct commit *c\n> \tcommit->object.flags &= ~mark;\n> \twhile (parents) {\n> \t\tstruct commit *parent = parents->item;\n> -\t\tif (parent && parent->object.parsed &&\n> -\t\t (parent->object.flags & mark))\n> +\t\tif (parent && (parent->object.flags & mark))\n\nThis is probably not necessary for existing users, but it's a good change \nfor the future: new users might be surprised to learn that there are \nunparsed objects, which still want to be handled.\n\nCiao,\nDscho\n"},{"id":"22619","messageId":"7vr71bh6sv.fsf@assigned-by-dhcp.cox.net","threadId":"4659","inReplyTo":"46a038f90606261520k7df8cb3ci7a4a609644e0be12@mail.gmail.com","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-26T22:48:48Z","receivedAt":"2006-06-26T22:48:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'll be pushing out a new test for format-patch shortly in\n\"next\".\n\n>From ece3c67f9c8f0074cae76204a648cbfc6075bb44 Mon Sep 17 00:00:00 2001\nFrom: Junio C Hamano \nDate: Mon, 26 Jun 2006 15:40:09 -0700\nSubject: [PATCH] t4014: add format-patch --ignore-if-in-upstream test\n\nSigned-off-by: Junio C Hamano \n---\n t/t4014-format-patch.sh | 69 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 69 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nnew file mode 100755\nindex 0000000..c044044\n--- /dev/null\n+++ b/t/t4014-format-patch.sh\n@@ -0,0 +1,69 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 Junio C Hamano\n+#\n+\n+test_description='Format-patch skipping already incorporated patches'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\n+\tfor i in 1 2 3 4 5 6 7 8 9 10; do echo \"$i\"; done >file &&\n+\tgit add file &&\n+\tgit commit -m Initial &&\n+\tgit checkout -b side &&\n+\n+\tfor i in 1 2 5 6 A B C 7 8 9 10; do echo \"$i\"; done >file &&\n+\tgit update-index file &&\n+\tgit commit -m \"Side change #1\" &&\n+\n+\tfor i in D E F; do echo \"$i\"; done >>file &&\n+\tgit update-index file &&\n+\tgit commit -m \"Side change #2\" &&\n+\tgit tag C1 &&\n+\n+\tfor i in 5 6 1 2 3 A 4 B C 7 8 9 10 D E F; do echo \"$i\"; done >file &&\n+\tgit update-index file &&\n+\tgit commit -m \"Side change #3\" &&\n+\n+\tgit checkout master &&\n+\tgit diff-tree -p C1 | git apply --index &&\n+\tgit commit -m \"Master accepts moral equivalent of #1\"\n+\n+'\n+\n+test_expect_success \"format-patch --ignore-if-in-upstream\" '\n+\n+\tgit format-patch --stdout master..side >patch0 &&\n+\tcnt=`grep \"^From \" patch0 | wc -l` &&\n+\ttest \"$cnt\" = 3\n+\n+'\n+\n+test_expect_success \"format-patch --ignore-if-in-upstream\" '\n+\n+\tgit format-patch --stdout \\\n+\t\t--ignore-if-in-upstream master..side >patch1 &&\n+\tcnt=`grep \"^From \" patch1 | wc -l` &&\n+\ttest \"$cnt\" = 2\n+\n+'\n+\n+test_expect_success \"format-patch result applies\" '\n+\n+\tgit checkout -b rebuild-0 master &&\n+\tgit am -3 patch0 &&\n+\tcnt=`git rev-list master.. | wc -l` &&\n+\ttest \"$cnt\" = 2\n+'\n+\n+test_expect_success \"format-patch --ignore-if-in-upstream result applies\" '\n+\n+\tgit checkout -b rebuild-1 master &&\n+\tgit am -3 patch1 &&\n+\tcnt=`git rev-list master.. | wc -l` &&\n+\ttest \"$cnt\" = 2\n+'\n+\n+test_done\n-- \n1.4.1.rc1.g96b82\n"},{"id":"22620","messageId":"46a038f90606261550p145b20a6gbe960e0abc16e8a4@mail.gmail.com","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606270038200.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-26T22:50:43Z","receivedAt":"2006-06-26T22:50:43Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 6/27/06, Johannes Schindelin wrote:\n> Hi,\n>\n> On Tue, 27 Jun 2006, Martin Langhoff wrote:\n>\n> > And it errors out with ignore-if-in-upstream:\n> >\n> > $ ./git format-patch --ignore-if-in-upstream -o .patches origin master\n> > fatal: Not a range.\n>\n> Could you test with \"origin..master\" instead of \"origin master\"?\n\nFunny you mention that! Now it works ;-) and it even produces the\npatches I would expect. There is something strange though. I have a\nrepo with ~150 pending patches to push, of which git-cherry spots ~100\nas already merged upstream. So the old git-format-patch.sh would spit\n50 patches, and the initial C version would do 150.\n\nNow this version gives me 50 patches, regardless of\n--ignore-if-in-upstream. Is that expected?\n\nAlso, if the two heads are identical, it still says 'Fatal: Not a\nrange\", but that isn't so important.\n\ncheers,\n\n\nmartin\n"},{"id":"22621","messageId":"Pine.LNX.4.63.0606270056440.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"7vr71bh6sv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T22:57:20Z","receivedAt":"2006-06-26T22:57:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Jun 2006, Junio C Hamano wrote:\n\n> I'll be pushing out a new test for format-patch shortly in\n> \"next\".\n\nThanks. I had a little test, but it was not nearly as complete as I liked \nit...\n\nCiao,\nDscho\n"},{"id":"22622","messageId":"Pine.LNX.4.63.0606270057260.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"46a038f90606261550p145b20a6gbe960e0abc16e8a4@mail.gmail.com","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T23:00:57Z","receivedAt":"2006-06-26T23:00:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jun 2006, Martin Langhoff wrote:\n\n> On 6/27/06, Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Tue, 27 Jun 2006, Martin Langhoff wrote:\n> > \n> > > And it errors out with ignore-if-in-upstream:\n> > >\n> > > $ ./git format-patch --ignore-if-in-upstream -o .patches origin master\n> > > fatal: Not a range.\n> > \n> > Could you test with \"origin..master\" instead of \"origin master\"?\n> \n> Funny you mention that! Now it works ;-) and it even produces the\n> patches I would expect.\n\nThe funny thing is: I did something to account for the old syntax, but \nonly if you specified _one_ ref, not _two_. It would be easy, but is it \nneeded? (I.e. are your fingers so trained on it?)\n\n> There is something strange though. I have a repo with ~150 pending \n> patches to push, of which git-cherry spots ~100 as already merged \n> upstream. So the old git-format-patch.sh would spit 50 patches, and the \n> initial C version would do 150.\n> \n> Now this version gives me 50 patches, regardless of\n> --ignore-if-in-upstream. Is that expected?\n\nHell, no! Something is really wrong there.\n\nWhat does \"git-rev-list their..my | wc\" say?\n\n> Also, if the two heads are identical, it still says 'Fatal: Not a\n> range\", but that isn't so important.\n\nThis is a consequence of my being too lazy to support the old \"theirs \nmine\" syntax (see above).\n\nCiao,\nDscho\n"},{"id":"22623","messageId":"7vk673h62w.fsf@assigned-by-dhcp.cox.net","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606270057260.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-26T23:04:23Z","receivedAt":"2006-06-26T23:04:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > Could you test with \"origin..master\" instead of \"origin master\"?\n>> \n>> Funny you mention that! Now it works ;-) and it even produces the\n>> patches I would expect.\n>\n> The funny thing is: I did something to account for the old syntax, but \n> only if you specified _one_ ref, not _two_. It would be easy, but is it \n> needed? (I.e. are your fingers so trained on it?)\n\nIf possible I'd rather correct the two syntaxes once and for all now.\nMaybe accept two with a warning for deprecation?\n"},{"id":"22626","messageId":"Pine.LNX.4.63.0606270113360.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"7vk673h62w.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] format-patch: support really old non-range syntax, with a warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T23:14:59Z","receivedAt":"2006-06-26T23:14:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nNow you can say (again)\n\n\tgit format-patch \n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\n\tRe: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some \n\tadhocery\n\n\tOn Mon, 26 Jun 2006, Junio C Hamano wrote:\n\n\t> Johannes Schindelin writes:\n\t> \n\t> >> > Could you test with \"origin..master\" instead of \"origin master\"?\n\t> >> \n\t> >> Funny you mention that! Now it works ;-) and it even produces the\n\t> >> patches I would expect.\n\t> >\n\t> > The funny thing is: I did something to account for the old syntax, but \n\t> > only if you specified _one_ ref, not _two_. It would be easy, but is it \n\t> > needed? (I.e. are your fingers so trained on it?)\n\t> \n\t> If possible I'd rather correct the two syntaxes once and for all now.\n\t> Maybe accept two with a warning for deprecation?\n\n\tHere you are. (Tested once -- works great!)\n\n builtin-log.c | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 44d2d13..64b2830 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -260,6 +260,11 @@ int cmd_format_patch(int argc, const cha\n \tif (rev.pending.nr == 1) {\n \t\trev.pending.objects[0].item->flags |= UNINTERESTING;\n \t\tadd_head(&rev);\n+\t} else if (rev.pending.nr == 2\n+\t\t\t&& !(rev.pending.objects[0].item->flags & UNINTERESTING)\n+\t\t\t&& !(rev.pending.objects[1].item->flags & UNINTERESTING)) {\n+\t\trev.pending.objects[0].item->flags |= UNINTERESTING;\n+\t\tfprintf(stderr, \"WARNING: obsolete syntax (no range)!\\n\");\n \t}\n \n \tif (!use_stdout)\n"},{"id":"22625","messageId":"44A06A8D.7080202@catalyst.net.nz","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606270057260.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Martin Langhoff (CatalystIT)","fromEmail":"martin@catalyst.net.nz","sentAt":"2006-06-26T23:15:25Z","receivedAt":"2006-06-26T23:15:25Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Johannes Schindelin wrote:\n> The funny thing is: I did something to account for the old syntax, but \n> only if you specified _one_ ref, not _two_. It would be easy, but is it \n> needed? (I.e. are your fingers so trained on it?)\n\nMy fingers are retrainable ;-)\n\n>>There is something strange though. I have a repo with ~150 pending \n>>patches to push, of which git-cherry spots ~100 as already merged \n>>upstream. So the old git-format-patch.sh would spit 50 patches, and the \n>>initial C version would do 150.\n>>\n>>Now this version gives me 50 patches, regardless of\n>>--ignore-if-in-upstream. Is that expected?\n> \n> \n> Hell, no! Something is really wrong there.\n> \n> What does \"git-rev-list their..my | wc\" say?\n\nOk, I cooked the numbers up a bit, it was 60 total, with 10 merged \nupstream. Here's what I have today:\n\n $ git-cherry svnhead..master | grep -c '+'\n 52\n $ git-rev-list svnhead..master | wc -l\n 61\n\n $ ~/local/git/git-format-patch.sh -o .patchesold svnhead master\n ...\n $ ls .patchesold | wc -l\n 52\n\n $ ~/local/git/git format-patch -o .patchesnewall svnhead..master\n ...\n $ ls .patchesnewall/ | wc -l\n 53\n\n $ ~/local/git/git format-patch --ignore-if-in-upstream -o \n.patchesnewignore svnhead..master\n ...\n $ ls .patchesnewignore | wc -l\n 52\n\nThis is a public repo --\n\nmaster tracks origin which is\nhttp://git.catalyst.net.nz/git/elgg-r2.git#nzvleportfolio\n\nsvnhead is\nhttp://git.catalyst.net.nz/git/elgg-r2.git#svnhead\n\ncheers,\n\n\nm\n-- \n-----------------------------------------------------------------------\nMartin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington\nWEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St\nOFFICE: +64(4)916-7224 MOB: +64(21)364-017\n Make things as simple as possible, but no simpler - Einstein\n-----------------------------------------------------------------------\n"},{"id":"22628","messageId":"Pine.LNX.4.63.0606270117470.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"44A06A8D.7080202@catalyst.net.nz","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-26T23:21:47Z","receivedAt":"2006-06-26T23:21:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jun 2006, Martin Langhoff (CatalystIT) wrote:\n\n> > > There is something strange though. I have a repo with ~150 pending patches\n> > > to push, of which git-cherry spots ~100 as already merged upstream. So the\n> > > old git-format-patch.sh would spit 50 patches, and the initial C version\n> > > would do 150.\n> > > \n> > > Now this version gives me 50 patches, regardless of\n> > > --ignore-if-in-upstream. Is that expected?\n> > \n> > \n> > Hell, no! Something is really wrong there.\n> > \n> > What does \"git-rev-list their..my | wc\" say?\n> \n> Ok, I cooked the numbers up a bit, it was 60 total, with 10 merged upstream.\n> Here's what I have today:\n> \n> $ git-cherry svnhead..master | grep -c '+'\n> 52\n> $ git-rev-list svnhead..master | wc -l\n> 61\n> \n> $ ~/local/git/git-format-patch.sh -o .patchesold svnhead master\n> ...\n> $ ls .patchesold | wc -l\n> 52\n> \n> $ ~/local/git/git format-patch -o .patchesnewall svnhead..master\n> ...\n> $ ls .patchesnewall/ | wc -l\n> 53\n\nThat is very, very strange. One more! Do you have any idea which patch it \nis (53 being one more than 52 I suspect there is one slipping through)?\n\n> $ ~/local/git/git format-patch --ignore-if-in-upstream -o .patchesnewignore\n> svnhead..master\n> ...\n> $ ls .patchesnewignore | wc -l\n> 52\n\nCould it be that the other patches are merges? format-patch _must_ ignore \nthese.\n\n> This is a public repo --\n> \n> master tracks origin which is\n> http://git.catalyst.net.nz/git/elgg-r2.git#nzvleportfolio\n> \n> svnhead is\n> http://git.catalyst.net.nz/git/elgg-r2.git#svnhead\n\nEven if I do not use cogito, I will fetch them. However, that must wait \nfor tomorrow, as I can no longer keep awake.\n\nCiao,\nDscho \"where do you put the middle name if your nick is only one word?\"\n"},{"id":"22645","messageId":"Pine.LNX.4.63.0606271016450.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"44A06A8D.7080202@catalyst.net.nz","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-27T08:31:46Z","receivedAt":"2006-06-27T08:31:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI just cloned your repo, and as far as I can tell, the latest commit is on \nJune 23rd. So your numbers should be the same as mine. But not all are.\n\nOn Tue, 27 Jun 2006, Martin Langhoff (CatalystIT) wrote:\n\n> Ok, I cooked the numbers up a bit, it was 60 total, with 10 merged upstream.\n> Here's what I have today:\n> \n> $ git-cherry svnhead..master | grep -c '+'\n> 52\n\nHere, it is 51!\n\nThe problem I see: from the 53 non-merges in nzvleportfolio (who makes up \nyour branch names anyway?), there are two already in upstream: e3f56c and \n7e448c5c. So it really should be 51.\n\n> $ git-rev-list svnhead..master | wc -l\n> 61\n\nSame on my repo.\n\n> $ ~/local/git/git-format-patch.sh -o .patchesold svnhead master\n> ...\n> $ ls .patchesold | wc -l\n> 52\n\nI guess this propagates from git-cherry. (Did not test here, since I do \nnot have an old git-format-patch.sh handy, and am too lazy to get the last \nversion from my git repo.)\n\n> $ ~/local/git/git format-patch -o .patchesnewall svnhead..master\n> ...\n> $ ls .patchesnewall/ | wc -l\n> 53\n\nSame on my repo. Correct, since\n\"git rev-list --parents svnhead..master | grep -c \\ .*\\ \"\nyields 8, which is exactly 61 - 53.\n\n> $ ~/local/git/git format-patch --ignore-if-in-upstream -o .patchesnewignore\n> svnhead..master\n> ...\n> $ ls .patchesnewignore | wc -l\n> 52\n\nAgain, I get 51.\n\nI do not know what is happening, but it could be that there was a bug \nfixed lately. I run a slightly modified version of 'next', updated about \n15 minutes ago.\n\nBut anyway, looking at your numbers I take it that the new format-patch \nwith --ignore-if-in-upstream has the same output as the old format-patch, \nright?\n\nCiao,\nDscho\n"},{"id":"22653","messageId":"46a038f90606270252p2beac88bo3cf7aa8d3845450c@mail.gmail.com","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606271016450.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-27T09:52:21Z","receivedAt":"2006-06-27T09:52:21Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 6/27/06, Johannes Schindelin wrote:\n> Hi,\n>\n> I just cloned your repo, and as far as I can tell, the latest commit is on\n> June 23rd. So your numbers should be the same as mine. But not all are.\n\nTaht repo is on a machine at work, but it's possible that I've cherry\npicked a new commit onto master that isn't on the publc repo yet.\n\n> The problem I see: from the 53 non-merges in nzvleportfolio (who makes up\n> your branch names anyway?), there are two already in upstream: e3f56c and\n> 7e448c5c. So it really should be 51.\n>\n> > $ git-rev-list svnhead..master | wc -l\n> > 61\n>\n> Same on my repo.\n\nAnd if you add --no-merges it'll give you 51 or 52, depending...\n\n> > $ ~/local/git/git-format-patch.sh -o .patchesold svnhead master\n> > ...\n> > $ ls .patchesold | wc -l\n> > 52\n>\n> I guess this propagates from git-cherry. (Did not test here, since I do\n> not have an old git-format-patch.sh handy, and am too lazy to get the last\n> version from my git repo.)\n\nI think I did something like\n\n git-cat-file blob v1.3.3:git-format-patch.sh > git-format-patch.sh\n chmod ugo+x git-format-patch.sh\n\n> But anyway, looking at your numbers I take it that the new format-patch\n> with --ignore-if-in-upstream has the same output as the old format-patch,\n> right?\n\nIt does, but it may be \"right\" even though it's not realising that\nsome of the patches were cherry picked. git-cherry doesn't either. So\nthat algorythm isn't so hot in this case :-/\n\nI jumped to conclusions earlier because I called git format-patch with\nthe old syntax (theirs ours) and it gave me 186 patches, which may\nvery well be the whole repo history, like it did earlier with git\nitself. I thought that 186 were the patches pending, and that\ngit-cherry was cutting that to 52. Not so: it was a syntax issue.\nSorry about the noise!\n\ncheers,\n\n\n\nmartin\n"},{"id":"22662","messageId":"Pine.LNX.4.63.0606271251440.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"46a038f90606270252p2beac88bo3cf7aa8d3845450c@mail.gmail.com","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-27T10:54:39Z","receivedAt":"2006-06-27T10:54:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jun 2006, Martin Langhoff wrote:\n\n> On 6/27/06, Johannes Schindelin wrote:\n> > \n> > I just cloned your repo, and as far as I can tell, the latest commit is on\n> > June 23rd. So your numbers should be the same as mine. But not all are.\n> \n> Taht repo is on a machine at work, but it's possible that I've cherry\n> picked a new commit onto master that isn't on the publc repo yet.\n\nI hoped that much.\n\n> > (Did not test here, since I do not have an old git-format-patch.sh \n> > handy, and am too lazy to get the last version from my git repo.)\n> \n> I think I did something like\n> \n> git-cat-file blob v1.3.3:git-format-patch.sh > git-format-patch.sh\n> chmod ugo+x git-format-patch.sh\n\nYes, I am lazy.\n\n> > But anyway, looking at your numbers I take it that the new format-patch\n> > with --ignore-if-in-upstream has the same output as the old format-patch,\n> > right?\n> \n> It does, but it may be \"right\" even though it's not realising that\n> some of the patches were cherry picked. git-cherry doesn't either. So\n> that algorythm isn't so hot in this case :-/\n\nWhat do you mean? Is the patch-id of the cherry-picked different? (If \nthere was a conflict which was manually resolved, I think there is no way \nwe can detect that that patch was cherry-picked, but if it applied \ncleanly, the patch-id should be equal both in upstream and downstream.)\n\nCiao,\nDscho\n"},{"id":"22664","messageId":"46a038f90606270409t6724b105h6c56d55138fd58c5@mail.gmail.com","threadId":"4659","inReplyTo":"Pine.LNX.4.63.0606271251440.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-27T11:09:28Z","receivedAt":"2006-06-27T11:09:28Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 6/27/06, Johannes Schindelin wrote:\n> > It does, but it may be \"right\" even though it's not realising that\n> > some of the patches were cherry picked. git-cherry doesn't either. So\n> > that algorythm isn't so hot in this case :-/\n>\n> What do you mean? Is the patch-id of the cherry-picked different? (If\n> there was a conflict which was manually resolved, I think there is no way\n> we can detect that that patch was cherry-picked, but if it applied\n> cleanly, the patch-id should be equal both in upstream and downstream.)\n\nI'll look more into it tomorrow at work. Knowing the codebase, some of\nthe patches should be rather obvious, GNU patch recognises them as\n'already applied', but they may be slightly different in ways that\nbreak the hashing. Maybe. I'll play with git-cherry to see, I assume\nthat you've copied the logic from there.\n\ncheers,\n\n\nmartin\n"},{"id":"22668","messageId":"Pine.LNX.4.63.0606271434210.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"46a038f90606270409t6724b105h6c56d55138fd58c5@mail.gmail.com","subject":"Re: [PATCH 3/2] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-27T12:39:15Z","receivedAt":"2006-06-27T12:39:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jun 2006, Martin Langhoff wrote:\n\n> the patches should be rather obvious, GNU patch recognises them as\n> 'already applied', but they may be slightly different in ways that\n> break the hashing.\n\nDid you try with git-apply? This is much more anal than GNU patch, and \nshould give you an idea what is happening.\n\n> I'll play with git-cherry to see, I assume that you've copied the logic \n> from there.\n\nYup. Well, from git-patch-id, but that was used by git-cherry.\n\nNote that I made sure that the patch-id is the same as for git-patch-id. \nIt only differs with renaming patches (the \"---\" and \"+++\" lines are \nhashed, to make sure it is the same patch, but the \"rename from\" and \n\"rename to\" lines are not generated by diff_flush_patch_id()).\n\nCiao,\nDscho\n"},{"id":"22686","messageId":"Pine.LNX.4.63.0606272236350.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4659","inReplyTo":"7vr71bh6sv.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] format-patch: use clear_commit_marks() instead of some adhocery","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-27T20:38:04Z","receivedAt":"2006-06-27T20:38:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt is cleaner, and it describes better what is the idea behind the code.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tOn Mon, 26 Jun 2006, Junio C Hamano wrote:\n\n\t> I'll be pushing out a new test for format-patch shortly in\n\t> \"next\".\n\n\t... and this test fails with my original patch: We also need to \n\treset ADDED, and I threw in SHOWN for good measure.\n\n\tMaybe there is room for improvement of the revision walker here;\n\tIt smells a little like ADDED is not only used to avoid duplicate\n\tparsing (which I guess is now happening, even with\n\treset_all_objects_flags() instead of clear_commit_marks()), but\n\talso to decide if the revision walker should walk on.\n\n\tWe are getting more and more users of the revision walker, and this\n\tis just the first to call the walker more than once.\n\n builtin-log.c | 14 ++++----------\n 1 files changed, 4 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 4ee5891..f9515a8 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -160,15 +160,6 @@ static void reopen_stdout(struct commit \n \tfreopen(filename, \"w\", stdout);\n }\n \n-static void reset_all_objects_flags()\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < obj_allocs; i++)\n-\t\tif (objs[i])\n-\t\t\tobjs[i]->flags = 0;\n-}\n-\n static int get_patch_id(struct commit *commit, struct diff_options *options,\n \t\tunsigned char *sha1)\n {\n@@ -220,7 +211,10 @@ static void get_patch_ids(struct rev_inf\n \t}\n \n \t/* reset for next revision walk */\n-\treset_all_objects_flags();\n+\tclear_commit_marks((struct commit *)o1,\n+\t\t\tSEEN | UNINTERESTING | SHOWN | ADDED);\n+\tclear_commit_marks((struct commit *)o2,\n+\t\t\tSEEN | UNINTERESTING | SHOWN | ADDED);\n \to1->flags = flags1;\n \to2->flags = flags2;\n }\n-- \n1.4.1.rc1.g9de8f-dirty\n"}]}