git-cvsserver commit trouble (unexpected end of file in client)

git-cvsserver commit trouble (unexpected end of file in client)

[Jan Wielemaker]

Hi,

I'm pretty new to GIT. I quickly convinced our project to move to GIT,
except for one guy that wants to do Windows and CVS :-( I setup a test
and the checkout works just fine, but commit from cvs doesn't work. The
details:

	* Server: git version 1.5.3.1 compiled on SuSE 10.2, 64-bits
	* Client, both SuSE CVS 1.12.12 and the current WinCVS cvs.exe,
	so it appears irrelevant.

I created a test repository from a papers directory using the sequence
beloe. (P.s. isn't there a more elegant way to get to a bare shared repo
from a set of files? I tried (mkdir papers.git && cd papers.git && git
--bare init --shared=all), but I can't clone an empty bare repository
(doesn't create anything), so I can't add to it).

	% cd ~/tmp/papers
	% git-init
	% git-add .
	% git-commit
	% cd /home/git
	% git-clone --bare --no-hardlinks ~/tmp/papers/ papers.git
	% cd papers.git
	% git-config core.sharedrepository all
	% chmod g+ws `find . -type d`
	% chmod g+w `find . -type f`

Anyway, appears to work fine with GIT to clone, pull and push.  Using
CVS over SSH, I can checkout this just fine, creating HEAD.  Now I change
a file and run "cvs commit" to get:

gollem (2006) 11_> cvs commit -m "test"
cvs [commit aborted]: end of file from server (consult above messages if any)

I enabled logging and added a few statements to git-cvsserver (line 1203)

    $log->info("Start git show-ref -s refs/heads/$state->{module}");
    # Remember where the head was at the beginning.
    my $parenthash = `git show-ref -s refs/heads/$state->{module}`;
    $log->info("parenthash = $parenthash");
    chomp $parenthash;
    if ($parenthash !~ /^[0-9a-f]{40}$/) {
            $log->warn("error 1 pserver cannot find the current HEAD of 
module");
            exit;
    }

Then I get this log output:

================================================================
2007-10-03 12:25:16 : DEBUG - Temporary directory is '/tmp/XwYVFFqjyd'
2007-10-03 12:25:16 : DEBUG - req_Root : /home/git/papers.git
2007-10-03 12:25:16 : DEBUG - req_Validresponses : ok error Valid-requests 
Referrer Redirect Checked-in New-entry Checksum Co
py-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 Mbi
nary E F MT
2007-10-03 12:25:16 : DEBUG - req_validrequests
2007-10-03 12:25:16 : DEBUG - SEND : Valid-requests remove add status Entry 
watchers ci tag log co Modified Questionable admi
n Root history valid-requests Global_option Argumentx annotate Valid-responses 
Unchanged Directory rlog Argument expand-modul
es diff editors update
2007-10-03 12:25:16 : DEBUG - SEND : ok
2007-10-03 12:25:16 : DEBUG - req_Globaloption : -q
2007-10-03 12:25:16 : DEBUG - Argument : -m
2007-10-03 12:25:16 : DEBUG - Argument : test
2007-10-03 12:25:16 : DEBUG - Argument : --
2007-10-03 12:25:16 : INFO  - Setting prepend to '2006/'
2007-10-03 12:25:16 : DEBUG - Prepending '2006/' to state|directory
2007-10-03 12:25:16 : DEBUG - req_Directory : localdir=. 
repository=/home/git/papers.git/HEAD/2006 path=2006/ directory=2006/
 module=HEAD
2007-10-03 12:25:16 : INFO  - Received entry line '/README.txt/1.1///' 
=> '2006/README.txt'
2007-10-03 12:25:16 : DEBUG - Argument : README.txt
2007-10-03 12:25:16 : INFO  - req_ci : [NULL]
2007-10-03 12:25:16 : INFO  - Lockless commit start, basing commit 
on '/tmp/XwYVFFqjyd/ud4uGbbUJg', index file is '/tmp/XwYVF
Fqjyd/3FeXMladmb'
2007-10-03 12:25:16 : INFO  - Start git show-ref -s refs/heads/HEAD
2007-10-03 12:25:16 : INFO  - parenthash =
2007-10-03 12:25:16 : WARN  - error 1 pserver cannot find the current HEAD of 
module
================================================================

I don't like the req_ci : [NULL] very much, and the last 3 lines clearly shows 
a problem.  I checked
the latest git repository of git. There is no change to git-cvsserver.perl.

I'm still a bit too newbie to (and not much of a Perl programmer).  Does 
anyone has a clue?  Do I have
the wrong version for something?  Did I setup the repository wrongly?

	Thanks --- Jan

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Jan Wielemaker]

On Wednesday 03 October 2007 13:48, Jan Wielemaker wrote:
> Hi,
>
> I'm pretty new to GIT. I quickly convinced our project to move to GIT,
> except for one guy that wants to do Windows and CVS :-( I setup a test
> and the checkout works just fine, but commit from cvs doesn't work. The
> details:
>
> 	* Server: git version 1.5.3.1 compiled on SuSE 10.2, 64-bits
> 	* Client, both SuSE CVS 1.12.12 and the current WinCVS cvs.exe,
> 	so it appears irrelevant.
>
> I created a test repository from a papers directory using the sequence
> beloe. (P.s. isn't there a more elegant way to get to a bare shared repo
> from a set of files? I tried (mkdir papers.git && cd papers.git && git
> --bare init --shared=all), but I can't clone an empty bare repository
> (doesn't create anything), so I can't add to it).
>
> 	% cd ~/tmp/papers
> 	% git-init
> 	% git-add .
> 	% git-commit
> 	% cd /home/git
> 	% git-clone --bare --no-hardlinks ~/tmp/papers/ papers.git
> 	% cd papers.git
> 	% git-config core.sharedrepository all
> 	% chmod g+ws `find . -type d`
> 	% chmod g+w `find . -type f`
>
> Anyway, appears to work fine with GIT to clone, pull and push.  Using
> CVS over SSH, I can checkout this just fine, creating HEAD.  Now I change
> a file and run "cvs commit" to get:
>
> gollem (2006) 11_> cvs commit -m "test"
> cvs [commit aborted]: end of file from server (consult above messages if
> any)

I start to get a clue.  Adding a line

	$log->info("Heads: " . `git show-ref --heads`);

I see this in the log:

	2007-10-03 14:13:44 : INFO  - Heads: 0b7b372d525a4fe7f662996fec9cd11b1038a6be 
refs/heads/master

Thus, I though I created the repository the wrong way.  Tried again,
following the recipy of 
http://www.kernel.org/pub/software/scm/git/docs/cvs-migration.html:

$ mkdir /pub/my-repo.git
$ cd /pub/my-repo.git
$ git --bare init --shared
$ git --bare fetch /home/alice/myproject master:master

Checked out freshly using CVS. No problem. But committing a change,
nothing changed :-( The log output is exactly the same, showing only
refs/heads/master. I'm starting to suspect git-cvsserver afterall, but
the docs suggests it is operational for quite a while. Could someone
give me a clue on what am I missed?

	Thanks --- Jan 

> I enabled logging and added a few statements to git-cvsserver (line 1203)
>
>     $log->info("Start git show-ref -s refs/heads/$state->{module}");
>     # Remember where the head was at the beginning.
>     my $parenthash = `git show-ref -s refs/heads/$state->{module}`;
>     $log->info("parenthash = $parenthash");
>     chomp $parenthash;
>     if ($parenthash !~ /^[0-9a-f]{40}$/) {
>             $log->warn("error 1 pserver cannot find the current HEAD of
> module");
>             exit;
>     }
>
> Then I get this log output:
>
> ================================================================
> 2007-10-03 12:25:16 : DEBUG - Temporary directory is '/tmp/XwYVFFqjyd'
> 2007-10-03 12:25:16 : DEBUG - req_Root : /home/git/papers.git
> 2007-10-03 12:25:16 : DEBUG - req_Validresponses : ok error Valid-requests
> Referrer Redirect Checked-in New-entry Checksum Co
> py-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 Mbi
> nary E F MT
> 2007-10-03 12:25:16 : DEBUG - req_validrequests
> 2007-10-03 12:25:16 : DEBUG - SEND : Valid-requests remove add status Entry
> watchers ci tag log co Modified Questionable admi
> n Root history valid-requests Global_option Argumentx annotate
> Valid-responses Unchanged Directory rlog Argument expand-modul
> es diff editors update
> 2007-10-03 12:25:16 : DEBUG - SEND : ok
> 2007-10-03 12:25:16 : DEBUG - req_Globaloption : -q
> 2007-10-03 12:25:16 : DEBUG - Argument : -m
> 2007-10-03 12:25:16 : DEBUG - Argument : test
> 2007-10-03 12:25:16 : DEBUG - Argument : --
> 2007-10-03 12:25:16 : INFO  - Setting prepend to '2006/'
> 2007-10-03 12:25:16 : DEBUG - Prepending '2006/' to state|directory
> 2007-10-03 12:25:16 : DEBUG - req_Directory : localdir=.
> repository=/home/git/papers.git/HEAD/2006 path=2006/ directory=2006/
>  module=HEAD
> 2007-10-03 12:25:16 : INFO  - Received entry line '/README.txt/1.1///'
> => '2006/README.txt'
> 2007-10-03 12:25:16 : DEBUG - Argument : README.txt
> 2007-10-03 12:25:16 : INFO  - req_ci : [NULL]
> 2007-10-03 12:25:16 : INFO  - Lockless commit start, basing commit
> on '/tmp/XwYVFFqjyd/ud4uGbbUJg', index file is '/tmp/XwYVF
> Fqjyd/3FeXMladmb'
> 2007-10-03 12:25:16 : INFO  - Start git show-ref -s refs/heads/HEAD
> 2007-10-03 12:25:16 : INFO  - parenthash =
> 2007-10-03 12:25:16 : WARN  - error 1 pserver cannot find the current HEAD
> of module
> ================================================================
>
> I don't like the req_ci : [NULL] very much, and the last 3 lines clearly
> shows a problem.  I checked
> the latest git repository of git. There is no change to git-cvsserver.perl.
>
> I'm still a bit too newbie to (and not much of a Perl programmer).  Does
> anyone has a clue?  Do I have
> the wrong version for something?  Did I setup the repository wrongly?
>
> 	Thanks --- Jan
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Jan Wielemaker]

On Wednesday 03 October 2007 15:13, Jan Wielemaker wrote:
> $ mkdir /pub/my-repo.git
> $ cd /pub/my-repo.git
> $ git --bare init --shared
> $ git --bare fetch /home/alice/myproject master:master
>
> Checked out freshly using CVS. No problem. But committing a change,
> nothing changed :-( The log output is exactly the same, showing only
> refs/heads/master. I'm starting to suspect git-cvsserver afterall, but
> the docs suggests it is operational for quite a while. Could someone
> give me a clue on what am I missed?

More tests ...  As it didn't like the HEAD, and insisted it only knows
about master, I though what happens on

	cvs -d :ext:user@host:/git-repos.git co master

<works fine>
<edit>
	cvs commit
Commit failed (unknown reason)

:-(  Logfile says:

================================================================
2007-10-03 16:43:37 : INFO  - req_ci : [NULL]
2007-10-03 16:43:37 : INFO  - Lockless commit start, basing commit 
on '/tmp/VP2P
VNHPs0/6t4xncbMoN', index file is '/tmp/VP2PVNHPs0/SO4A6pzpau'
2007-10-03 16:43:37 : INFO  - Start git show-ref -s refs/heads/master
2007-10-03 16:43:37 : INFO  - Heads: 0b7b372d525a4fe7f662996fec9cd11b1038a6be 
re
fs/heads/master

2007-10-03 16:43:37 : INFO  - parenthash = 
0b7b372d525a4fe7f662996fec9cd11b1038a
6be

2007-10-03 16:43:37 : INFO  - Created index '/tmp/VP2PVNHPs0/SO4A6pzpau' with 
fo
r head master - exit status 0
2007-10-03 16:43:37 : INFO  - Committing collections-representation.txt
2007-10-03 16:43:37 : DEBUG - rename /tmp/VP2PVNHPs0/0SfbkMq6AN 
collections-repr
esentation.txt
2007-10-03 16:43:37 : DEBUG - chmod u+rw-x collections-representation.txt
2007-10-03 16:43:37 : INFO  - Updating file 'collections-representation.txt'
2007-10-03 16:43:37 : DEBUG - Treehash : 
aba0f583177b3b7fca05935452de22612164a7f
3, Parenthash : 0b7b372d525a4fe7f662996fec9cd11b1038a6be
2007-10-03 16:43:37 : INFO  - Commit hash :
2007-10-03 16:43:37 : WARN  - Commit failed (Invalid commit hash)
================================================================

!? What happens?  Is git-cvsserver completely broken and should I thus
forget about GIT for now (saying we cannot deal with cvs commit is
politically unacceptable in this project)?  Any clue?

	Please help

		--- Jan

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Johannes Schindelin]

Hi,

On Wed, 3 Oct 2007, Jan Wielemaker wrote:

> 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the current 
> HEAD of module

AFAIR we do not allow committing via pserver protocol.  Might that be your 
problem?

Ciao,
Dscho

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Jan Wielemaker]

Dscho,

On Wednesday 03 October 2007 18:11, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the current
> > HEAD of module
>
> AFAIR we do not allow committing via pserver protocol.  Might that be your
> problem?

Thanks, but no. I'm using CVS over SSH. I've been looking around in
git-cvsserver source a bit and it aborts quite quickly if you try a
commit through pserver. I get a bit further, but it cannot find the HEAD
revision for some reason and (from later message), if I try to checkout
master instead of HEAD it finds the revision but I get a hash mismatch.

I've tried a bit debugging this, but in 15 years CVS experience I never
really needed to debug the protocol and my GIT experience is only 2
weeks old :-( 

My hope is I'm doing something fundamentally wrong and git-cvsserver
just doesn't give a sensible error. I did setup the git repository using
two different routes, one adviced in the CVS conversion manual. GIT
operations work just fine, so does CVS checkout. I don't think you can
to that much wrong with cvs over ssh clients, especially if checkout
works just fine.

Does anyone out there has a working GIT <-> CVS+SHH setup? Based on
what version of GIT? Using what route to create the repository?

	Thanks --- Jan

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Johannes Schindelin]

Hi,

On Wed, 3 Oct 2007, Jan Wielemaker wrote:

> On Wednesday 03 October 2007 18:11, Johannes Schindelin wrote:
> > Hi,
> >
> > On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > > 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the current
> > > HEAD of module
> >
> > AFAIR we do not allow committing via pserver protocol.  Might that be 
> > your problem?
> 
> Thanks, but no. I'm using CVS over SSH. I've been looking around in 
> git-cvsserver source a bit and it aborts quite quickly if you try a 
> commit through pserver. I get a bit further, but it cannot find the HEAD 
> revision for some reason and (from later message), if I try to checkout 
> master instead of HEAD it finds the revision but I get a hash mismatch.

Okay, another stab: is your HEAD detached?

Ciao,
Dscho

Re: git-cvsserver commit trouble (unexpected end of file in client)

[Jan Wielemaker]

Hi Dscho,

Thanks for the comments.  I'll keep in on the list, just in case there is
someone else with a brilliant idea.

On Thursday 04 October 2007 00:14, you wrote:
> Hi,
>
> On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > On Wednesday 03 October 2007 20:55, you wrote:
> > > On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > > > On Wednesday 03 October 2007 18:11, Johannes Schindelin wrote:
> > > > > Hi,
> > > > >
> > > > > On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > > > > > 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the
> > > > > > current HEAD of module
> > > > >
> > > > > AFAIR we do not allow committing via pserver protocol.  Might that
> > > > > be your problem?
> > > >
> > > > Thanks, but no. I'm using CVS over SSH. I've been looking around in
> > > > git-cvsserver source a bit and it aborts quite quickly if you try a
> > > > commit through pserver. I get a bit further, but it cannot find the
> > > > HEAD revision for some reason and (from later message), if I try to
> > > > checkout master instead of HEAD it finds the revision but I get a
> > > > hash mismatch.
> > >
> > > Okay, another stab: is your HEAD detached?
> >
> > I'm a humble git beginner, though I think *my* head is still attached
> > :-) In any case, we are talking a fresh repository and I can perfectly
> > well clone it as well as pull and push from the clone using GIT
> > commands. How do I tell whether the HEAD is detached?
>
> You can tell by looking into .git/HEAD (on the side that runs the server).
> If it is a 40-character hex string, the HEAD is detached.  Otherwise, it
> should contain something like "refs/heads/master".

Its the latter, so my HEAD is still attached. I hope I understand this
correctly, but browsing the docs suggests a detached head is not really
a normal situation, so I'm fine.  Right?

> Other reasons for the failure could be:
>
> 	- your user does not have write access

Definitely ok (also put an strace -o logfile git-cvsserver "$@" script
around it.  No alarming permission or non-existence errors).

> 	- the uid under which git-cvsserver runs has no write access

See above

> 	- you found an error that only triggers with your repo

Great!  Its so damn simple and and tried with three repos created
in three different ways, that I'm either extremely unlucky or many
more should be faced with this or nobody uses git-cvsserver.

I'm hoping for a command-by-command sequence that gets me a definitely
fine repository, so at least I can see it running correctly once.  Then
maybe I can analyse traces in detail to see where they differ and what
is wrong.  Somebody?

	Thanks --- Jan

Re: git-cvsserver commit trouble BUG+Work-around

[Jan Wielemaker]

>> Other reasons for the failure could be:
>>
>> 	- your user does not have write access
>
> Definitely ok (also put an strace -o logfile git-cvsserver "$@" script
> around it.  No alarming permission or non-existence errors).
>
>> 	- the uid under which git-cvsserver runs has no write access
>
> See above
>
>> 	- you found an error that only triggers with your repo
>
> Great!  Its so damn simple and and tried with three repos created
> in three different ways, that I'm either extremely unlucky or many
> more should be faced with this or nobody uses git-cvsserver.

Finally getting at what is either a bug in git-show-ref or
git-cvsserver.  I retried the whole lot using git-1.5.3.4:

<create initial repository>
	% Created /home/jan/tmp/test/, added a file README with some text
	% git-init
	% git-add .
	% git-commit

<create bare one for sharing>
	% cd /home/git
	% mkdir test.git
	% cd test.git
	% git --bare init --shared
	% git --bare fetch /home/jan/tmp/test master:master
	% git-config gitcvs.enabled 1
	% git-config gitcvs.logfile `pwd`/cvs.log
	% touch cvs.log
	% chmod g+w cvs.log
	% chgrp -R eculture .

<checkout>

	% export CVS_SERVER=git-cvsserver
	% cvs -d:ext:jan@localhost:/home/git/test.git co HEAD
	% cd HEAD
	% emacs README
	% cvs commit -m test
	cvs [commit aborted]: end of file from server (consult above messages if
any)

Same user, so we should not have permission rights. Indeed, the trouble
is here: git-cvsserver, near line 1203:

    my $parenthash = `git show-ref -s refs/heads/$state->{module}`;

$state->{module} is -of course- HEAD. git show-ref -s  refs/heads/HEAD
indeed gives no output. git show-ref -s refs/heads/master works just
fine.  HEAD in the repo says:

gollem (test.git) 2_> cat HEAD
ref: refs/heads/master

I guess this link must be picked up by either git-cvsserver or
git-show-ref. Right? Anyway, if I add the line marked + to
git-cvsserver, cvs commit and cvs add operations appear to work (I
assume only if I work on the HEAD).

    $ENV{GIT_INDEX_FILE} = $file_index;

+   $state->{module} = "master";

    # Remember where the head was at the beginning.

Thanks for the comments. Really worrying is that such a big flaw can be
in stable git and remain their for at least 3 patch levels unnoticed.

Will someone take care of this and fix it properly in the right place
instead of this dirty hack?

	Thanks --- Jan

Re: git-cvsserver commit trouble BUG+Work-around

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Jan Wielemaker wrote:

> Indeed, the trouble is here: git-cvsserver, near line 1203:
> 
>     my $parenthash = `git show-ref -s refs/heads/$state->{module}`;
> 
> $state->{module} is -of course- HEAD. git show-ref -s refs/heads/HEAD 
> indeed gives no output. git show-ref -s refs/heads/master works just 
> fine.

Ah!  Did you do "CVSROOT=:ext:blablub cvs co HEAD"?

You should "co master".  The branches in git are the modules in cvs.

HEAD is too volatile, you cannot make a proper module from it (imagine for 
example "git checkout next" where "next" is a branch, followed by "git 
checkout html", where "html" is another branch).

Ciao,
Dscho

Re: git-cvsserver commit trouble BUG+Work-around

[Jan Wielemaker]

Hi Dscho,

On Thursday 04 October 2007 14:56, Johannes Schindelin wrote:
> On Thu, 4 Oct 2007, Jan Wielemaker wrote:
> > Indeed, the trouble is here: git-cvsserver, near line 1203:
> >
> >     my $parenthash = `git show-ref -s refs/heads/$state->{module}`;
> >
> > $state->{module} is -of course- HEAD. git show-ref -s refs/heads/HEAD
> > indeed gives no output. git show-ref -s refs/heads/master works just
> > fine.
>
> Ah!  Did you do "CVSROOT=:ext:blablub cvs co HEAD"?

Yip ...

> You should "co master".  The branches in git are the modules in cvs.
>
> HEAD is too volatile, you cannot make a proper module from it (imagine for
> example "git checkout next" where "next" is a branch, followed by "git
> checkout html", where "html" is another branch).

Ok.  Pretty sure I got literal HEAD from one of the examples somewhere ...
Or, I've been blind all along.  Anyway, the maintainer may consider giving
an error when trying to access HEAD as a module.  Would have saved about
2 days work and its unlikely I'm the last victim :-(

Part of the two days is that during all the things I tried somehow
messed up with master too, so the one time I tried that it failed as
well :-(

Anyway, case closed.  On with the testing ...

	Thanks --- Jan

[PATCH/RFT] cvsserver: only allow checkout of branches

[Johannes Schindelin]

It does not make sense to check out tags or "HEAD".

Noticed by Jan Wielemaker.

---

	On Thu, 4 Oct 2007, Jan Wielemaker wrote:

	> On Thursday 04 October 2007 14:56, Johannes Schindelin wrote:
	>
	> > Ah!  Did you do "CVSROOT=:ext:blablub cvs co HEAD"?
	> 
	> Yip ...
	> 
	> > You should "co master".  The branches in git are the modules 
	> > in cvs.
	> >
	> > HEAD is too volatile, you cannot make a proper module from it 
	> > (imagine for example "git checkout next" where "next" is a 
	> > branch, followed by "git checkout html", where "html" is 
	> > another branch).
	> 
	> Ok.  Pretty sure I got literal HEAD from one of the examples 
	> somewhere ... Or, I've been blind all along.  Anyway, the 
	> maintainer may consider giving an error when trying to access 
	> HEAD as a module.  Would have saved about 2 days work and its 
	> unlikely I'm the last victim :-(

	This patch is completely untested.  Could you try it, please?

	If it works:
	Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

	Cc'ed Frank, who is de-facto maintainer (according to shortlog) 
	and Martin, who started it all IIRC.

 git-cvsserver.perl |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 13dbd27..869690c 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -770,6 +770,14 @@ sub req_co
 
     $log->debug("req_co : " . ( defined($data) ? $data : "[NULL]" ) );
 
+    if( system("git", "rev-parse", "--verify", "refs/heads/$module" ) != 0 )
+    {
+	$log->warn("Checkout failed: $module is not a branch");
+	print "error 1 Checkout failed: $module is not a branch\n";
+	chdir "/";
+	exit;
+    }
+
     $log->info("Checking out module '$module' ($state->{CVSROOT}) to '$checkout_path'");
 
     $ENV{GIT_DIR} = $state->{CVSROOT} . "/";

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Frank Lichtenheld]

On Thu, Oct 04, 2007 at 04:29:04PM +0100, Johannes Schindelin wrote:
> 	Cc'ed Frank, who is de-facto maintainer (according to shortlog) 
> 	and Martin, who started it all IIRC.
> 
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index 13dbd27..869690c 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -770,6 +770,14 @@ sub req_co
>  
>      $log->debug("req_co : " . ( defined($data) ? $data : "[NULL]" ) );
>  
> +    if( system("git", "rev-parse", "--verify", "refs/heads/$module" ) != 0 )
> +    {
> +	$log->warn("Checkout failed: $module is not a branch");
> +	print "error 1 Checkout failed: $module is not a branch\n";
> +	chdir "/";
> +	exit;
> +    }
> +
>      $log->info("Checking out module '$module' ($state->{CVSROOT}) to '$checkout_path'");
>  
>      $ENV{GIT_DIR} = $state->{CVSROOT} . "/";

Looks good, haven't tested it yet, though. Care to add a testcase
or should I take care of that?

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Frank Lichtenheld wrote:

> On Thu, Oct 04, 2007 at 04:29:04PM +0100, Johannes Schindelin wrote:
> > 	Cc'ed Frank, who is de-facto maintainer (according to shortlog) 
> > 	and Martin, who started it all IIRC.
> > 
> > diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> > index 13dbd27..869690c 100755
> > --- a/git-cvsserver.perl
> > +++ b/git-cvsserver.perl
> > @@ -770,6 +770,14 @@ sub req_co
> >  
> >      $log->debug("req_co : " . ( defined($data) ? $data : "[NULL]" ) );
> >  
> > +    if( system("git", "rev-parse", "--verify", "refs/heads/$module" ) != 0 )
> > +    {
> > +	$log->warn("Checkout failed: $module is not a branch");
> > +	print "error 1 Checkout failed: $module is not a branch\n";
> > +	chdir "/";
> > +	exit;
> > +    }
> > +
> >      $log->info("Checking out module '$module' ($state->{CVSROOT}) to '$checkout_path'");
> >  
> >      $ENV{GIT_DIR} = $state->{CVSROOT} . "/";
> 
> Looks good, haven't tested it yet, though. Care to add a testcase
> or should I take care of that?

I would be eternally thankful if you could add the test case, since I am 
really short on time.

Ciao,
Dscho

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Jan Wielemaker]

On Thursday 04 October 2007 17:29, Johannes Schindelin wrote:
> It does not make sense to check out tags or "HEAD".
>
> Noticed by Jan Wielemaker.

Thanks (though it won't bite me a second time :-)

> ) +    {
> +	$log->warn("Checkout failed: $module is not a branch");
> +	print "error 1 Checkout failed: $module is not a branch\n";

There are various places where it says print "...", one that even
started some suggestion on what might be wrong, but these things don't
end up at a visible place when using ssh as transport. Its a good idea
this one is at least in the logfile as well. Not sure, but might
printing to stderr work better?

	Cheers --- Jan

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Jan Wielemaker wrote:

> On Thursday 04 October 2007 17:29, Johannes Schindelin wrote:
> > It does not make sense to check out tags or "HEAD".
> >
> > Noticed by Jan Wielemaker.
> 
> Thanks (though it won't bite me a second time :-)

Heh.  But this is Open Source, so we can make it better for others, too.  
Thank you for spending so much time on this, and actually uncovering that 
error in cvsserver!

> > ) +    {
> > +	$log->warn("Checkout failed: $module is not a branch");
> > +	print "error 1 Checkout failed: $module is not a branch\n";
> 
> There are various places where it says print "...", one that even
> started some suggestion on what might be wrong, but these things don't
> end up at a visible place when using ssh as transport. Its a good idea
> this one is at least in the logfile as well. Not sure, but might
> printing to stderr work better?

I think not.  AFAICT stderr is not visible with cvs either.  I somewhat 
hoped that the "error 1" was an indicator to the cvs client to show the 
message, but that's way beyond my knowledge...

Ciao,
Dscho

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Frank Lichtenheld]

On Thu, Oct 04, 2007 at 07:06:20PM +0200, Jan Wielemaker wrote:
> On Thursday 04 October 2007 17:29, Johannes Schindelin wrote:
> > ) +    {
> > +	$log->warn("Checkout failed: $module is not a branch");
> > +	print "error 1 Checkout failed: $module is not a branch\n";
> 
> There are various places where it says print "...", one that even
> started some suggestion on what might be wrong, but these things don't
> end up at a visible place when using ssh as transport. Its a good idea

huh?

$ CVS_SERVER=/home/djpig/devel/git/git-cvsserver cvs -d :ext:localhost:/home/djpig/devel/gitcvs.git/ co -d cvswork HEAD 
djpig@localhost's password: 
server doesn't support gzip-file-contents
fatal: Needed a single revision
Checkout failed: HEAD is not a branch

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Frank Lichtenheld]

On Thu, Oct 04, 2007 at 04:29:04PM +0100, Johannes Schindelin wrote:
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index 13dbd27..869690c 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -770,6 +770,14 @@ sub req_co
>  
>      $log->debug("req_co : " . ( defined($data) ? $data : "[NULL]" ) );
>  
> +    if( system("git", "rev-parse", "--verify", "refs/heads/$module" ) != 0 )
> +    {
> +	$log->warn("Checkout failed: $module is not a branch");
> +	print "error 1 Checkout failed: $module is not a branch\n";
> +	chdir "/";
> +	exit;
> +    }
> +
>      $log->info("Checking out module '$module' ($state->{CVSROOT}) to '$checkout_path'");
>  
>      $ENV{GIT_DIR} = $state->{CVSROOT} . "/";

I've found a problem with the patch. Since git-rev-parse will produce
output (which will end up at the client and confuse it), we will have to use a fork/exec
combination instead of system to be able to close STDOUT and STDERR of the child.

Should I sent a patch on top of this one or as a replacement?

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Frank Lichtenheld wrote:

> On Thu, Oct 04, 2007 at 04:29:04PM +0100, Johannes Schindelin wrote:
> > diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> > index 13dbd27..869690c 100755
> > --- a/git-cvsserver.perl
> > +++ b/git-cvsserver.perl
> > @@ -770,6 +770,14 @@ sub req_co
> >  
> >      $log->debug("req_co : " . ( defined($data) ? $data : "[NULL]" ) );
> >  
> > +    if( system("git", "rev-parse", "--verify", "refs/heads/$module" ) != 0 )
> > +    {
> > +	$log->warn("Checkout failed: $module is not a branch");
> > +	print "error 1 Checkout failed: $module is not a branch\n";
> > +	chdir "/";
> > +	exit;
> > +    }
> > +
> >      $log->info("Checking out module '$module' ($state->{CVSROOT}) to '$checkout_path'");
> >  
> >      $ENV{GIT_DIR} = $state->{CVSROOT} . "/";
> 
> I've found a problem with the patch. Since git-rev-parse will produce
> output (which will end up at the client and confuse it), we will have to use a fork/exec
> combination instead of system to be able to close STDOUT and STDERR of the child.
> 
> Should I sent a patch on top of this one or as a replacement?

Better a replacement.  Otherwise I have to fly to California with a brown 
paper bag over my HEAD.

Ciao,
Dscho

Re: [PATCH/RFT] cvsserver: only allow checkout of branches

[Shawn O. Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Thu, 4 Oct 2007, Frank Lichtenheld wrote:
> > 
> > Should I sent a patch on top of this one or as a replacement?
> 
> Better a replacement.  Otherwise I have to fly to California with a brown 
> paper bag over my HEAD.

That may be the safer way to travel.  If your head is in a bag you
have no idea what sort of wait in the security line you are about
to face.  Or what horrible movie they are showing you on the flight
both over *and* back.

-- 
Shawn.