{"thread":{"id":"10989","subject":"[PATCH] git-cvsserver runs hooks/post-receive","startedAt":"2007-11-23T09:12:54Z","lastAt":"2007-12-01T19:38:37Z","messageCount":11,"participants":["Michael Witten","Junio C Hamano","Johannes Schindelin","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"60747","messageId":"1195809174-28142-1-git-send-email-mfwitten@mit.edu","threadId":"10989","inReplyTo":null,"subject":"[PATCH] git-cvsserver runs hooks/post-receive","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-11-23T09:12:54Z","receivedAt":"2007-11-23T09:12:54Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"git-cvsserver just did the following:\n    (1) run hooks/update\n    (2) commit if hooks/update passed\n\nThis commit simply adds:\n    (3) run hooks/post-receive\n\nAlso, there are a few grammar cleanups and\nconsistency improvements.\n\nSigned-off-by: Michael Witten <mfwitten@mit.edu>\n---\n git-cvsserver.perl |   27 ++++++++++++++++++++-------\n 1 files changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 0d55fec..8d4d735 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1211,13 +1211,13 @@ sub req_ci\n \n     chdir $tmpdir;\n \n-    # populate the temporary index based\n+    # populate the temporary index\n     system(\"git-read-tree\", $parenthash);\n     unless ($? == 0)\n     {\n \tdie \"Error running git-read-tree $state->{module} $file_index $!\";\n     }\n-    $log->info(\"Created index '$file_index' with for head $state->{module} - exit status $?\");\n+    $log->info(\"Created index '$file_index' for head $state->{module} - exit status $?\");\n \n     my @committedfiles = ();\n     my %oldmeta;\n@@ -1237,7 +1237,7 @@ sub req_ci\n \n         my ( $filepart, $dirpart ) = filenamesplit($filename);\n \n-        # do a checkout of the file if it part of this tree\n+        # do a checkout of the file if it is part of this tree\n         if ($wrev) {\n             system('git-checkout-index', '-f', '-u', $filename);\n             unless ($? == 0) {\n@@ -1324,11 +1324,11 @@ sub req_ci\n         exit;\n     }\n \n-\t# Check that this is allowed, just as we would with a receive-pack\n-\tmy @cmd = ( $ENV{GIT_DIR}.'hooks/update', \"refs/heads/$state->{module}\",\n+\t### Emulate git-receive-pack by running hooks/update\n+\tmy @hook = ( $ENV{GIT_DIR}.'hooks/update', \"refs/heads/$state->{module}\",\n \t\t\t$parenthash, $commithash );\n-\tif( -x $cmd[0] ) {\n-\t\tunless( system( @cmd ) == 0 )\n+\tif( -x $hook[0] ) {\n+\t\tunless( system( @hook ) == 0 )\n \t\t{\n \t\t\t$log->warn(\"Commit failed (update hook declined to update ref)\");\n \t\t\tprint \"error 1 Commit failed (update hook declined)\\n\";\n@@ -1337,6 +1337,7 @@ sub req_ci\n \t\t}\n \t}\n \n+\t### Update the ref\n \tif (system(qw(git update-ref -m), \"cvsserver ci\",\n \t\t\t\"refs/heads/$state->{module}\", $commithash, $parenthash)) {\n \t\t$log->warn(\"update-ref for $state->{module} failed.\");\n@@ -1344,6 +1345,18 @@ sub req_ci\n \t\texit;\n \t}\n \n+\t### Emulate git-receive-pack by running hooks/post-receive\n+\tmy $hook = $ENV{GIT_DIR}.'hooks/post-receive';\n+\tif( -x $hook ) {\n+\t\topen(my $pipe, \"| $hook\") || die \"can't fork $!\";\n+\n+\t\tlocal $SIG{PIPE} = sub { die 'pipe broke' };\n+\n+\t\tprint $pipe \"$parenthash $commithash refs/heads/$state->{module}\\n\";\n+\n+\t\tclose $pipe || die \"bad pipe: $! $?\";\n+\t}\n+\n     $updater->update();\n \n     # foreach file specified on the command line ...\n-- \n1.5.3.5.1623.g37154\n"},{"id":"61340","messageId":"7v3aup291c.fsf@gitster.siamese.dyndns.org","threadId":"10989","inReplyTo":"1195809174-28142-1-git-send-email-mfwitten@mit.edu","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-29T00:24:31Z","receivedAt":"2007-11-29T00:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@mit.edu> writes:\n\n> git-cvsserver just did the following:\n>     (1) run hooks/update\n>     (2) commit if hooks/update passed\n>\n> This commit simply adds:\n>     (3) run hooks/post-receive\n>\n> Also, there are a few grammar cleanups and\n> consistency improvements.\n\nI gave only a very cursory look; looks Ok to me.  This makes me wonder\nif post-update wants to run as well.\n\nWhat do other people who run git-cvsserver think?\n"},{"id":"61474","messageId":"7F81126E-5A76-40CA-94BF-82B46C57AFF6@mit.edu","threadId":"10989","inReplyTo":"7v3aup291c.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-11-30T04:06:31Z","receivedAt":"2007-11-30T04:06:31Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 28 Nov 2007, at 7:24:31 PM, Junio C Hamano wrote:\n\n> Michael Witten <mfwitten@mit.edu> writes:\n>\n>> git-cvsserver just did the following:\n>>     (1) run hooks/update\n>>     (2) commit if hooks/update passed\n>>\n>> This commit simply adds:\n>>     (3) run hooks/post-receive\n>>\n>> Also, there are a few grammar cleanups and\n>> consistency improvements.\n>\n> I gave only a very cursory look; looks Ok to me.  This makes me wonder\n> if post-update wants to run as well.\n\nSeems like post-receive is supposed to supersede post-update anyhow,\nso might as well leave post-update out at this point?\n\nIn any case, I haven't taken a thorough look at how git-cvsserver works,\nbut it seems to duplicate a lot of git-receive-pack.\n\nHow about turning git-cvsserver into a true middleman, so that it  \nconstructs\na 'temporary git working tree' and then does a real git-push into the  \nfinal\ngit repository.\n\nBecause git-cvsserver would be using git-send-pack to do the pushing,  \nit could\npush to yet another server. How cool is that!\n"},{"id":"61496","messageId":"Pine.LNX.4.64.0711301202230.27959@racer.site","threadId":"10989","inReplyTo":"7F81126E-5A76-40CA-94BF-82B46C57AFF6@mit.edu","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-30T12:03:15Z","receivedAt":"2007-11-30T12:03:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 Nov 2007, Michael Witten wrote:\n\n> In any case, I haven't taken a thorough look at how git-cvsserver works, \n> but it seems to duplicate a lot of git-receive-pack.\n> \n> How about turning git-cvsserver into a true middleman, so that it \n> constructs a 'temporary git working tree' and then does a real git-push \n> into the final git repository.\n\nThat would yield a horrible performance.\n\nWould be opposed, if a regular cvsserver user,\nDscho\n"},{"id":"61501","messageId":"BDA3CE08-FFA4-4D84-A2FC-5810AAA6EEAB@MIT.EDU","threadId":"10989","inReplyTo":"Pine.LNX.4.64.0711301202230.27959@racer.site","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-11-30T12:26:41Z","receivedAt":"2007-11-30T12:26:41Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 30 Nov 2007, at 7:03:15 AM, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Thu, 29 Nov 2007, Michael Witten wrote:\n>\n>> In any case, I haven't taken a thorough look at how git-cvsserver  \n>> works,\n>> but it seems to duplicate a lot of git-receive-pack.\n>>\n>> How about turning git-cvsserver into a true middleman, so that it\n>> constructs a 'temporary git working tree' and then does a real git- \n>> push\n>> into the final git repository.\n>\n> That would yield a horrible performance.\n>\n> Would be opposed, if a regular cvsserver user,\n\nHow come?\n\ngit-cvsserver it seems already does just that!\nThe difference is that it puts objects in place\nby hand, requiring the code to mirror hook calls\nanyway.\n\nI'm simply proposing that the code be reworked,\nso that cvs commits actually become git pushes,\nso that all future changes to the pushing mechanism\nare automatically handled.\n\nIt just makes sense as far as design; git-cvsserver\nshould be an adaptor between the two kinds of repositories,\nnot an infiltrator.\n"},{"id":"61502","messageId":"Pine.LNX.4.64.0711301317060.27959@racer.site","threadId":"10989","inReplyTo":"BDA3CE08-FFA4-4D84-A2FC-5810AAA6EEAB@MIT.EDU","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-30T13:19:07Z","receivedAt":"2007-11-30T13:19:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Nov 2007, Michael Witten wrote:\n\n> On 30 Nov 2007, at 7:03:15 AM, Johannes Schindelin wrote:\n> \n> > On Thu, 29 Nov 2007, Michael Witten wrote:\n> > \n> > > In any case, I haven't taken a thorough look at how git-cvsserver \n> > > works, but it seems to duplicate a lot of git-receive-pack.\n> > > \n> > > How about turning git-cvsserver into a true middleman, so that it \n> > > constructs a 'temporary git working tree' and then does a real \n> > > git-push into the final git repository.\n> > \n> > That would yield a horrible performance.\n> > \n> > Would be opposed, if a regular cvsserver user,\n> \n> How come?\n\nI remember driving git-cvsserver on a bare repository, with \"ln -s \n/usr/bin/git-cvsserver ~cvsuser/bin/\", and it was superfast.\n\nOne user even asked if I upgraded my cvs, because it was so much faster \nthan before.  I said yes.\n\nCiao,\nDscho\n"},{"id":"61503","messageId":"47500F0B.10300@viscovery.net","threadId":"10989","inReplyTo":"BDA3CE08-FFA4-4D84-A2FC-5810AAA6EEAB@MIT.EDU","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-30T13:24:27Z","receivedAt":"2007-11-30T13:24:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michael Witten schrieb:\n> On 30 Nov 2007, at 7:03:15 AM, Johannes Schindelin wrote:\n>> On Thu, 29 Nov 2007, Michael Witten wrote:\n>>> How about turning git-cvsserver into a true middleman, so that it\n>>> constructs a 'temporary git working tree' and then does a real git-push\n>>> into the final git repository.\n>>\n>> That would yield a horrible performance.\n>>\n>> Would be opposed, if a regular cvsserver user,\n> \n> How come?\n> \n> git-cvsserver it seems already does just that!\n> The difference is that it puts objects in place\n> by hand, requiring the code to mirror hook calls\n> anyway.\n> \n> I'm simply proposing that the code be reworked,\n> so that cvs commits actually become git pushes,\n> so that all future changes to the pushing mechanism\n> are automatically handled.\n\nBut in order push something, you must first have the commit in a repository. \nHow would git-cvsserver do that? For example, by putting objects in place by \nhand. You gain nothing, except that it would push instead of call the hooks \ndirectly.\n\n-- Hannes\n"},{"id":"61531","messageId":"AB064BA5-1F99-4CCF-9BA5-048BB14E9157@MIT.EDU","threadId":"10989","inReplyTo":"47500F0B.10300@viscovery.net","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-12-01T00:19:32Z","receivedAt":"2007-12-01T00:19:32Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 30 Nov 2007, at 8:24:27 AM, Johannes Sixt wrote:\n\n>> The difference is that it puts objects in place\n>> by hand, requiring the code to mirror hook calls\n>> anyway.\n>> I'm simply proposing that the code be reworked,\n>> so that cvs commits actually become git pushes,\n>> so that all future changes to the pushing mechanism\n>> are automatically handled.\n>\n> But in order push something, you must first have the commit in a  \n> repository. How would git-cvsserver do that? For example, by  \n> putting objects in place by hand. You gain nothing, except that it  \n> would push instead of call the hooks directly.\n\n\"The difference is that it puts objects in place\nby hand, requiring the code to mirror hook calls\nanyway.\"\n\nAs far as I can tell, hook calls are the only thing.\n\nHowever, I still had to write a patch to include post-receive;\nby doing everything by hand, git-cvsserver is inherently unsafe\nand incomplete over time.\n\nThe question is not the RESULT, but HOW YOU GET that result;\nthe current design is---essentially---a hack; it is written\nin perl, after all ;-P\n"},{"id":"61547","messageId":"7vmysv87jm.fsf@gitster.siamese.dyndns.org","threadId":"10989","inReplyTo":"7F81126E-5A76-40CA-94BF-82B46C57AFF6@mit.edu","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-01T02:37:01Z","receivedAt":"2007-12-01T02:37:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@MIT.EDU> writes:\n\n>> I gave only a very cursory look; looks Ok to me.  This makes me wonder\n>> if post-update wants to run as well.\n>\n> Seems like post-receive is supposed to supersede post-update anyhow,\n> so might as well leave post-update out at this point?\n\nI'll queue your patch, but I think it should be enhanced to support\npost-update for consistency.  I do not think anybody said anything about\ndeprecating let alone dropping support for post-update so far.\n\nWhen we talk about deprecation and later removal of post-update hook, we\nshould drop that additional support from cvsserver and receive-pack at\nthe same time, but not before that.\n"},{"id":"61569","messageId":"BFDB9A00-6E92-4880-9FE7-3D0E392D18A9@mit.edu","threadId":"10989","inReplyTo":"7vmysv87jm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-12-01T09:17:02Z","receivedAt":"2007-12-01T09:17:02Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 30 Nov 2007, at 9:37:01 PM, Junio C Hamano wrote:\n\n> I'll queue your patch, but I think it should be enhanced to support\n> post-update for consistency.\n\nI'll send another patch that includes support for post-update.\n\n> I'll queue your patch,\n\nWill the old patch already be in place?\n\nAlso, I explicitly decided to pipe input into post-receive\nby hand rather than relying on a system() call that someone\nmight exploit maliciously.\n\nUnfortunately, it turns out that open() with a pipe essentially\ninvokes system(); the solution is to fork a child process and\nthen to turn the child into the process with which communication\nis desired via a call to exec().\n\nBecause the rest of git-cvsserver.perl uses explicit system()\ncalls, I have been wondering if I am being overly cautious.\n"},{"id":"61596","messageId":"7vprxq4342.fsf@gitster.siamese.dyndns.org","threadId":"10989","inReplyTo":"BFDB9A00-6E92-4880-9FE7-3D0E392D18A9@mit.edu","subject":"Re: [PATCH] git-cvsserver runs hooks/post-receive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-01T19:38:37Z","receivedAt":"2007-12-01T19:38:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@MIT.EDU> writes:\n\n> On 30 Nov 2007, at 9:37:01 PM, Junio C Hamano wrote:\n>\n>> I'll queue your patch, but I think it should be enhanced to support\n>> post-update for consistency.\n>\n> I'll send another patch that includes support for post-update.\n>\n>> I'll queue your patch,\n>\n> Will the old patch already be in place?\n\nAlthough I would encourage total replacement patch for things not in\n'next' yet, in this case, I think we both would prefer a separate patch\nthat builds on top of the old patch that teaches the server that it\nshould call post-update as well.\n\nI think it is potentially a sensible thing to do to eventually deprecate\nand then remove post-update hook (as your first patch that is queued\nalready suggests).  I was just saying that 'eventually' is not now.\nWhen that time comes, if you have a separate patch to teach post-update,\nthat single patch can be reverted independently from the old patch,\nwhich should be less error prone than yanking the support out by hand.\n"}]}