[I don't like #1. A comment for release.c might go: /* Some versions of Linux will allow rmdir() on ".", most operating systems will not. Trying to second-guess the operating system in this way is almost surely a bad idea. Code to try looking at st_ino, or however one might try to detect ".", tends to be a portability hassle and fragile. Furthermore I don't see any reason why CVS shouldn't just act the way the operating system does and have "cvs release -d ." behave the same way as "rmdir ." or any other command using rmdir(). */ Something along the lines of #2 should be good and doable, although I haven't looked at this patch in great detail or thought much about variations on that patch. -kingdon] From: mitch@enetis.net To: bug-cvs@gnu.org, info-cvs@gnu.org Subject: Solved FAIL: basicc8 on Linux Date: Thu, 25 Mar 1999 20:56:50 -0700 Lots of people on the list have noticed that basicc-8 is always failing on Linux. I got tired of not passing the tests. It turns out on some Linuxen (At least Debian and RH 2.2.1...) apparently, you are allowed to "rmdir ." and it works fine. The process doing it keeps the directory until a cwd. Tests basicc 8-11 expect "release -d ." etc to fail but they work on linux. So the question for the lists is: Should a CVS user be allowed to "release -d ." on an OS which allows "rmdir ."? As usually, it's come down to bug or feature. There are two ways to go: 1. If you should not be allowed to "release -d ." even if your OS allows it, then you can patch unlink_file_dir() to disallow it. Patch1 below, eg. I vote for this option because then CVS commands will act more the same everywhere and we don't touch the tests. 2. If you should be allowed to "release -d ." even if your OS allows it, then I've patched sanity.sh to guess that this is one of those systems and adjusted the basicc results to be right. (It also fixes a typo in dotest_line_by_line. Patch2 below. So which way should we go? Patches are to 1.10.5.1 from the cyclic repository. Respectfully, Mitchell Perilstein mitch@enetis.net --------------------------- begin patch1-------------------------------- Index: src/filesubr.c =================================================================== RCS file: /home2/cvsroot/ccvs/src/filesubr.c,v retrieving revision 1.41 diff -c -r1.41 filesubr.c *** filesubr.c 1998/08/24 15:40:17 1.41 --- filesubr.c 1999/03/26 03:39:27 *************** *** 426,432 **** unlink_file_dir (f) const char *f; { ! struct stat sb; if (trace #ifdef SERVER_SUPPORT --- 426,432 ---- unlink_file_dir (f) const char *f; { ! struct stat sb, sb2; if (trace #ifdef SERVER_SUPPORT *************** *** 457,464 **** return -1; } } ! else if (S_ISDIR (sb.st_mode)) ! return deep_remove_dir (f); return unlink (f); } --- 457,479 ---- return -1; } } ! else if (S_ISDIR (sb.st_mode)) { ! /* See if we're trying to unlink "." by checking inos. Especially in ! this case we should check stat's result! */ ! if (stat(".", &sb2) < 0) ! { ! if (existence_error (errno)) ! { ! return -1; ! } ! } ! if (sb.st_ino == sb2.st_ino) { ! errno = EINVAL; /* Force errno */ ! return -1; ! } ! else ! return deep_remove_dir (f); ! } return unlink (f); } ---------------------------- end patch1--------------------------------- --------------------------- begin patch2-------------------------------- Index: src/sanity.sh =================================================================== RCS file: /home2/cvsroot/ccvs/src/sanity.sh,v retrieving revision 1.523 diff -c -r1.523 sanity.sh *** sanity.sh 1999/03/24 18:34:02 1.523 --- sanity.sh 1999/03/26 03:09:05 *************** *** 350,356 **** dotest_line_by_line () { line=1 ! while [ $line -le `wc -l ${TESTDIR}/dotest.tmp` ] ; do echo "$line matched \c" >>$LOGFILE if $EXPR "`sed -n ${line}p ${TESTDIR}/dotest.tmp`" : \ "`sed -n ${line}p ${TESTDIR}/dotest.exp`" >/dev/null; then --- 350,356 ---- dotest_line_by_line () { line=1 ! while [ $line -le "`cat ${TESTDIR}/dotest.tmp | wc -l`" ] ; do echo "$line matched \c" >>$LOGFILE if $EXPR "`sed -n ${line}p ${TESTDIR}/dotest.tmp`" : \ "`sed -n ${line}p ${TESTDIR}/dotest.exp`" >/dev/null; then *************** *** 539,544 **** --- 539,556 ---- # TMPPWD=/tmp/cvs-sanity/realdir TMPPWD=`pwd` + # Detect a system where removal of directory "." is allowed, such as Linux. + # This will let us branch in tests which expect "release -d ." to fail; but + # it won't on such systems. To test it, we'll use /bin/rmdir and hope that + # rmdir() works the same way. + mkdir blah + cd blah + if rmdir . > /dev/null 2>&1; then + RMDIR_DOT_ALLOWED=yes + fi + cd ${TESTDIR} + rmdir blah > /dev/null 2>&1 + # Avoid picking up any stray .cvsrc, etc., from the user running the tests mkdir home HOME=${TESTDIR}/home; export HOME *************** *** 1174,1186 **** cd first-dir dotest basicc-6 "${testcvs} release -d" "" dotest basicc-7 "test -d ../first-dir" "" ! dotest basicc-8 "${testcvs} -Q release -d ." \ "${PROG} release: deletion of directory \. failed: .*" ! dotest basicc-9 "test -d ../second-dir" "" ! dotest basicc-10 "test -d ../first-dir" "" ! # For CVS to make a syntactic check for "." wouldn't suffice. ! dotest basicc-11 "${testcvs} -Q release -d ./." \ "${PROG} release: deletion of directory \./\. failed: .*" cd .. cd .. --- 1186,1207 ---- cd first-dir dotest basicc-6 "${testcvs} release -d" "" dotest basicc-7 "test -d ../first-dir" "" ! if test x$RMDIR_DOT_ALLOWED = xyes; then ! dotest basicc-8 "${testcvs} -Q release -d ." "" ! dotest basicc-9 "test -d ../second-dir" "" ! # make sure it's gone ! dotest_fail basicc-10 "test -d ../first-dir" "" ! # this now passes for free ! dotest basicc-11 "true" "" ! else ! dotest basicc-8 "${testcvs} -Q release -d ." \ "${PROG} release: deletion of directory \. failed: .*" ! dotest basicc-9 "test -d ../second-dir" "" ! dotest basicc-10 "test -d ../first-dir" "" ! # For CVS to make a syntactic check for "." wouldn't suffice. ! dotest basicc-11 "${testcvs} -Q release -d ./." \ "${PROG} release: deletion of directory \./\. failed: .*" + fi # if RMDIR_DOT_ALLOWED cd .. cd .. ---------------------------- end inclusion---------------------------------