{"thread":{"id":"9535","subject":"[PATCH] Make git-cvsexportcommit \"status\" each file in turn","startedAt":"2007-08-15T13:27:28Z","lastAt":"2007-08-15T19:40:35Z","messageCount":5,"participants":["Alex Bennee","Peter Baumann","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50772","messageId":"1187184448.13096.54.camel@murta.transitives.com","threadId":"9535","inReplyTo":null,"subject":"[PATCH] Make git-cvsexportcommit \"status\" each file in turn","fromName":"Alex Bennee","fromEmail":"kernel-hacker@bennee.com","sentAt":"2007-08-15T13:27:28Z","receivedAt":"2007-08-15T13:27:28Z","isPatch":true,"sender":{"key":"kernel-hacker@bennee.com","avatar":null},"body":"Hi,\n\nIt turns out CVS doesn't always give the status output in the order\nrequested. According to my local CVS gurus this is a known CVS issue.\n\nThe attached patch just makes the script check each file in turn. It's\nslower but correct.\n\nI also slightly formatted the warn output when it detects problems as\nmultiple line wraps with long file paths where making my eyes bleed :-)\n\n-- \nAlex, homepage: http://www.bennee.com/~alex/\nAbsence makes the heart go wander.\n"},{"id":"50776","messageId":"20070815140431.GC4550@xp.machine.xx","threadId":"9535","inReplyTo":"1187184448.13096.54.camel@murta.transitives.com","subject":"Re: [PATCH] Make git-cvsexportcommit \"status\" each file in turn","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-08-15T14:04:31Z","receivedAt":"2007-08-15T14:04:31Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Aug 15, 2007 at 02:27:28PM +0100, Alex Bennee wrote:\n> Hi,\n> \n> It turns out CVS doesn't always give the status output in the order\n> requested. According to my local CVS gurus this is a known CVS issue.\n> \n> The attached patch just makes the script check each file in turn. It's\n> slower but correct.\n> \n> I also slightly formatted the warn output when it detects problems as\n> multiple line wraps with long file paths where making my eyes bleed :-)\n> \n\nI inlined the patch for easier commenting. Please inline further\npatches.\n\n> ---\n> git-cvsexportcommit.perl | 30 ++++++++++++++++++++----------\n> 1 files changed, 20 insertions(+), 10 deletions(-)\n> \n> diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\n> index e9832d2..ee02c56 100755\n> --- a/git-cvsexportcommit.perl\n> +++ b/git-cvsexportcommit.perl\n> @@ -182,15 +182,21 @@ if (@canstatusfiles) {\n> my @updated = safe_pipe_capture(@cvs, 'update', @canstatusfiles);\n> print @updated;\n> }\n> - my @cvsoutput;\n> - @cvsoutput= safe_pipe_capture(@cvs, 'status', @canstatusfiles);\n> - my $matchcount = 0;\n> - foreach my $l (@cvsoutput) {\n> - chomp $l;\n> - if ( $l =~ /^File:/ and $l =~ /Status: (.*)$/ ) {\n> - $cvsstat{$canstatusfiles[$matchcount]} = $1;\n> - $matchcount++;\n> - }\n> +\n> + # We can't status all the files at once as CVS doesn't gaurentee\n> + # that it will output the status bits in the order requested.\n> +\n> + foreach my $f (@canstatusfiles)\n> + {\n> +\tmy $cvscmd = join(' ', @cvs).\" status $f\";\n> +\tmy $cvsoutput = `$cvscmd`;\n> +\n> +\t# slurp out the status out of the result\n> +\tmy ($status) = $cvsoutput =~ m/.*Status: (\\S*)/;\n> +\n> +\t$opt_v && print \"Status of $f is $status\\n\";\n> +\n> +\t$cvsstat{$f} = $status;\n> }\n> }\n> \n\nThis is extremly wastefull, because it will spawn a CVS process for each file.\nA better fix would be to parse the filename from the output of\n'cvs status' and use that as input for $cvsstat.\n\n(And/or you could use an hash instead of an array for 'cvsoutput', so\nyou could double check that you only get the status for those files you\nasked for.)\n\n> \n> \n> @@ -198,10 +204,14 @@ if (@canstatusfiles) {\n> foreach my $f (@afiles) {\n> if (defined ($cvsstat{$f}) and $cvsstat{$f} ne \"Unknown\") {\n> \t$dirty = 1;\n> -\twarn \"File $f is already known in your CVS checkout -- perhaps it has been added by another user. Or this may indicate that it exists on a different branch. If this is the case, use -f to force the merge.\\n\";\n> +\twarn \"File $f is already known in your CVS checkout.\\n\"\n> +\twarn \" Perhaps it has been added by another user.\\n\"\n> +\twarn \" Or this may indicate that it exists on a different branch.\\n\"\n> +\twarn \" If this is the case, use -f to force the merge.\\n\";\n> \twarn \"Status was: $cvsstat{$f}\\n\";\n> }\n> }\n> +\n> # ... validate known files.\n> foreach my $f (@files) {\n> next if grep { $_ eq $f } @afiles;\n> -- \n> 1.5.2.3\n> \n\n-Peter\n"},{"id":"50784","messageId":"1187192876.13096.60.camel@murta.transitives.com","threadId":"9535","inReplyTo":"1187184448.13096.54.camel@murta.transitives.com","subject":"Re: [PATCH] Make git-cvsexportcommit \"status\" each file in turn","fromName":"Alex Bennee","fromEmail":"kernel-hacker@bennee.com","sentAt":"2007-08-15T15:47:56Z","receivedAt":"2007-08-15T15:47:56Z","isPatch":true,"sender":{"key":"kernel-hacker@bennee.com","avatar":null},"body":"On Wed, 2007-08-15 at 14:27 +0100, Alex Bennee wrote:\n> Hi,\n> \n\n\n> I also slightly formatted the warn output when it detects problems as\n> multiple line wraps with long file paths where making my eyes bleed :-)\n\nWhich I seem to have missed a bunch of crucial ;'s from\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex ee02c56..65c12b2 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -204,9 +204,9 @@ if (@canstatusfiles) {\n foreach my $f (@afiles) {\n if (defined ($cvsstat{$f}) and $cvsstat{$f} ne \"Unknown\") {\n $dirty = 1;\n- warn \"File $f is already known in your CVS checkout.\\n\"\n- warn \" Perhaps it has been added by another user.\\n\"\n- warn \" Or this may indicate that it exists on a different\nbranch.\\n\"\n+ warn \"File $f is already known in your CVS checkout.\\n\";\n+ warn \" Perhaps it has been added by another user.\\n\";\n+ warn \" Or this may indicate that it exists on a different\nbranch.\\n\";\n warn \" If this is the case, use -f to force the merge.\\n\";\n warn \"Status was: $cvsstat{$f}\\n\";\n }\n\n-- \nAlex, homepage: http://www.bennee.com/~alex/\nLove may laugh at locksmiths, but he has a profound respect for money\nbags. -- Sidney Paternoster, \"The Folly of the Wise\"\n"},{"id":"50790","messageId":"1187195112.13096.71.camel@murta.transitives.com","threadId":"9535","inReplyTo":"20070815140431.GC4550@xp.machine.xx","subject":"Re: [PATCH] Make git-cvsexportcommit \"status\" each file in turn","fromName":"Alex Bennee","fromEmail":"kernel-hacker@bennee.com","sentAt":"2007-08-15T16:25:12Z","receivedAt":"2007-08-15T16:25:12Z","isPatch":true,"sender":{"key":"kernel-hacker@bennee.com","avatar":null},"body":"On Wed, 2007-08-15 at 16:04 +0200, Peter Baumann wrote:\n> On Wed, Aug 15, 2007 at 02:27:28PM +0100, Alex Bennee wrote:\n> > Hi,\n> > \n> > It turns out CVS doesn't always give the status output in the order\n> > requested. According to my local CVS gurus this is a known CVS issue.\n> \n> I inlined the patch for easier commenting. Please inline further\n> patches.\n\nWill do. I assumed Evolution would do something sensible. My mistake :-(\n\n> \n> > ---\n> > git-cvsexportcommit.perl | 30 ++++++++++++++++++++----------\n> > 1 files changed, 20 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\n> \n> This is extremly wastefull, because it will spawn a CVS process for each file.\n> A better fix would be to parse the filename from the output of\n> 'cvs status' and use that as input for $cvsstat.\n> \n> (And/or you could use an hash instead of an array for 'cvsoutput', so\n> you could double check that you only get the status for those files you\n> asked for.)\n\nI agree it's wasteful and could be done better however I'm no perl\nhacker so I just went for something that was correct and worked. \n\nThe path that is echoed later in the status output is however the CVS\nfile path which may not be directly related to the actual path in your\nsource tree. For example I have one status reported as:\n\n$ cvs status src/proj_version\n===================================================================\nFile: proj_version Status: Up-to-date\n\n Working revision: 1.1.380.1\n Repository revision: 1.1.380.1 /export/cvsroot/project/src/Attic/proj_version,v\n Sticky Tag: ATAG (branch: 1.1.380)\n Sticky Date: (none)\n Sticky Options: (none)\n\nThis makes the matching more than a little problematic.\n\nIt depends on how much people that use this script care about performance?\n\nFor my part it's a fire and forget script once I've finished my hacking\nin a git tree so I don't mind it taking some time. I'm not particularly\nminded to dig further in perl to make it faster unless there is a real\nclamour - or perhaps someone with a bigger itch and more perl foo can\ntackle it. \n\nIn the meantime it does fix a bug in the script so I would say it's\napplying.\n\n-- \nAlex, homepage: http://www.bennee.com/~alex/\nBlessed is he who has reached the point of no return and knows it, for\nhe shall enjoy living. -- W. C. Bennett\n"},{"id":"50812","messageId":"200708152140.36432.robin.rosenberg.lists@dewire.com","threadId":"9535","inReplyTo":"1187195112.13096.71.camel@murta.transitives.com","subject":"Re: [PATCH] Make git-cvsexportcommit \"status\" each file in turn","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-08-15T19:40:35Z","receivedAt":"2007-08-15T19:40:35Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 15 augusti 2007 skrev Alex Bennee:\n> On Wed, 2007-08-15 at 16:04 +0200, Peter Baumann wrote:\n> > On Wed, Aug 15, 2007 at 02:27:28PM +0100, Alex Bennee wrote:\n> > > Hi,\n> > > \n> > > It turns out CVS doesn't always give the status output in the order\n> > > requested. According to my local CVS gurus this is a known CVS issue.\n> > \n> > I inlined the patch for easier commenting. Please inline further\n> > patches.\n> \n> Will do. I assumed Evolution would do something sensible. My mistake :-(\n> \n> > \n> > > ---\n> > > git-cvsexportcommit.perl | 30 ++++++++++++++++++++----------\n> > > 1 files changed, 20 insertions(+), 10 deletions(-)\n> > > \n> > > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\n> > \n> > This is extremly wastefull, because it will spawn a CVS process for each \nfile.\n> > A better fix would be to parse the filename from the output of\n> > 'cvs status' and use that as input for $cvsstat.\n> > \n> > (And/or you could use an hash instead of an array for 'cvsoutput', so\n> > you could double check that you only get the status for those files you\n> > asked for.)\n> \n> I agree it's wasteful and could be done better however I'm no perl\n> hacker so I just went for something that was correct and worked. \n> \n> The path that is echoed later in the status output is however the CVS\n> file path which may not be directly related to the actual path in your\n> source tree. For example I have one status reported as:\n> \n> $ cvs status src/proj_version\n> ===================================================================\n> File: proj_version Status: Up-to-date\n> \n> Working revision: 1.1.380.1\n> Repository revision: \n1.1.380.1 /export/cvsroot/project/src/Attic/proj_version,v\n> Sticky Tag: ATAG (branch: 1.1.380)\n> Sticky Date: (none)\n> Sticky Options: (none)\n> \n> This makes the matching more than a little problematic.\nCVS/Root contains the first part of the path. Just use the one found at the \ntop of the CVS checkout. Using CVS may be insane, but mixing different repos \nin the same checkout is absolute madness.\n\nThen drop /Attic/ if present and the ,v at the end of the file name. Not \nrocket science I'd say.\n\n> It depends on how much people that use this script care about performance?\nWe do although I rarely commit big patches so cvsexportcommit usually does \nit's job in seconds for me. I also use the -u option and don't do any work in \nthe CVS checkout so performing status if just overhead for me. We might drop \nit completely with a switch.\n\n> For my part it's a fire and forget script once I've finished my hacking\n> in a git tree so I don't mind it taking some time. I'm not particularly\n> minded to dig further in perl to make it faster unless there is a real\n> clamour - or perhaps someone with a bigger itch and more perl foo can\n> tackle it. \n\n> \n> In the meantime it does fix a bug in the script so I would say it's\n> applying.\n> \n\nI suggest Junio wait a few days in case a real fix materializes.\n\n-- robin\n"}]}