{"thread":{"id":"10230","subject":"[PATCH] git-cvsserver: added support for update -p","startedAt":"2007-10-10T11:16:03Z","lastAt":"2007-10-11T21:07:37Z","messageCount":13,"participants":["Jan Wielemaker","Johannes Schindelin","Frank Lichtenheld","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"55360","messageId":"200710101316.03633.jan@swi-prolog.org","threadId":"10230","inReplyTo":null,"subject":"[PATCH] git-cvsserver: added support for update -p","fromName":"Jan Wielemaker","fromEmail":"jan@swi-prolog.org","sentAt":"2007-10-10T11:16:03Z","receivedAt":"2007-10-10T11:16:03Z","isPatch":true,"sender":{"key":"jan@swi-prolog.org","avatar":null},"body":"[PATCH] git-cvsserver: added support for update -p\n---\nHi,\n\nSomeone in our team uses \"cvs update -p [-r rev] file\" (somehow invoked\nthrough TortoiseCVS). The patch below provides that. I think it is fine,\nexcept that I don't know with wich other flags -p can be combined and\ntherefore when exactly this should be tested. Figured out that normal\nCVS sends the file line-by-line preceeded by \"M \" using strace on the\nclient to a real CVS server.\n\n\tEnjoy --- Jan\n\n git-cvsserver.perl | 15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 13dbd27..987f4d6 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -956,6 +956,21 @@ sub req_update\n $meta = $updater->getmeta($filename);\n }\n \n+\t# if we have a -p we should just send the file\n+ if ( exists ( $state->{opt}{p} ) )\n+\t{\n+\t if ( open my $fh, '-|', \"git-cat-file\", \"blob\", $meta->{filehash} )\n+\t { while ( <$fh> )\n+\t\t{ print \"M \" . $_;\n+\t\t}\n+\t\tclose $fh or die (\"Couldn't close filehandle for transmitfile(): $!\");\n+\t } else\n+\t { die(\"Couldn't execute git-cat-file\");\n+\t }\n+\n+\t next;\n+\t}\n+\n \tif ( ! defined $meta )\n \t{\n \t $meta = {\n-- \n1.5.3.4\n"},{"id":"55366","messageId":"Pine.LNX.4.64.0710101442571.4174@racer.site","threadId":"10230","inReplyTo":"200710101316.03633.jan@swi-prolog.org","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-10T13:47:11Z","receivedAt":"2007-10-10T13:47:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Oct 2007, Jan Wielemaker wrote:\n\n> [PATCH] git-cvsserver: added support for update -p\n> ---\n\nProposed alternative for the commit message:\n\n-- snip --\nThe cvs subcommand \"update -p \" is frequently used to see the \ncontents of a given file in HEAD, sort of our \"git show \". It\nis not that hard to support it, so here it is.\n\nCommit-message-proposed-by: Johannes Schindelin \nSigned-off-by: Jan Wielemaker \n-- snap --\n\nRemember: having such a commit message already at the beginning of your \nmail body makes it easier to everyone reading your email, for a small \ncost (time) of just one person (you).\n\nCiao,\nDscho\n\nP.S.: Have not reviewed the patch at all, so cannot say anything about the \nmerits of it; will leave it to djpig ;-)\n"},{"id":"55372","messageId":"200710101626.53303.jan@swi-prolog.org","threadId":"10230","inReplyTo":"Pine.LNX.4.64.0710101442571.4174@racer.site","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Jan Wielemaker","fromEmail":"jan@swi-prolog.org","sentAt":"2007-10-10T14:26:53Z","receivedAt":"2007-10-10T14:26:53Z","isPatch":true,"sender":{"key":"jan@swi-prolog.org","avatar":null},"body":"On Wednesday 10 October 2007 15:47, Johannes Schindelin wrote:\n> Hi,\n>\n> On Wed, 10 Oct 2007, Jan Wielemaker wrote:\n> > [PATCH] git-cvsserver: added support for update -p\n> > ---\n>\n> Proposed alternative for the commit message:\n>\n> -- snip --\n> The cvs subcommand \"update -p \" is frequently used to see the\n> contents of a given file in HEAD, sort of our \"git show \". It\n> is not that hard to support it, so here it is.\n>\n> Commit-message-proposed-by: Johannes Schindelin \n> Signed-off-by: Jan Wielemaker \n> -- snap --\n\nOk. I'm still a guy of ChangeLog files, which you generally needed for\nCVS to keep track of a large project :-) As the CVS commit message\naren't much good anyway, I kept them short. Also for my own project I'm\nconsidering to replace these with larger commit messages and drop the\nChangeLog files.\n\n> P.S.: Have not reviewed the patch at all, so cannot say anything about the\n> merits of it; will leave it to djpig ;-)\n\nDon't trust my Perl; its just copy and intelligent(-ish) paste :-) Works\nfor me though and this isn't very complicated. Is there a test suite for\ngit-cvsserver?\n\n\tCheers --- Jan\n"},{"id":"55388","messageId":"Pine.LNX.4.64.0710101740400.4174@racer.site","threadId":"10230","inReplyTo":"200710101626.53303.jan@swi-prolog.org","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-10T16:41:01Z","receivedAt":"2007-10-10T16:41:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Oct 2007, Jan Wielemaker wrote:\n\n> Is there a test suite for git-cvsserver?\n\nYes: t/t9400-git-cvsserver-server.sh\n\nHth,\nDscho\n"},{"id":"55391","messageId":"200710101927.38949.wielemak@science.uva.nl","threadId":"10230","inReplyTo":"Pine.LNX.4.64.0710101740400.4174@racer.site","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Jan Wielemaker","fromEmail":"wielemak@science.uva.nl","sentAt":"2007-10-10T17:27:38Z","receivedAt":"2007-10-10T17:27:38Z","isPatch":true,"sender":{"key":"wielemak@science.uva.nl","avatar":null},"body":"> On Wed, 10 Oct 2007, Jan Wielemaker wrote:\n> > Is there a test suite for git-cvsserver?\n>\n> Yes: t/t9400-git-cvsserver-server.sh\n\nThanks. B.t.w. from the main directory:\n\ngollem (git) 21_> make check\nfor i in *.c; do \nsparse -g -O2 -Wall -DSHA1_HEADER='' -DETC_GITCONFIG='\"/home/jan/etc/gitconfig\"' -DNO_STRLCPY -D__BIG_ENDIAN__ -D__powerpc__ \n$i || exit; done\n/bin/sh: sparse: command not found\nmake: *** [check] Error 127\n\nDunno, but maybe something like this is more appropriate:\n\n\techo \"See t/README for testing GIT\"\n\n\tCheers --- Jan\n\nP.s.\tMy modified version passes all tests.\n"},{"id":"55399","messageId":"Pine.LNX.4.64.0710102025390.4174@racer.site","threadId":"10230","inReplyTo":"200710101927.38949.wielemak@science.uva.nl","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-10T19:27:15Z","receivedAt":"2007-10-10T19:27:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Oct 2007, Jan Wielemaker wrote:\n\n> > On Wed, 10 Oct 2007, Jan Wielemaker wrote:\n> > > Is there a test suite for git-cvsserver?\n> >\n> > Yes: t/t9400-git-cvsserver-server.sh\n> \n> Thanks. B.t.w. from the main directory:\n> \n> gollem (git) 21_> make check\n\nmake check is to check with the static code analyzer \"sparse\".\n\nTo test, try \"make test\". Since this is so commonly used to test \npackages (for example, the vast majority of Perl packages have it), I do \nnot see the need to put a message pointing to \"make test\" in the \"check\" \ntarget.\n\nCiao,\nDscho\n"},{"id":"55405","messageId":"20071010200029.GK31659@planck.djpig.de","threadId":"10230","inReplyTo":"200710101316.03633.jan@swi-prolog.org","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-10-10T20:00:30Z","receivedAt":"2007-10-10T20:00:30Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Wed, Oct 10, 2007 at 01:16:03PM +0200, Jan Wielemaker wrote:\n> +\t# if we have a -p we should just send the file\n> + if ( exists ( $state->{opt}{p} ) )\n> +\t{\n> +\t if ( open my $fh, '-|', \"git-cat-file\", \"blob\", $meta->{filehash} )\n> +\t { while ( <$fh> )\n> +\t\t{ print \"M \" . $_;\n> +\t\t}\n> +\t\tclose $fh or die (\"Couldn't close filehandle for transmitfile(): $!\");\n> +\t } else\n> +\t { die(\"Couldn't execute git-cat-file\");\n> +\t }\n> +\n> +\t next;\n> +\t}\n\n\nThere seems to be inconsistent whitespace in the patch.\nAnd please never do that else\\n{ again, it hurts my eye ;)\n\nWill try to test (and write a testcase for) it tomorrow. \n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"55456","messageId":"470DE29A.5010001@op5.se","threadId":"10230","inReplyTo":"20071010200029.GK31659@planck.djpig.de","subject":"Re: [PATCH] git-cvsserver: added support for update -p","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-11T08:45:14Z","receivedAt":"2007-10-11T08:45:14Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Frank Lichtenheld wrote:\n> On Wed, Oct 10, 2007 at 01:16:03PM +0200, Jan Wielemaker wrote:\n>> +\t# if we have a -p we should just send the file\n>> + if ( exists ( $state->{opt}{p} ) )\n>> +\t{\n>> +\t if ( open my $fh, '-|', \"git-cat-file\", \"blob\", $meta->{filehash} )\n>> +\t { while ( <$fh> )\n>> +\t\t{ print \"M \" . $_;\n>> +\t\t}\n>> +\t\tclose $fh or die (\"Couldn't close filehandle for transmitfile(): $!\");\n>> +\t } else\n>> +\t { die(\"Couldn't execute git-cat-file\");\n>> +\t }\n>> +\n>> +\t next;\n>> +\t}\n> \n> \n> There seems to be inconsistent whitespace in the patch.\n> And please never do that else\\n{ again, it hurts my eye ;)\n> \n\nThat cuddled opening brace hurts mine more.\n\n{ while()\\n{ print()...\n\nIt's usually a good idea to pick some indentation style that at least *some* tool\ncan create, and when contributing to a project it's usually considered good form\nto stick to the style already used.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"55481","messageId":"1192120573-16765-1-git-send-email-frank@lichtenheld.de","threadId":"10230","inReplyTo":"200710101316.03633.jan@swi-prolog.org","subject":"[PATCH] cvsserver: added support for update -p","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-10-11T16:36:13Z","receivedAt":"2007-10-11T16:36:13Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"Based on a patch by Jan Wielemaker .\n\nSigned-off-by: Frank Lichtenheld \n---\n git-cvsserver.perl | 23 +++++++++++++++++++++++\n t/t9400-git-cvsserver-server.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 0 deletions(-)\n\n Test cases added and fixed behaviour for non-existant files.\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 2e112fa..7374875 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -973,6 +973,29 @@ sub req_update\n $meta = $updater->getmeta($filename);\n }\n \n+\t# if we have a -p we should just send the file\n+\tif ( exists ( $state->{opt}{p} ) )\n+\t{\n+\t if (! defined $meta)\n+\t {\n+\t\t# non-existant files are ignored\n+\t\tprint \"E cvs update: nothing known about `$filename'\\n\";\n+\t\tnext;\n+\t }\n+\t if ( open my $fh, '-|', \"git-cat-file\", \"blob\", $meta->{filehash} )\n+\t {\n+\t\twhile ( <$fh> )\n+\t\t{\n+\t\t print \"M $_\";\n+\t\t}\n+\t\tclose $fh or die (\"Couldn't close filehandle: $!\");\n+\t } else {\n+\t\tdie(\"Couldn't execute git-cat-file\");\n+\t }\n+\n+\t next;\n+\t}\n+\n \tif ( ! defined $meta )\n \t{\n \t $meta = {\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex ee58c0f..4f45578 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -446,6 +446,38 @@ test_expect_success 'cvs update (merge no-op)' \\\n diff -q merge ../merge'\n \n cd \"$WORKDIR\"\n+test_expect_success 'cvs update (-p)' \\\n+ 'cd cvswork &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update -p merge non-existant testfile1 empty >log &&\n+ cat merge testfile1 empty >../expected &&\n+ diff -q log ../expected'\n+\n+cd \"$WORKDIR\"\n+test_expect_success 'cvs update (-p -r)' \\\n+ 'echo testfile1 >expected &&\n+ for i in 1 2 3 4 5 6 7\n+ do\n+ echo Line $i >>expected\n+ done &&\n+ echo >>expected &&\n+ cd cvswork &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update -p -r1.1 testfile1 merge empty >log &&\n+ diff -q log ../expected'\n+\n+cd \"$WORKDIR\"\n+test_expect_success 'cvs update (-p unclean and out-of-date)' \\\n+ 'echo testfile2 >testfile2 &&\n+ echo Line 10 >>merge &&\n+ git add testfile2 merge &&\n+ git commit -q -m \"update -p\" &&\n+ git push gitcvs.git >/dev/null &&\n+ cat testfile2 merge >expected &&\n+ cd cvswork &&\n+ echo \"Line 10 workdir\" >>merge\n+ GIT_CONFIG=\"$git_config\" cvs -Q update -p testfile2 merge >log &&\n+ diff -q log ../expected'\n+\n+cd \"$WORKDIR\"\n cat <list-modules-cmd\n Root $SERVERDIR\n Valid-responses ok error Valid-requests Force-gzip Referrer Redirect Checked-in New-entry Checksum Copy-file Updated Created Update-existing Merged Patched Rcs-diff Mode Mod-time Removed Remove-entry Set-static-directory Clear-static-directory Set-sticky Clear-sticky Edit-file Template Clear-template Notified Module-expansion Wrapper-rcsOption M Mbinary E F MT\n-- \n1.5.3.4\n"},{"id":"55482","messageId":"200710111852.32740.wielemak@science.uva.nl","threadId":"10230","inReplyTo":"1192120573-16765-1-git-send-email-frank@lichtenheld.de","subject":"Re: [PATCH] cvsserver: added support for update -p","fromName":"Jan Wielemaker","fromEmail":"wielemak@science.uva.nl","sentAt":"2007-10-11T16:52:32Z","receivedAt":"2007-10-11T16:52:32Z","isPatch":true,"sender":{"key":"wielemak@science.uva.nl","avatar":null},"body":"On Thursday 11 October 2007 18:36, Frank Lichtenheld wrote:\n> Based on a patch by Jan Wielemaker .\n>\n> Signed-off-by: Frank Lichtenheld \n\nThanks. You are a bigger Perl programmer than I :-) Are you also\ninterested in one that makes \"cvs diff -c\" work? It works, but it\ndoes not handle things like \"cvs diff -C 5\" and I'm a bit lost in\nPerl-space ... If someone knowing more about the server wants to\nhave a look, I'm happy to post the part I have.\n\n\tCheers --- Jan\n"},{"id":"55486","messageId":"20071011172938.GN31659@planck.djpig.de","threadId":"10230","inReplyTo":"200710111852.32740.wielemak@science.uva.nl","subject":"Re: [PATCH] cvsserver: added support for update -p","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-10-11T17:29:38Z","receivedAt":"2007-10-11T17:29:38Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Thu, Oct 11, 2007 at 06:52:32PM +0200, Jan Wielemaker wrote:\n> On Thursday 11 October 2007 18:36, Frank Lichtenheld wrote:\n> > Based on a patch by Jan Wielemaker .\n> >\n> > Signed-off-by: Frank Lichtenheld \n> \n> Thanks. You are a bigger Perl programmer than I :-) Are you also\n> interested in one that makes \"cvs diff -c\" work? It works, but it\n> does not handle things like \"cvs diff -C 5\" and I'm a bit lost in\n> Perl-space ... If someone knowing more about the server wants to\n> have a look, I'm happy to post the part I have.\n\nHmm, the more half-patches you submit the more I'd rather prefer you\nlearning Perl ;) Or at least write your own testcases.\n\ndiff -c doesn't really interest me at all. So I'd really prefer you\ndoing the bulk of the work...\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"55505","messageId":"Pine.LNX.4.64.0710112158330.4174@racer.site","threadId":"10230","inReplyTo":"1192120573-16765-1-git-send-email-frank@lichtenheld.de","subject":"Re: [PATCH] cvsserver: added support for update -p","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-11T20:59:28Z","receivedAt":"2007-10-11T20:59:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 11 Oct 2007, Frank Lichtenheld wrote:\n\n> +\tif ( exists ( $state->{opt}{p} ) )\n\nI see you kept the coding style, which is not in agreement with the rest \nof git... Intention or oversight?\n\nCiao,\nDscho\n"},{"id":"55506","messageId":"20071011210737.GP31659@planck.djpig.de","threadId":"10230","inReplyTo":"Pine.LNX.4.64.0710112158330.4174@racer.site","subject":"Re: [PATCH] cvsserver: added support for update -p","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-10-11T21:07:37Z","receivedAt":"2007-10-11T21:07:37Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Thu, Oct 11, 2007 at 09:59:28PM +0100, Johannes Schindelin wrote:\n> On Thu, 11 Oct 2007, Frank Lichtenheld wrote:\n> \n> > +\tif ( exists ( $state->{opt}{p} ) )\n> \n> I see you kept the coding style, which is not in agreement with the rest \n> of git... Intention or oversight?\n\nIt is in agreement with the rest of git-cvsserver. I really like the\nstyle of the other perl stuff in git better, but I wasn't sure what\nstyle takes precedence...\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"}]}