[PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Scott Collins]

Currently, git-cvsserver requires CVS_SERVER to be set to 'cvs' if you are
restricted to git-shell, so we need to mention that here.  Previous wording
mentioning GIT_AUTHOR, GIT_COMMITTER variables may not have made clear we
really meant GIT_AUTHOR_(NAME|COMMITTER), etc.  Add a bit of text to
differentiate cvs -d (setting CVSROOT) from cvs co -d (setting the name of
the newly checked out directory).

Signed-off-by: Scott Collins <scc@ScottCollins.net>
---
 Documentation/git-cvsserver.txt |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 9cec802..5e67a89 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -106,7 +106,10 @@ Note: Newer CVS versions (>= 1.12.11) also support specifying
 CVS_SERVER directly in CVSROOT like
 
 ------
-cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name>
+cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
+
+# or for git-shell users:
+cvs -d ":ext;CVS_SERVER=cvs:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
 ------
 This has the advantage that it will be saved in your 'CVS/Root' files and
 you don't need to worry about always setting the correct environment
@@ -146,7 +149,8 @@ allowing access over SSH.
    appropriate git repo. For example:
 +
 --
-For SSH access, CVS_SERVER should be set to git-cvsserver
+For normal SSH access, CVS_SERVER should be set to git-cvsserver.  For those
+whose access is restricted to git-shell, CVS_SERVER should be set to cvs.
 
 Example:
 
@@ -155,14 +159,17 @@ Example:
      export CVS_SERVER=git-cvsserver
 ------
 --
-4. For SSH clients that will make commits, make sure their .bashrc file
-   sets the GIT_AUTHOR and GIT_COMMITTER variables.
+4. For SSH clients that will make commits, make sure their (server-side) .bashrc
+   files export GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, and
+   GIT_COMMITTER_EMAIL.
 
 5. Clients should now be able to check out the project. Use the CVS 'module'
-   name to indicate what GIT 'head' you want to check out. Example:
+   name to indicate what GIT 'head' you want to check out.  This also sets the
+   name of your newly checked-out directory, unless you tell it otherwise with
+   `-d <dir_name>` Example:
 +
 ------
-     cvs co -d project-master master
+     cvs co master -d project-master
 ------
 
 [[dbbackend]]
-- 
1.5.5.49.gf43e2

Re: [PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Junio C Hamano]

Scott Collins <scc@ScottCollins.net> writes:

> Currently, git-cvsserver requires CVS_SERVER to be set to 'cvs' if you are
> restricted to git-shell, so we need to mention that here.  Previous wording
> mentioning GIT_AUTHOR, GIT_COMMITTER variables may not have made clear we
> really meant GIT_AUTHOR_(NAME|COMMITTER), etc.  Add a bit of text to
> differentiate cvs -d (setting CVSROOT) from cvs co -d (setting the name of
> the newly checked out directory).
>
> Signed-off-by: Scott Collins <scc@ScottCollins.net>
> ---
>  Documentation/git-cvsserver.txt |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)

Thanks; please make it a habit to give a final round of proofreading
before you send your messages out.  For example, I do not think we meant
"GIT_AUTHOR_(NAME|COMMITTER)" ;-)

> diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
> index 9cec802..5e67a89 100644
> --- a/Documentation/git-cvsserver.txt
> +++ b/Documentation/git-cvsserver.txt
> @@ -106,7 +106,10 @@ Note: Newer CVS versions (>= 1.12.11) also support specifying
>  CVS_SERVER directly in CVSROOT like
>  
>  ------
> -cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name>
> +cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
> +
> +# or for git-shell users:
> +cvs -d ":ext;CVS_SERVER=cvs:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
>  ------
>  This has the advantage that it will be saved in your 'CVS/Root' files and
>  you don't need to worry about always setting the correct environment
> @@ -146,7 +149,8 @@ allowing access over SSH.
>     appropriate git repo. For example:
>  +
>  --
> -For SSH access, CVS_SERVER should be set to git-cvsserver
> +For normal SSH access, CVS_SERVER should be set to git-cvsserver.  For those
> +whose access is restricted to git-shell, CVS_SERVER should be set to cvs.
>  
>  Example:

