{"thread":{"id":"3525","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","startedAt":"2006-03-02T16:44:05Z","lastAt":"2006-03-03T01:52:21Z","messageCount":10,"participants":["Christopher Faylor","Shawn Pearce","Johannes Schindelin","Alex Riesen","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"17056","messageId":"20060302164405.GB7292@trixie.casa.cgf.cx","threadId":"3525","inReplyTo":null,"subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Christopher Faylor","fromEmail":"me@cgf.cx","sentAt":"2006-03-02T16:44:05Z","receivedAt":"2006-03-02T16:44:05Z","isPatch":true,"sender":{"key":"me@cgf.cx","avatar":null},"body":"So to summarize:\n\nIf anyone has a problem with Cygwin where signals do not seem to be\nworking, I'd appreciate a bug report to the Cygwin list.  We really do\nexpect that things should work and want to fix things if they don't.\n\nIf that isn't possible to use the Cygwin list for some reason, I will\ncontinue to read this mailing list and respond to Cygwin problems but I\nwould appreciate it if any Cygwin problem report contained details for\nreproducing the problem.  We usually point people to this page\nhttp://cygwin.com/problems.html when they have problems.  The basic take\naway from that page is to provide the cygcheck output which shows what\nsettings have been used for your Cygwin installation.  The interesting\nstuff in that output is the cygwin mount points, the CYGWIN environment\nvariable, and version information about the Cygwin DLL.\n\nThe Cygwin web site is http://cygwin.com/ and it has a lot of information\nabout Cygwin.  Some of it is undoubtedly out-of-date or unclear but we\ndo try to improve things if they are brought to our attention.\n\nI don't see any reason to respond to this thread any further but I will\ncontinue to rectify any misstatements that I see being made about\nWindows or Cygwin here.\n\ncgf (Cygwin Maintainer)\n"},{"id":"17058","messageId":"20060302165510.GB18929@spearce.org","threadId":"3525","inReplyTo":"20060302164405.GB7292@trixie.casa.cgf.cx","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-03-02T16:55:10Z","receivedAt":"2006-03-02T16:55:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Maybe I missed this but why are people using the native Windows\nActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl\ninstallation available?\n\nI've been using the Cygwin Perl with GIT without any problems\nwhatsoever.  Including the open(I, \"-|\")... exec(@argv) code that\ndoesn't work correctly in ActiveState and started this whole thread.\n\n-- \nShawn.\n"},{"id":"17063","messageId":"Pine.LNX.4.63.0603021832460.31727@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"3525","inReplyTo":"20060302164405.GB7292@trixie.casa.cgf.cx","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-03-02T17:33:31Z","receivedAt":"2006-03-02T17:33:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 2 Mar 2006, Christopher Faylor wrote:\n\n> If anyone has a problem with Cygwin where signals do not seem to be\n> working, I'd appreciate a bug report to the Cygwin list.  We really do\n> expect that things should work and want to fix things if they don't.\n\nI am glad to have you on this list. Thanks for all your efforts.\n\nCiao,\nDscho\n"},{"id":"17076","messageId":"20060302220930.GE6183@steel.home","threadId":"3525","inReplyTo":"20060302165510.GB18929@spearce.org","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-03-02T22:09:30Z","receivedAt":"2006-03-02T22:09:30Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:\n> Maybe I missed this but why are people using the native Windows\n> ActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl\n> installation available?\n\nbecause the people _can't_ use cygwin's perl. There are a lot of\nreasons mainly: administrative, perl script incompatibilities and\ncygwin.dll incompatibilities (if you use perl from cygwin, it'll need\nthe correct cygwin.dll. And if a build process uses cygwin tools from,\nfor example, QNX Momentics it often comes to clashes).\n\n> I've been using the Cygwin Perl with GIT without any problems\n> whatsoever.  Including the open(I, \"-|\")... exec(@argv) code that\n> doesn't work correctly in ActiveState and started this whole thread.\n\nUnfortunately...\n"},{"id":"17082","messageId":"Pine.LNX.4.64.0603021521250.22647@g5.osdl.org","threadId":"3525","inReplyTo":"20060302220930.GE6183@steel.home","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-03-02T23:27:24Z","receivedAt":"2006-03-02T23:27:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 2 Mar 2006, Alex Riesen wrote:\n\n> Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:\n> \n> > I've been using the Cygwin Perl with GIT without any problems\n> > whatsoever.  Including the open(I, \"-|\")... exec(@argv) code that\n> > doesn't work correctly in ActiveState and started this whole thread.\n> \n> Unfortunately...\n\nHere's a stupid first cut at git-fmt-merge-msg in C using the new revlist \nlibrary interface.\n\nIt's not actually doing exactly the same thing, because I'm a lazy \nbastard, but some things it does better.\n\nFor example, afaik, when merging multiple branches that had partially been \nmerged already (ie they had overlapping new stuff), if I read the old perl \ncode correctly, it would talk about the new stuff multiple times. This one \ndoesn't.\n\nThe things it doesn't do:\n - the old one had a limit of 20, the new one has a limit of 10 commits \n   reported\n - the old one was tested, the new one is written by me.\n - the old one honored the \"merge.summary\" git config option. The new one \n   doesn't.\n - the old one did some formatting of the branch message that I don't \n   follow because I'm not a perl user. The new one just takes the \n   explanatory message for the branch merging as-is.\n\nBut hey, this is all part of my cunning plan to make people get involved \nwith the new rev-list libification, by giving them things that _almost_ \nwork, but might need some tweaking.\n\n\t\tLinus\n\n--- snip snip for \"fmt-merge-msg.c\" snip snip---\n/*\n * fmt-merge-msg.c\n *\n * Magic auto-generation of merge messages.\n *\n * Copyright (C) 2006 Linus Torvalds and his army of programming ferrets\n */\n#include \"cache.h\"\n#include \"commit.h\"\n#include \"revision.h\"\n\nstatic void show_commit(struct commit *commit)\n{\n\tchar buffer[256];\n\n\tpretty_print_commit(CMIT_FMT_ONELINE, commit, ~0, buffer, sizeof(buffer), 0);\n\tprintf(\"   * %s\\n\", buffer);\n}\n\nint main(int argc, char **argv)\n{\n\tstruct rev_info revs;\n\tstruct commit *commit;\n\tunsigned char sha1[20];\n\tchar buffer[256];\n\n\tsetup_revisions(0, NULL, &revs, NULL);\n\tif (get_sha1(\"HEAD\", sha1) < 0)\n\t\tdie(\"no HEAD revision\");\n\tcommit = lookup_commit_reference(sha1);\n\tif (!commit)\n\t\tdie(\"no HEAD revision\");\n\n\tcommit->object.flags |= UNINTERESTING;\n\tinsert_by_date(commit, &revs.commits);\n\trevs.topo_order = 1;\n\trevs.limited = 1;\n\n\twhile (fgets(buffer, sizeof(buffer), stdin)) {\n\t\tint max;\n\t\tchar *marker;\n\n\t\tif (get_sha1_hex(buffer, sha1) < 0)\n\t\t\tcontinue;\n\t\tcommit = lookup_commit_reference(sha1);\n\t\tif (!commit)\n\t\t\tcontinue;\n\n\t\t/*\n\t\t * Format after the SHA1:\n\t\t *\t<tab>marker<tab><type>'<name>' of <src>'\n\t\t *\n\t\t * where string is \"not-for-merge\" if\n\t\t * we're not interested in this one,\n\t\t * and empty otherwise.\n\t\t */\n\t\tmarker = buffer + 40;\n\t\tif (*marker++ != '\\t')\n\t\t\tcontinue;\n\t\tif (*marker++ != '\\t')\n\t\t\tcontinue;\n\t\tprintf(\"Merge %s\", marker);\n\n\t\tinsert_by_date(commit, &revs.commits);\n\t\tprepare_revision_walk(&revs);\n\n\t\tmax = 10;\n\t\twhile ((commit = get_revision(&revs)) != NULL) {\n\t\t\tint n = --max;\n\t\t\tif (n > 0)\n\t\t\t\tshow_commit(commit);\n\t\t\telse if (!n)\n\t\t\t\tprintf(\"   ...\");\n\t\t}\n\t}\n}\n"},{"id":"17087","messageId":"20060303001434.GA7497@trixie.casa.cgf.cx","threadId":"3525","inReplyTo":"20060302220930.GE6183@steel.home","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Christopher Faylor","fromEmail":"me@cgf.cx","sentAt":"2006-03-03T00:14:34Z","receivedAt":"2006-03-03T00:14:34Z","isPatch":true,"sender":{"key":"me@cgf.cx","avatar":null},"body":"On Thu, Mar 02, 2006 at 11:09:30PM +0100, Alex Riesen wrote:\n>Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:\n>>Maybe I missed this but why are people using the native Windows\n>>ActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl\n>>installation available?\n>\n>because the people _can't_ use cygwin's perl.  There are a lot of\n>reasons mainly: administrative, perl script incompatibilities and\n>cygwin.dll incompatibilities (if you use perl from cygwin, it'll need\n>the correct cygwin.dll.  And if a build process uses cygwin tools from,\n>for example, QNX Momentics it often comes to clashes).\n\n(Hmm.  I wonder if QNX Momentics is YA GPL violator)\n\nIf you have multiple versions of the Cygwin DLL on your system and try\nto use them all jumbled up together then, yes, you will have problems.\nThis isn't a perl-specific issue.  The solution is to put the latest\nversion of your Cygwin DLL in your path (presumably in /bin) and delete\nall of the older ones.\n\nThe newest version is undoubtedly going to be the one downloaded from\nthe Cygwin web site (http://cygwin.com/) but you can get version\ninformation from the cygwin DLL by using grep:\n\n  grep -a \"^%%% Cygwin\" WHEREEVER/cygwin1.dll\n\nif you are not inclined to install the newest version of Cygwin.\n\nI'm sure that there are incompatibilities between ActiveState perl and\nCygwin's perl which make it hard to use the same scripts in each so I am\nnot doubting that some people might want to use only ActiveState perl.\nI don't see how the multiple Cygwin DLL issue can be a problem only for\nCygwin perl vs. ActiveState perl.\n\ncgf\n(who sees a new full-time job looming in the git list)\n"},{"id":"17089","messageId":"7v1wxk5ptf.fsf@assigned-by-dhcp.cox.net","threadId":"3525","inReplyTo":"Pine.LNX.4.64.0603021521250.22647@g5.osdl.org","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-03T00:34:20Z","receivedAt":"2006-03-03T00:34:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> For example, afaik, when merging multiple branches that had partially been \n> merged already (ie they had overlapping new stuff), if I read the old perl \n> code correctly, it would talk about the new stuff multiple times. This one \n> doesn't.\n\nI think this is not quite right, even though it only matters in\nOctopus and not many people do Octopus anyway.  Suppose you are\nmerging lt/rev-list and fk/blame branches into master, starting\nfrom this state:\n\n    ! [master] GIT-VERSION-GEN: squelch unneed\n     ! [lt/rev-list] setup_revisions(): handle\n      ! [fk/blame] git-blame, take 2\n    ---\n     +  [lt/rev-list] setup_revisions(): handl\n     +  [lt/rev-list^] git-log (internal): mor\n     +  [lt/rev-list~2] git-log (internal): ad\n      + [fk/blame] git-blame, take 2\n      - [fk/blame^] Merge part of 'lt/rev-list\n     ++ [lt/rev-list~3] Rip out merge-order an\n     ++ [lt/rev-list~4] Tie it all together: \"\n     ++ [lt/rev-list~5] Introduce trivial new \n     ++ [lt/rev-list~6] git-rev-list libificat\n     ++ [lt/rev-list~7] Splitting rev-list int\n     ++ [lt/rev-list~8] rev-list split: minimu\n     ++ [lt/rev-list~9] First cut at libifying\n      + [fk/blame~2] Add git-blame, a tool for\n    --- [lt/rev-list~10] Merge branch 'maint' \n\nAnd you had lt/rev-list branch first listed in FETCH_HEAD.  In\nthis particular example, lt/rev-list has only 3 commits on top\nof common things, but if your max were 3 instead of 10, the\nfirst round would actually show the tip 3 without showing any\ncommon stuff, and then the next round to show fk/blame branch\nwould show only the remaining two, without ever showing the\ncommon stuff, even though it _could_ say the latest of the\ncommon stuff.\n\n> The things it doesn't do:\n>  - the old one had a limit of 20, the new one has a limit of 10 commits \n>    reported\n\nGood change I would say, except for the above.\n\n>  - the old one was tested, the new one is written by me.\n>  - the old one honored the \"merge.summary\" git config option. The new one \n>    doesn't.\n\nEasily rectifiable ;-).\n\n>  - the old one did some formatting of the branch message that I don't \n>    follow because I'm not a perl user. The new one just takes the \n>    explanatory message for the branch merging as-is.\n\nFETCH_HEAD has explanatory message in more or less \"canonical\"\nform.  It has noise word \"branch\", and the current repository is\ntypically \" of .\".  These are removed by the code, so that you would\nnot have to see:\n\n\tMerge branch 'jc/delta' of .\n\nInstead you would see:\n\n\tMerge 'jc/delta' into 'next'.\n\nThe last part, \" into 'next'\", is also missing from your\nversion.  I can distinguish a merge into 'master' (which does\nnot have \" into 'master'\") and other branches easily that way,\nand I find it handy.\n\nOther things the Perl code does are purely for Octopus support:\nthings like coalescing multiple branches taken from the same\nrepositories.  You would get something like:\n\n\tMerge 'lt/rev-list' and 'fk/blame' into 'next'.\n\n\t* lt/rev-list:\n\t  commit 1\n          commit 2\n\n\t* fk/blame:\n\t  commit 3\n\t  commit 4\n\ninstead of (your version):\n\n\tMerge branch 'lt/rev-list' of .\n\t   * commit 1\n           * commit 2\n\n\tMerge branch 'fk/blame' of .\n\t   * commit 3\n           * commit 4\n"},{"id":"17090","messageId":"Pine.LNX.4.64.0603021643560.22647@g5.osdl.org","threadId":"3525","inReplyTo":"7v1wxk5ptf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-03-03T00:49:26Z","receivedAt":"2006-03-03T00:49:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 2 Mar 2006, Junio C Hamano wrote:\n> \n> And you had lt/rev-list branch first listed in FETCH_HEAD.  In\n> this particular example, lt/rev-list has only 3 commits on top\n> of common things, but if your max were 3 instead of 10, the\n> first round would actually show the tip 3 without showing any\n> common stuff, and then the next round to show fk/blame branch\n> would show only the remaining two, without ever showing the\n> common stuff, even though it _could_ say the latest of the\n> common stuff.\n\nYes. I considered it briefly, and it's fixable, but to fix it you'd \nhave to actualyl walk the parent list yourself, rather than letting \nget_revision do it all for you.\n\nAnd what my simple thing shows isn't really technically \"wrong\", since it \nhas shown that there are commits missing from the output with the \"...\"\n\nThe question is just whether shared commits should be \"balanced out\", or \nshown as part of the first branch that merged them. I chose the latter, \nbecause it's not only simple, it's unambiguous (any balancing algorithm \nwill depend on some random heuristic or other, and on how many commits are \nshown.\n\n> >  - the old one did some formatting of the branch message that I don't \n> >    follow because I'm not a perl user. The new one just takes the \n> >    explanatory message for the branch merging as-is.\n> \n> FETCH_HEAD has explanatory message in more or less \"canonical\"\n> form.  It has noise word \"branch\", and the current repository is\n> typically \" of .\".\n\nYeah, I actually looked at a few examples, so I knew what it was basically \ntrying to do, and then I ignored it as not interesting to the exercise, \nwhich was to abuse the new revision listing library in interesting ways by \ncalling it multiple times.\n\n\t\tLinus\n"},{"id":"17096","messageId":"7vveuw48uw.fsf@assigned-by-dhcp.cox.net","threadId":"3525","inReplyTo":"Pine.LNX.4.64.0603021643560.22647@g5.osdl.org","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-03T01:25:59Z","receivedAt":"2006-03-03T01:25:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Yeah, I actually looked at a few examples, so I knew what it was basically \n> trying to do, and then I ignored it as not interesting to the exercise, \n> which was to abuse the new revision listing library in interesting ways by \n> calling it multiple times.\n\nAbuse is exactly the word.  The reason it is an abuse is exactly\nwhy you said \"... but to fix it you'd have to actualyl walk the\nparent list yourself, rather than letting get_revision do it all\nfor you.\"  Which relates to the fact that object.c layer is not\ndesigned to be used multiple times...\n\nMaybe we want to make object.c layer reusable first?\n"},{"id":"17097","messageId":"Pine.LNX.4.64.0603021747430.22647@g5.osdl.org","threadId":"3525","inReplyTo":"7vveuw48uw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] fmt-merge-msg: avoid open \"-|\" list form for Perl 5.6","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-03-03T01:52:21Z","receivedAt":"2006-03-03T01:52:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 2 Mar 2006, Junio C Hamano wrote:\n>\n> Linus Torvalds <torvalds@osdl.org> writes:\n> \n> > Yeah, I actually looked at a few examples, so I knew what it was basically \n> > trying to do, and then I ignored it as not interesting to the exercise, \n> > which was to abuse the new revision listing library in interesting ways by \n> > calling it multiple times.\n> \n> Abuse is exactly the word.  The reason it is an abuse is exactly\n> why you said \"... but to fix it you'd have to actualyl walk the\n> parent list yourself, rather than letting get_revision do it all\n> for you.\"  Which relates to the fact that object.c layer is not\n> designed to be used multiple times...\n> \n> Maybe we want to make object.c layer reusable first?\n\nNo, the \"abuse\" is actually very much done that way on purpose. It's a bit \nstrange to do \"incremental\" prepare_revision_walk() calls, but it all \ncomes from the fact that the object structures are \"persistent\" across the \ncalls, even if we remove them from the list when we walk them. \n\nSo it's strange, but that was kind of part of the reason for doing it. \nIt's a _good_ strangeness.\n\nThe thing about handling commits that were already in another branch but \nweren't shown is different: the way to handle that is to generate the \n_whole_ revision list in one go - instead of incrementally - and then for \neach branch you merge you show the top 10 \"not yet shown\" commits.\n\nIOW, that thing would never use \"get_revision()\" at all, but would instead \ndepend on \"prepare_revision_walk()\" generating the whole tree, and then \nyou just walk the parent pointers from the branch heads by hand, marking \nthen \"seen\" as you print them.\n\nSo the object layer and the revision parsing actually does exactly the \nright thing, you just have to decide on how to use them..\n\n\t\t\tLinus\n"}]}