Re: update: patch: relative path bugs in cvs (client/server mode only)

[Chris Bohn]

regenerated against the head, attached to issue 81, and pasted below 
(additionally, I fixed a hard-coded path in sanity.sh to be ${CVSROOT_DIRNAME}):

Note that while I tested these changes under Windows, there is currently no
regression testing framework for it, but the changes are regression tested
under UNIX (though one change is specific to Windows).

NEWS entry:
* using relative paths now works under client/server mode

Log entry:
2004-03-30        Chris Bohn  <cbohn@rrinc.com>

  * client.c, client.h (send_files, send_file_names, send_max_dotdot):
  I pulled out the code for calculating and sending max-dotdot to the
  server into a new function that is now called from both send_files
  and send_file_names.  Max-dotdot was previously only sent from
  send_file_names.  That was fine because send_file_names was called
  before send_files for a while, but when multiple repository support
  was added, the send_files and send_file_names calls were swapped, which
  led to Max-dotdot not getting sent at the right time.  That led to
  relative path operations typically failing.  This change leaves
  send_file_names sending Max-dotdot and adds it being sent with send_files.
  This fixes the relative path problems described in issue 81.  It may be ok
  to remove sending Max-dotdot from send_file_names, but since I'm sure all
  possible variations likely aren't covered by the regression tests, I thought
  it would be safer to leave it, and it causes no harm (other than a little
  extra traffic).

  *sanity.sh (files, status): I modified files-14 to be a successful
  test instead of a failure now that relative paths are supported, and I added
  a few extra tests to status to test more relative path command lines.


[D:\temp\ccvs\src]cvs diff -u client.h
Index: client.h
===================================================================
RCS file: /cvs/ccvs/src/client.h,v
retrieving revision 1.48
diff -u -r1.48 client.h
--- client.h    22 Mar 2004 15:37:34 -0000      1.48
+++ client.h    1 Apr 2004 18:30:02 -0000
@@ -83,6 +83,10 @@
 void
 start_server (void);

+/* Calculate and send max-dotdot to the server */
+void
+send_max_dotdot(int argc, char **argv);
+
 /* Send the names of all the argument files to the server.  */
 void
 send_file_names (int argc, char **argv, unsigned int flags);


[D:\temp\ccvs\src]cvs diff -u client.c
Index: client.c
===================================================================
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.359
diff -u -r1.359 client.c
--- client.c  26 Mar 2004 02:17:04 -0000  1.359
+++ client.c  1 Apr 2004 18:35:28 -0000
@@ -4236,6 +4236,45 @@
     free (copy);
 }

+/* Calculate and send max-dotdot to the server */
+void
+send_max_dotdot(int argc, char **argv)
+{
+    int i;
+  int level = 0;
+    int max_level = 0;
+
+    /* Send Max-dotdot if needed.  */
+    for (i = 0; i < argc; ++i)
+    {
+    level = pathname_levels (argv[i]);
+    if (level > max_level)
+      max_level = level;
+    }
+
+    if (max_level > 0)
+    {
+    if (supported_request ("Max-dotdot"))
+    {
+      char buf[10];
+      sprintf (buf, "%d", max_level);
+
+      send_to_server ("Max-dotdot ", 0);
+      send_to_server (buf, 0);
+      send_to_server ("\012", 1);
+    }
+    else
+    {
+      /*
+       * "leading .." is not strictly correct, as this also includes
+       * cases like "foo/../..".  But trying to explain that in the
+       * error message would probably just confuse users.
+       */
+      error (1, 0,
+         "leading .. not supported by old (pre-Max-dotdot) servers");
+    }
+    }
+} /* end calculate */

 /* Send the names of all the argument files to the server.  */