Hmm.  Can't this be further improved?

My undestanding of the reasoning behind 0c696fe (Support cvs via
git-shell, 2007-10-09) is by doing this people do not have to say
CVS_SERVER and do this instead:

	cvs -d :ext:[user@]cvs.example.com:/path/to/repo checkout foo

which would truly hide the fact that the server end is running something
funky, because the command line exactly matches how you would access the
real CVS repository over ssh.

Dscho, care to comment?  This commit happened while I was away so I do not
really _recall_ the discussion that led to it, but I have always thought
that was the intention.

> @@ -155,14 +159,17 @@ Example:
>       export CVS_SERVER=git-cvsserver
>  ------
>  --
> -4. For SSH clients that will make commits, make sure their .bashrc file
> -   sets the GIT_AUTHOR and GIT_COMMITTER variables.
> +4. For SSH clients that will make commits, make sure their (server-side) .bashrc
> +   files export GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, and
> +   GIT_COMMITTER_EMAIL.
>  
>  5. Clients should now be able to check out the project. Use the CVS 'module'
> -   name to indicate what GIT 'head' you want to check out. Example:
> +   name to indicate what GIT 'head' you want to check out.  This also sets the
> +   name of your newly checked-out directory, unless you tell it otherwise with
> +   `-d <dir_name>` Example:
>  +
>  ------
> -     cvs co -d project-master master
> +     cvs co master -d project-master
>  ------

Are you sure you wanted to do this?  In any case this is different from
what I read you changed in your commit log message, where you clarified
distinction between d1 and d2 in "cvs -d d1 co -d d2 module".  Instead
what you did here is to change "cvs co -d d module" (which looks more
natural) to "cvs co module -d d" (which might work if the options are
reordered across parameters, but is unusual).

Re: [PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Scott Collins]

On Tue, Apr 15, 2008 at 2:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
>  Thanks; please make it a habit to give a final round of proofreading
>  before you send your messages out.  For example, I do not think we meant
>  "GIT_AUTHOR_(NAME|COMMITTER)" ;-)

Yikes!  Correct.

>  Hmm.  Can't this be further improved?
>
>  My undestanding of the reasoning behind 0c696fe (Support cvs via
>  git-shell, 2007-10-09) is by doing this people do not have to say
>  CVS_SERVER and do this instead:
>
>         cvs -d :ext:[user@]cvs.example.com:/path/to/repo checkout foo
>
>  which would truly hide the fact that the server end is running something
>  funky, because the command line exactly matches how you would access the
>  real CVS repository over ssh.

If that's the case, then I should mention instead that users
restricted to git-shell should _not_ set or change CVS_SERVER, and
remove the extra examples.

>  Dscho, care to comment?  This commit happened while I was away so I do not
>  really _recall_ the discussion that led to it, but I have always thought
>  that was the intention.

I actually was very tempted to modify git-shell to understand
git-cvsserver; but decided to submit a documentation patch first,
hoping from the discussion I would learn if that was the right path.


>  >  5. Clients should now be able to check out the project. Use the CVS 'module'
>  > -   name to indicate what GIT 'head' you want to check out. Example:
>  > +   name to indicate what GIT 'head' you want to check out.  This also sets the
>  > +   name of your newly checked-out directory, unless you tell it otherwise with
>  > +   `-d <dir_name>` Example:
>  >  +
>  >  ------
>  > -     cvs co -d project-master master
>  > +     cvs co master -d project-master
>  >  ------
>
>  Are you sure you wanted to do this?  In any case this is different from
>  what I read you changed in your commit log message, where you clarified
>  distinction between d1 and d2 in "cvs -d d1 co -d d2 module".  Instead
>  what you did here is to change "cvs co -d d module" (which looks more
>  natural) to "cvs co module -d d" (which might work if the options are
>  reordered across parameters, but is unusual).

