{"thread":{"id":"8617","subject":"[PATCH] cvsserver: fix legacy cvs client and branch rev issues","startedAt":"2007-06-16T18:50:06Z","lastAt":"2007-06-17T21:27:17Z","messageCount":8,"participants":["Dirk Koopman","Frank Lichtenheld","Martin Langhoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45177","messageId":"11820198064114-git-send-email-djk@tobit.co.uk","threadId":"8617","inReplyTo":null,"subject":"[PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Dirk Koopman","fromEmail":"djk@tobit.co.uk","sentAt":"2007-06-16T18:50:06Z","receivedAt":"2007-06-16T18:50:06Z","isPatch":true,"sender":{"key":"djk@tobit.co.uk","avatar":null},"body":"Early cvs clients don't cause state->{args} to be initialised,\nso force this to occur.\nSome revision checking code assumes that revisions will be\nrecognisably numeric to perl, Branches are not, because they\nhave more decimal points (eg 1.2.3.4 instead of just 1.2).\n---\n git-cvsserver.perl |   17 +++++++++++------\n 1 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 5cbf27e..0a4b75e 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1813,11 +1813,14 @@ sub req_annotate\n # the second is $state->{files} which is everything after it.\n sub argsplit\n {\n+    $state->{args} = [];        # need this here because later code depends on it\n+                                # and for some reason earlier versions of CVS don't\n+                                # satisfy the next condition on plain 'cvs update'\n+\n     return unless( defined($state->{arguments}) and ref $state->{arguments} eq \"ARRAY\" );\n \n     my $type = shift;\n \n-    $state->{args} = [];\n     $state->{files} = [];\n     $state->{opt} = {};\n \n@@ -1906,11 +1909,13 @@ sub argsfromdir\n \n     # push added files\n     foreach my $file (keys %{$state->{entries}}) {\n-\tif ( exists $state->{entries}{$file}{revision} &&\n-\t\t$state->{entries}{$file}{revision} == 0 )\n-\t{\n-\t    push @gethead, { name => $file, filehash => 'added' };\n-\t}\n+        # remember that revisions could be on branches 1.2.3.4[.5.6..]\n+        # not just a recogisable \"numeric\" 1.2\n+        if ( exists $state->{entries}{$file}{revision} &&\n+             !$state->{entries}{$file}{revision} )\n+        {\n+            push @gethead, { name => $file, filehash => 'added' };\n+        }\n     }\n \n     if ( scalar(@{$state->{args}}) == 1 )\n-- \n1.5.2.1\n"},{"id":"45212","messageId":"20070617081959.GD1828@planck.djpig.de","threadId":"8617","inReplyTo":"11820198064114-git-send-email-djk@tobit.co.uk","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-17T08:19:59Z","receivedAt":"2007-06-17T08:19:59Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"Hi.\n\nOn Sat, Jun 16, 2007 at 07:50:06PM +0100, Dirk Koopman wrote:\n> Early cvs clients don't cause state->{args} to be initialised,\n> so force this to occur.\n> Some revision checking code assumes that revisions will be\n> recognisably numeric to perl, Branches are not, because they\n> have more decimal points (eg 1.2.3.4 instead of just 1.2). \n> ---\n>  git-cvsserver.perl |   17 +++++++++++------\n>  1 files changed, 11 insertions(+), 6 deletions(-)\n> \n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index 5cbf27e..0a4b75e 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -1813,11 +1813,14 @@ sub req_annotate\n>  # the second is $state->{files} which is everything after it.\n>  sub argsplit\n>  {\n> +    $state->{args} = [];        # need this here because later code depends on it\n> +                                # and for some reason earlier versions of CVS don't\n> +                                # satisfy the next condition on plain 'cvs update'\n> +\n>      return unless( defined($state->{arguments}) and ref $state->{arguments} eq \"ARRAY\" );\n>  \n>      my $type = shift;\n>  \n> -    $state->{args} = [];\n>      $state->{files} = [];\n>      $state->{opt} = {};\n>  \n\nI just would move all the initializations up there. And I think the\ncomment is really unnecessary. Will prepare a replacement patch.\n\n> @@ -1906,11 +1909,13 @@ sub argsfromdir\n>  \n>      # push added files\n>      foreach my $file (keys %{$state->{entries}}) {\n> -\tif ( exists $state->{entries}{$file}{revision} &&\n> -\t\t$state->{entries}{$file}{revision} == 0 )\n> -\t{\n> -\t    push @gethead, { name => $file, filehash => 'added' };\n> -\t}\n> +        # remember that revisions could be on branches 1.2.3.4[.5.6..]\n> +        # not just a recogisable \"numeric\" 1.2\n> +        if ( exists $state->{entries}{$file}{revision} &&\n> +             !$state->{entries}{$file}{revision} )\n> +        {\n> +            push @gethead, { name => $file, filehash => 'added' };\n> +        }\n>      }\n>  \n>      if ( scalar(@{$state->{args}}) == 1 )\n\nHmm, I don't see how you could have a problem with that since cvsserver\ndoesn't support branches and never generates any revision numbers in\nthat format?\n\nThere is probably much more code out there in cvsserver that does assume\nthat revision is always a simple integer.\n\nAnd again that comment is a but much IMHO.\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nwww: http://www.djpig.de/\n"},{"id":"45221","messageId":"11820690621003-git-send-email-frank@lichtenheld.de","threadId":"8617","inReplyTo":"11820198064114-git-send-email-djk@tobit.co.uk","subject":"[PATCH] cvsserver: always initialize state in argsplit()","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-17T08:31:02Z","receivedAt":"2007-06-17T08:31:02Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"Other code assumes that this is initialized, so do it\neven if there were no arguments given.\n\nSigned-off-by: Dirk Koopman <djk@tobit.co.uk>\nSigned-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n---\n git-cvsserver.perl |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\n Hrm, sorry to Dirk for the double mail. This time actually\n send to the list and not to git@localhost ...\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 5cbf27e..10aba50 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1813,14 +1813,14 @@ sub req_annotate\n # the second is $state->{files} which is everything after it.\n sub argsplit\n {\n-    return unless( defined($state->{arguments}) and ref $state->{arguments} eq \"ARRAY\" );\n-\n-    my $type = shift;\n-\n     $state->{args} = [];\n     $state->{files} = [];\n     $state->{opt} = {};\n \n+    return unless( defined($state->{arguments}) and ref $state->{arguments} eq \"ARRAY\" );\n+\n+    my $type = shift;\n+\n     if ( defined($type) )\n     {\n         my $opt = {};\n-- \n1.5.2.1\n"},{"id":"45215","messageId":"4674FA9B.10806@tobit.co.uk","threadId":"8617","inReplyTo":"20070617081959.GD1828@planck.djpig.de","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Dirk Koopman","fromEmail":"djk@tobit.co.uk","sentAt":"2007-06-17T09:10:51Z","receivedAt":"2007-06-17T09:10:51Z","isPatch":true,"sender":{"key":"djk@tobit.co.uk","avatar":null},"body":"Frank Lichtenheld wrote:\n> Hi.\n> \n> On Sat, Jun 16, 2007 at 07:50:06PM +0100, Dirk Koopman wrote:\n>> Early cvs clients don't cause state->{args} to be initialised,\n>> so force this to occur.\n>> Some revision checking code assumes that revisions will be\n>> recognisably numeric to perl, Branches are not, because they\n>> have more decimal points (eg 1.2.3.4 instead of just 1.2). \n\n<snip>\n\n> \n> Hmm, I don't see how you could have a problem with that since cvsserver\n> doesn't support branches and never generates any revision numbers in\n> that format?\n> \n> There is probably much more code out there in cvsserver that does assume\n> that revision is always a simple integer.\n> \n> And again that comment is a but much IMHO.\n> \n\nThe specific issue that I was trying to solve is that I have (in CVS \nterms) a main line (git head: master) and an active CVS development \nbranch and git head (called SR [for the sake of argument]).\n\nI have imported both into git using cvsimport. For compatibility (and \nwindows users) I need a anonymous, read only, :pserver: CVS \nimplementation that can serve either head.\n\nThe version numbers in the CVS import on branch SR are standard CVS \nsingle level branch 1.2.3.4. Doing a 'cvs update' on this branch was \ncausing all sorts of warnings about 1.2.3.4 not being numeric on that \ntest. After changing the test, the warnings have gone away and it all \nstill seems to work.\n\nHaving said that, I haven't worked out where cvsserver is getting those \nversion numbers from in the first place, but it obviously knows that it \nis dealing with a branch sufficient to work well enough for my needs.\n\nOf course, quite what happens when the branch merges back and people \nwant to 'cvs update -A', I shall leave for the future...\n\nGroetjes  Dirk\n"},{"id":"45219","messageId":"20070617103744.GE1828@planck.djpig.de","threadId":"8617","inReplyTo":"4674FA9B.10806@tobit.co.uk","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-17T10:37:44Z","receivedAt":"2007-06-17T10:37:44Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Sun, Jun 17, 2007 at 10:10:51AM +0100, Dirk Koopman wrote:\n> Frank Lichtenheld wrote:\n> >On Sat, Jun 16, 2007 at 07:50:06PM +0100, Dirk Koopman wrote:\n> >Hmm, I don't see how you could have a problem with that since cvsserver\n> >doesn't support branches and never generates any revision numbers in\n> >that format?\n> >\n> >There is probably much more code out there in cvsserver that does assume\n> >that revision is always a simple integer.\n\nLet me rephrase that (after actually looking through the code):\nAll of the revision handling code assumes that.\n\n> The specific issue that I was trying to solve is that I have (in CVS \n> terms) a main line (git head: master) and an active CVS development \n> branch and git head (called SR [for the sake of argument]).\n> \n> I have imported both into git using cvsimport. For compatibility (and \n> windows users) I need a anonymous, read only, :pserver: CVS \n> implementation that can serve either head.\n> \n> The version numbers in the CVS import on branch SR are standard CVS \n> single level branch 1.2.3.4. Doing a 'cvs update' on this branch was \n> causing all sorts of warnings about 1.2.3.4 not being numeric on that \n> test. After changing the test, the warnings have gone away and it all \n> still seems to work.\n>\n> Having said that, I haven't worked out where cvsserver is getting those \n> version numbers from in the first place, but it obviously knows that it \n> is dealing with a branch sufficient to work well enough for my needs.\n\nHmm, so you did the cvs update in an old working copy of the original\nCVS repository? Then CVS sent those version numbers from the CVS/Entries\nfile to the server, cvsserver certainly never generates numbers like\nthat. And I would be very suprised if you could do anything remotely\nuseful with abusing the old working copy this way... The revision\nnumbers that cvsserver assigns to the files of the main branch might\nbe almost always identical to the ones they had in CVS before the\nimport, but the ones for branches will definetly not be.\n\n> Of course, quite what happens when the branch merges back and people \n> want to 'cvs update -A', I shall leave for the future...\n\nI don't think that cvsserver actually cares about what the client sends\nas sticky tags/dates/..., so it might not actually change anything\nwhether you use -A or not (pure speculation on my part here).\n\nSummary: You're (ab)using cvsserver in very interesting ways that are not\nreally beeing thought of in the current design/implementation. There'll\nbe dragons ;)\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nwww: http://www.djpig.de/\n"},{"id":"45231","messageId":"46756707.5020805@tobit.co.uk","threadId":"8617","inReplyTo":"20070617103744.GE1828@planck.djpig.de","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Dirk Koopman","fromEmail":"djk@tobit.co.uk","sentAt":"2007-06-17T16:53:27Z","receivedAt":"2007-06-17T16:53:27Z","isPatch":true,"sender":{"key":"djk@tobit.co.uk","avatar":null},"body":"Frank Lichtenheld wrote:\n\n> \n> Summary: You're (ab)using cvsserver in very interesting ways that are not\n> really beeing thought of in the current design/implementation. There'll\n> be dragons ;)\n> \n\nHmm... I think that is becoming clear. The trouble is that I am not at \nall certain that what I am doing is particularly unusual. After all, \nusing git, the whole point is that working on branches or the main line \nshould easy and cheap!\n\nIf it were me, I might have been inclined to always set Repository to \n'master' (or even to the name of the repository with .git removed), then \ngit checkout <tag> <file> each file, one at a time, using the (<tag> || \n'master') from each Entry that is sent. So with no tag, you get the \nmaster copy, otherwise the <tag>ged copy - this all assuming that the \ngit repo is set up correctly.\n\nBut as I am CVS read only, what is there does for me so I am not \ncomplaining :-) The two people that can also commit can start to use git \nand send me patches... Do them good :-)\n\nDirk\n"},{"id":"45232","messageId":"20070617172041.GF1828@planck.djpig.de","threadId":"8617","inReplyTo":"46756707.5020805@tobit.co.uk","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-17T17:20:41Z","receivedAt":"2007-06-17T17:20:41Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Sun, Jun 17, 2007 at 05:53:27PM +0100, Dirk Koopman wrote:\n> Frank Lichtenheld wrote:\n> >Summary: You're (ab)using cvsserver in very interesting ways that are not\n> >really beeing thought of in the current design/implementation. There'll\n> >be dragons ;)\n> >\n> \n> Hmm... I think that is becoming clear. The trouble is that I am not at \n> all certain that what I am doing is particularly unusual. After all, \n> using git, the whole point is that working on branches or the main line \n> should easy and cheap!\n\nSure, it is a know limitation of cvsserver. But it is not trivially\nto remove. Patches welcome ;)\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nwww: http://www.djpig.de/\n"},{"id":"45246","messageId":"46a038f90706171427g43c4ccf2vdf1172a962481964@mail.gmail.com","threadId":"8617","inReplyTo":"20070617103744.GE1828@planck.djpig.de","subject":"Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-06-17T21:27:17Z","receivedAt":"2007-06-17T21:27:17Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 6/17/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:\n> On Sun, Jun 17, 2007 at 10:10:51AM +0100, Dirk Koopman wrote:\n> > Frank Lichtenheld wrote:\n> > >On Sat, Jun 16, 2007 at 07:50:06PM +0100, Dirk Koopman wrote:\n> > >Hmm, I don't see how you could have a problem with that since cvsserver\n> > >doesn't support branches and never generates any revision numbers in\n> > >that format?\n> > >\n> > >There is probably much more code out there in cvsserver that does assume\n> > >that revision is always a simple integer.\n>\n> Let me rephrase that (after actually looking through the code):\n> All of the revision handling code assumes that.\n\nExactly. cvsserver emulates CVS on a single HEAD, that's why you use\nthe headname as the 'module' parameter you pass to CVS when doing a\ncheckout.\n\n...\n\n> Hmm, so you did the cvs update in an old working copy of the original\n> CVS repository? Then CVS sent those version numbers from the CVS/Entries\n> file to the server, cvsserver certainly never generates numbers like\n> that. And I would be very suprised if you could do anything remotely\n> useful with abusing the old working copy this way...\n\nAgreed - that's not really supported.\n\nNow, I'd _love_ to have a bit of time to implement CVS-style branch\nsupport to cvsserver (so a check for valid version numbers that have\nmore dots would be a good thing), but it's hard hard hard, specially\nbecause there are many ambiguities to resolve. It would enormously\nuseful to have branch support together with support for a bit of\n\"version skew\" so that you can replace a real CVS server with\ncvsserver and have people continue using the old cvs checkouts --\nbecause the file versions and branches match.\n\nAs things stand, I want to say thanks to Frank for giving cvsserver\nsome love :-)\n\ncheers,\n\n\nmartin\n"}]}