@@ -4243,42 +4282,13 @@
 send_file_names( int argc, char **argv, unsigned int flags )
 {
     int i;
-    int level;
-    int max_level;

     /* The fact that we do this here as well as start_recursion is a bit
        of a performance hit.  Perhaps worth cleaning up someday.  */
     if (flags & SEND_EXPAND_WILD)
  expand_wild (argc, argv, &argc, &argv);

-    /* Send Max-dotdot if needed.  */
-    max_level = 0;
-    for (i = 0; i < argc; ++i)
-    {
- level = pathname_levels (argv[i]);
- if (level > max_level)
-     max_level = level;
-    }
-    if (max_level > 0)
-    {
- if (supported_request ("Max-dotdot"))
- {
-            char buf[10];
-            sprintf (buf, "%d", max_level);
-
-     send_to_server ("Max-dotdot ", 0);
-     send_to_server (buf, 0);
-     send_to_server ("\012", 1);
- }
- else
-     /*
-      * "leading .." is not strictly correct, as this also includes
-      * cases like "foo/../..".  But trying to explain that in the
-      * error message would probably just confuse users.
-      */
-     error (1, 0,
-      "leading .. not supported by old (pre-Max-dotdot) servers");
-    }
+    send_max_dotdot(argc, argv);

     for (i = 0; i < argc; ++i)
     {
@@ -4432,6 +4442,13 @@
 {
     struct send_data args;
     int err;
+
+
+    /* send max-dotdot here too (also in send_file_names) to avoid relative
+     * path issues, like those described in issue 81
+     * http://ccvs.cvshome.org/issues/show_bug.cgi?id=81
+     */
+    send_max_dotdot(argc, argv);

     /*
      * aflag controls whether the tag/date is copied into the vers_ts.

[D:\temp\ccvs\src]cvs diff -u sanity.sh
Index: sanity.sh
===================================================================
RCS file: /cvs/ccvs/src/sanity.sh,v
retrieving revision 1.899
diff -u -r1.899 sanity.sh
--- sanity.sh 24 Mar 2004 23:13:04 -0000  1.899
+++ sanity.sh 1 Apr 2004 18:48:19 -0000
@@ -4069,7 +4069,11 @@
    if $remote; then
      dotest_fail files-14 \
 "${testcvs} commit -fmtest ../../first-dir/dir/.file" \
-"protocol error: .\.\./\.\./first-dir/dir' has too many \.\."
+"Checking in \.\./\.\./first-dir/dir/\.file;
+${CVSROOT_DIRNAME}/first-dir/dir/Attic/\.file,v  <--  .file
+new revision: 1\.1\.2\.4; previous revision: 1\.1\.2\.3
+done"
    else
      dotest files-14 \
 "${testcvs} commit -fmtest ../../first-dir/dir/.file" \
@@ -4266,6 +4270,57 @@
    # finds resolved conflicts:
    dotest status-4 "grep 'Result of merge' CVS/Entries" \
 "/tfile/1\.2/Result of merge${PLUS}[a-zA-Z0-9 :]*//"
+
+   cd ..
+   mkdir fourth-dir
+   dotest status-init-8 "${testcvs} add fourth-dir" \
+"Directory ${CVSROOT_DIRNAME}/fourth-dir added to the repository"
+   cd fourth-dir
+   echo yet another line >t3file
+   dotest status-init-9 "${testcvs} add t3file" \
+"${SPROG} add: scheduling file .t3file. for addition
+${SPROG} add: use .${SPROG} commit. to add this file permanently"
+   dotest status-init-10 "${testcvs} -q ci -m add" \
+"RCS file: ${CVSROOT_DIRNAME}/fourth-dir/t3file,v
+done
+Checking in t3file;
+${CVSROOT_DIRNAME}/fourth-dir/t3file,v  <--  t3file
+initial revision: 1\.1
+done"
+   cd ../first-dir
+   mkdir third-dir
+   dotest status-init-11 "${testcvs} add third-dir" \
+"Directory ${CVSROOT_DIRNAME}/first-dir/third-dir added to the repository"
+   cd third-dir
+   echo another line >t2file
+   dotest status-init-12 "${testcvs} add t2file" \
+"${SPROG} add: scheduling file .t2file. for addition
+${SPROG} add: use .${SPROG} commit. to add this file permanently"
+   dotest status-init-13 "${testcvs} -q ci -m add" \
+"RCS file: ${CVSROOT_DIRNAME}/first-dir/third-dir/t2file,v
+done
+Checking in t2file;
+${CVSROOT_DIRNAME}/first-dir/third-dir/t2file,v  <--  t2file
+initial revision: 1\.1
+done"
+   dotest status-5 "${testcvs} status ../tfile" \
+"===================================================================
+File: tfile              Status: Locally Modified
+
+   Working revision: 1\.2.*
+   Repository revision:  1\.2  ${CVSROOT_DIRNAME}/first-dir/tfile,v
+   Sticky Tag:   (none)
+   Sticky Date:    (none)
+   Sticky Options: (none)"
+   dotest status-6 "${testcvs} status ../../fourth-dir/t3file" \
+"===================================================================
+File: t3file             Status: Up-to-date
+
+   Working revision: 1\.1.*
+   Repository revision:  1\.1  ${CVSROOT_DIRNAME}/fourth-dir/t3file,v
+   Sticky Tag:   (none)
+   Sticky Date:    (none)
+   Sticky Options: (none)"

    if $keep; then
      echo Keeping ${TESTDIR} and exiting due to --keep


Derek Robert Price wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris Bohn wrote:
>
>
> >  *subr.c, cvs.h (pathname_levels, path_to_UNIX):  pathname_levels
> > only looks
> >  for "/" characters for delimiting paths.  That's no good under
> > Windows, so
> >  I modified it to call a new function, path_to_UNIX, before doing its
> > work.
> >  The new function basically changes "\" to "/".  The FIXME in
> > pathname_levels
> >  is still a better solution, but it is also more complicated.  This
> > fixes the
>
>
>
> I haven't evaluated the rest of your patch, but I've already implemented
> the FIXME you refer to on both stable and feature in CVS.  It will be
> released in 1.12.7.  Please regenerate your patch against the version of
> CVS in the CVS repository.
>
> Derek
>
> - --
>                 *8^)
>
> Email: derek@ximbiot.com
>
> Get CVS support at <http://ximbiot.com>!
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.1 (GNU/Linux)
> Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org
>
> iD8DBQFAay3hLD1OTBfyMaQRApIjAKDGhkxnAdgv/l8VL0fgh+CqVeGsEQCgnJP/
> By0+LZrRkJZHpwy0RnRImX4=
> =kDcc
> -----END PGP SIGNATURE-----