Hmmm.  My intent was to make it read more naturally, which for me is
"checkout this module, put it into this directory".  At least that's
how I've always said it to CVS.  Warning readers that there are two
different meanings for -d, and they probably want them both in the
same command is more important to me than moving the -d to the end.  A
better fix to that example would be something along the lines of:

  cvs -d <CVSROOT-as-above> co -d <dir-name> <head-name>

I didn't do that, though, because it looked like this example was
intended to be concrete.

I'll update my patch and re-submit after I test not setting
CVS_SERVER, and feel I have the right replacement example, if any, for
the text just above.
__________
Scott Collins

[PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Scott Collins]

Currently, for SSH clients restricted to git-shell, CVS_SERVER should
not be changed: git-shell understands the default value of 'cvs' to mean
git-cvsserver and actually _doesn't_ understand the command
'git-cvsserver'. This makes it totally transparent to CVS users, but the
original wording here perhaps less so to the person setting-up CVS
access.  Previous wording mentioning GIT_AUTHOR, GIT_COMMITTER variables
may not have made clear we really meant GIT_AUTHOR_(NAME|EMAIL), etc.
Add a bit of text to differentiate cvs -d (setting CVSROOT) from cvs co
-d (setting the name of the newly checked out directory).  Removed an
extra 'Example:' string.

Signed-off-by: Scott Collins <scc@ScottCollins.net>
---
 Documentation/git-cvsserver.txt |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 9cec802..44734e8 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -106,11 +106,13 @@ Note: Newer CVS versions (>= 1.12.11) also support specifying
 CVS_SERVER directly in CVSROOT like
 
 ------
-cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name>
+cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
 ------
 This has the advantage that it will be saved in your 'CVS/Root' files and
 you don't need to worry about always setting the correct environment
