git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Karpinski <stefan.karpinski@gmail.com>
To: Andy Parkins <andyparkins@gmail.com>,
	Michael Witten <mfwitten@mit.edu>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org
Cc: Stefan Karpinski <stefan.karpinski@gmail.com>
Subject: [PATCH] git-cvsserver: run post-update hook *after* update.
Date: Thu, 29 Jan 2009 13:58:02 -0800	[thread overview]
Message-ID: <1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com> (raw)
In-Reply-To: <7viqo61mfq.fsf@gitster.siamese.dyndns.org>

CVS server was running the hook before the update action was
actually done. This performs the update before the hook is called.

The original commit that introduced the current incorrect behavior
was 394d66d "git-cvsserver runs hooks/post-update". The error in
ordering of the hook call appears to have gone unnoticed, but since
git-cvsserver is supposed to emulate receive-pack, it stands to
reason that the hook should be run *after* the update. Since this
behavior is inconsistent with recieve-pack, users are either:

  1) not using post-update hooks with git-cvsserver;
  2) using post-update hooks that don't care whether they are
     called before or after the actual update occurs;
  3) using post-update hooks *only* with git-cvsserver, and
     relying on the hook being called just before the update.

This patch would affect only users in case 3. These users are
depending on fairly obviously wrong behavior, and moreover they can
simply change their current post-update into post-recieve hooks,
and their systems will work correctly again.

Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>
---
I'm CCing Andy Parkins, Michael Witten, and Junio Hamano, who
authored the other three commits implementing or affecting hooks in
git-cvsserver (394d66d, cdf6328, b2741f6). If you could please take
a look at this patch and comment on if it's harmful or not, it
would be much appreciated.

 git-cvsserver.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index c1e09ea..d2e6003 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1413,14 +1413,14 @@ sub req_ci
 		close $pipe || die "bad pipe: $! $?";
 	}
 
+    $updater->update();
+
 	### Then hooks/post-update
 	$hook = $ENV{GIT_DIR}.'hooks/post-update';
 	if (-x $hook) {
 		system($hook, "refs/heads/$state->{module}");
 	}
 
-    $updater->update();
-
     # foreach file specified on the command line ...
     foreach my $filename ( @committedfiles )
     {
-- 
1.6.0.3.3.g08dd8

  reply	other threads:[~2009-01-29 21:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1232144521-21947-1-git-send-email-stefan.karpinski@gmail.com>
     [not found] ` <1232144521-21947-2-git-send-email-stefan.karpinski@gmail.com>
2009-01-23  5:43   ` [PATCH] git-cvsserver: run post-update hook *after* update Stefan Karpinski
2009-01-23  8:00     ` Junio C Hamano
2009-01-29 21:58       ` Stefan Karpinski [this message]
2009-01-29 22:48         ` Junio C Hamano
2009-01-29 23:26           ` Stefan Karpinski
2009-01-29 22:56         ` Andy Parkins
2009-01-29 21:35 ` [PATCH] git-cvsserver: handle CVS 'noop' command Stefan Karpinski
2009-01-29 22:45   ` Junio C Hamano
2009-01-29 23:39     ` Stefan Karpinski
2009-01-29 23:46       ` Junio C Hamano
2009-01-30  1:12         ` Stefan Karpinski
2009-01-30  1:32     ` Martin Langhoff

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com \
    --to=stefan.karpinski@gmail.com \
    --cc=andyparkins@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mfwitten@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).