-variable.
+variable.  SSH users restricted to git-shell don't need to override the default
+with CVS_SERVER (and probably shouldn't) as git-shell understands 'cvs' to mean
+git-cvsserver.
 --
 2. For each repo that you want accessible from CVS you need to edit config in
    the repo and add the following section.
@@ -141,25 +143,26 @@ allowing access over SSH.
         enabled=1
 ------
 --
-3. On the client machine you need to set the following variables.
-   CVSROOT should be set as per normal, but the directory should point at the
-   appropriate git repo. For example:
+3. If you didn't specify the CVSROOT/CVS_SERVER directly in the checkout command,
+   automatically saving it in your 'CVS/Root' files, then you need to set them
+   explicitly in your environment.  CVSROOT should be set as per normal, but the
+   directory should point at the appropriate git repo.  As above, for SSH clients
+   _not_ restricted to git-shell, CVS_SERVER should be set to git-cvsserver.
 +
 --
-For SSH access, CVS_SERVER should be set to git-cvsserver
-
-Example:
-
 ------
      export CVSROOT=:ext:user@server:/var/git/project.git
      export CVS_SERVER=git-cvsserver
 ------
 --
-4. For SSH clients that will make commits, make sure their .bashrc file
-   sets the GIT_AUTHOR and GIT_COMMITTER variables.
+4. For SSH clients that will make commits, make sure their (server-side) .bashrc
+   files export GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, and
+   GIT_COMMITTER_EMAIL.
 
 5. Clients should now be able to check out the project. Use the CVS 'module'
-   name to indicate what GIT 'head' you want to check out. Example:
+   name to indicate what GIT 'head' you want to check out.  This also sets the
+   name of your newly checked-out directory, unless you tell it otherwise with
+   `-d <dir_name>`.  Example:
 +
 ------
      cvs co -d project-master master
-- 
1.5.5.49.gf43e2

Re: [PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Johannes Schindelin]

Hi,

On Tue, 15 Apr 2008, Scott Collins wrote:

> Currently, for SSH clients restricted to git-shell, CVS_SERVER should
> not be changed: git-shell understands the default value of 'cvs' to mean
> git-cvsserver and actually _doesn't_ understand the command
> 'git-cvsserver'.

How about a new paragraph here?

> This makes it totally transparent to CVS users, but the
> original wording here perhaps less so to the person setting-up CVS
> access.  Previous wording mentioning GIT_AUTHOR, GIT_COMMITTER variables
> may not have made clear we really meant GIT_AUTHOR_(NAME|EMAIL), etc.

Likewise here.

> Add a bit of text to differentiate cvs -d (setting CVSROOT) from cvs co
> -d (setting the name of the newly checked out directory).  Removed an
> extra 'Example:' string.
> 
> Signed-off-by: Scott Collins <scc@ScottCollins.net>
> ---
>  Documentation/git-cvsserver.txt |   27 +++++++++++++++------------
>  1 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
> index 9cec802..44734e8 100644
> --- a/Documentation/git-cvsserver.txt
> +++ b/Documentation/git-cvsserver.txt
> @@ -106,11 +106,13 @@ Note: Newer CVS versions (>= 1.12.11) also support specifying
>  CVS_SERVER directly in CVSROOT like
>  
>  ------
> -cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name>
> +cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name> -d <dir_name>

I am not really sure.  Normal usage AFAIAC is without -d.

>  ------
>  This has the advantage that it will be saved in your 'CVS/Root' files and
>  you don't need to worry about always setting the correct environment
> -variable.
> +variable.  SSH users restricted to git-shell don't need to override the default
> +with CVS_SERVER (and probably shouldn't) as git-shell understands 'cvs' to mean
> +git-cvsserver.

Please make these lines shorter.

> @@ -141,25 +143,26 @@ allowing access over SSH.
>          enabled=1
>  ------
>  --
> -3. On the client machine you need to set the following variables.
> -   CVSROOT should be set as per normal, but the directory should point at the
> -   appropriate git repo. For example:
> +3. If you didn't specify the CVSROOT/CVS_SERVER directly in the checkout command,
> +   automatically saving it in your 'CVS/Root' files, then you need to set them
> +   explicitly in your environment.  CVSROOT should be set as per normal, but the
> +   directory should point at the appropriate git repo.  As above, for SSH clients
> +   _not_ restricted to git-shell, CVS_SERVER should be set to git-cvsserver.

Please make the lines shorter.

Also, I seem to recall that "didn't" is considered colloquial speech, 
while you should write "did not" in written documentation.

Another thing: "CVSROOT should be set as per normal" strikes me as a 
little short on concrete, helpful details.

Thanks, Dscho

[PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Scott Collins]

Currently, for SSH clients restricted to git-shell, CVS_SERVER should
not be changed: git-shell understands the default value of 'cvs' to mean
git-cvsserver and actually _doesn't_ understand the command
'git-cvsserver'. This makes it totally transparent to CVS users, but the
original wording here perhaps less so to the person setting-up CVS
access.  Previous wording mentioning GIT_AUTHOR, GIT_COMMITTER variables
may not have made clear we really meant GIT_AUTHOR_(NAME|EMAIL), etc.
Note that .ssh/environment is a good place to set these, and that
.bashrc is shell-specific. Add a bit of text to differentiate cvs -d
(setting CVSROOT) from cvs co -d (setting the name of the newly checked
out directory).  Removed an extra 'Example:' string.

Signed-off-by: Scott Collins <scc@ScottCollins.net>
---
...because of course I forget to mention .ssh/environment (required for
git-shell clients).  Sorry for the patch noise.


 Documentation/git-cvsserver.txt |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 9cec802..bd98fc1 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -106,11 +106,13 @@ Note: Newer CVS versions (>= 1.12.11) also support specifying
 CVS_SERVER directly in CVSROOT like
 
 ------
-cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name>
+cvs -d ":ext;CVS_SERVER=git-cvsserver:user@server/path/repo.git" co <HEAD_name> -d <dir_name>
 ------
 This has the advantage that it will be saved in your 'CVS/Root' files and
 you don't need to worry about always setting the correct environment
-variable.
+variable.  SSH users restricted to git-shell don't need to override the default
+with CVS_SERVER (and probably shouldn't) as git-shell understands 'cvs' to mean
+git-cvsserver.
 --
 2. For each repo that you want accessible from CVS you need to edit config in
    the repo and add the following section.
@@ -141,25 +143,28 @@ allowing access over SSH.
         enabled=1
 ------
 --
-3. On the client machine you need to set the following variables.
-   CVSROOT should be set as per normal, but the directory should point at the
-   appropriate git repo. For example:
+3. If you didn't specify the CVSROOT/CVS_SERVER directly in the checkout command,
+   automatically saving it in your 'CVS/Root' files, then you need to set them
+   explicitly in your environment.  CVSROOT should be set as per normal, but the
+   directory should point at the appropriate git repo.  As above, for SSH clients
+   _not_ restricted to git-shell, CVS_SERVER should be set to git-cvsserver.
 +
 --
-For SSH access, CVS_SERVER should be set to git-cvsserver
-
-Example:
-
 ------
      export CVSROOT=:ext:user@server:/var/git/project.git
      export CVS_SERVER=git-cvsserver
 ------
 --
-4. For SSH clients that will make commits, make sure their .bashrc file
-   sets the GIT_AUTHOR and GIT_COMMITTER variables.
+4. For SSH clients that will make commits, make sure their server-side
+   .ssh/environment files (or .bashrc, etc., according to their specific shell)
+   export appropriate values for GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
+   GIT_COMMITTER_NAME, and GIT_COMMITTER_EMAIL.  For SSH clients connecting to a
+   bash shell, .bashrc is a reasonable alternative.
 
 5. Clients should now be able to check out the project. Use the CVS 'module'
-   name to indicate what GIT 'head' you want to check out. Example:
+   name to indicate what GIT 'head' you want to check out.  This also sets the
+   name of your newly checked-out directory, unless you tell it otherwise with
+   `-d <dir_name>`.  Example:
 +
 ------
      cvs co -d project-master master
-- 
1.5.5.49.gf43e2

Re: [PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Junio C Hamano]

Scott Collins <scc@ScottCollins.net> writes:

> For SSH clients restricted to git-shell, CVS_SERVER does not have to be
> specified, because git-shell understands the default value of 'cvs' to
> mean git-cvsserver'. This makes it totally transparent to CVS users, but
> the instruction to set up CVS access for people with real shell access
> does not apply.

Thanks.  I was only _guessing_ that the reasoning behind the "cvs" hack in
git-shell was to allow a vanilla "cvs over ssh" to work transparently
without trying, but it appears to work exactly that way.

>
> Note that the .ssh/environment file is a good place to set these, and that
> the .bashrc is shell-specific. Add a bit of text to differentiate cvs -d
> (setting CVSROOT) from cvs co -d (setting the name of the newly checked
> out directory).  Removed an extra 'Example:' string.

Suggesting .ssh/environment as an alternative is a good idea, but not
everybody's sshd pays attention to it, so the user still needs to learn
how to drive his or her ssh environment.  But this document is not a
troubleshooting guide for ssh, so let's not go there too much.  I think
your description is fine.

Re: [PATCH] Clarify documentation of git-cvsserver, particularly in relation to git-shell

[Johannes Schindelin]

Hi,

On Tue, 15 Apr 2008, Junio C Hamano wrote:

> Dscho, care to comment?  This commit happened while I was away so I do 
> not really _recall_ the discussion that led to it, but I have always 
> thought that was the intention.

Sorry, was out of the loop for a few days.  Private life and moving to 
another country will do that to you.

I commented on the new version of the patch.  AFAIR I tried to get 
"git-cvsserver" allowed in git-shell, too, but that was shot down.  I am 
not sure if we should include that again, for ease of use, though.

Ciao,
Dscho