[this isn't particularly AFS-related, nor is it a bugfix, but it represents another variation on solution #2, "Provide a means to put the cvs locks in some directory apart from the repository" mentioned in lock.c. I don't see anything much wrong with it, but I'm not sure it is worth bothering with compatibility and other hassles if, by the author's own admission, other solutions would be better. There is also a question of whether CVS mechanisms such as CVSROOT/readers are a better way to grant readonly access than file permissions. It would be nice to make features like this (or the straight #2, in which the whole lock tree is elsewhere, rather than having a #cvs.locks in every repository directory) enabled by something which will cause old versions of CVS to barf (to avoid having locks silently fail to work). The most straightforward way, a new setting in CVSROOT/config, does not work with CVS 1.9 and older, which don't know anything about CVSROOT/config. I suppose we could add a "$have_config" line or some such to modules to indicate that CVSROOT/config should be present. -kingdon, 9 Oct 1997] Date: Mon, 15 Sep 1997 20:42:39 +0200 From: "Lassi A. Tuura" Organization: CERN, European Laboratory for Particle Physics To: bug-cvs@prep.ai.mit.edu Subject: AFS-related bugfix to CVS locking scheme This is a multi-part message in MIME format. --------------22DAC26AD3CEC0FC3C0A8951 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Hi, We found an AFS (Andrew File System) related bugfix with CVS 1.9. The problem occurs when we use ACLs (access control lists) with the directories, trying to restrict write access rights to the repository to appropriate people. Since we have potentially about hundreds of CVS users with a single repository, we would not want everyone to be able to write anywhere. However, we do want to let everybody to check the code out. What we thought we could do: - grant package authors write access to their package - grant everybody read access everywhere - grant maintainers write access everywhere This gets complicated because AFS defines ACLs per directory, not per file. With the above setup the package authors can checkout and commit their packages. The trouble is permitting everybody to check the code out. In particular there are two problems: - the history file has to be writable - cvs must be able to create the read locks The first problem is easy to work around: just create another directory inside CVSROOT (CVSROOT/public), and give everybody write access on that. Then, we can move the history file there and create a symlink in CVSROOT: CVSROOT/history -> CVSROOT/public/history. This works well, although it would be nicer to have an option in `modules' letting us define the right place (there are no symlinks on Windows NT). The second problem is impossible to work around given the way CVS works -- at least as far as I could make out; I would be glad to be proven wrong. Namely, CVS wants to create a directory `#cvs.lock' in each repository directory, and files `#cvs.?fl.*' for reader/writer locks. Under AFS this means that we have to give everybody the permission to create and delete files (`rlid' for those who know AFS) -- but this means that even though the people cannot *write* over the files in the repository, they can *delete* them. In other words, everyone is able to say `rm -fr $CVSROOT'. Not good :-(. Hence, I have come up with a patch that avoids this problem, although it isn't perfect. Namely, I have modified CVS to create a *persistent* `#cvs.locks' directory, in which it will create the lock directories/files. With this scheme we can give the appropriate ACLs for the lock directory (we cannot do that under the current scheme, since the new directories inherit ACLs from their parent, and the lock directories come and go on every CVS access). This directory is created whenever CVS adds a new directory into the repository (at least I hope it does; I tried to hunt down all occasions where it can happen). The downside of this scheme is that it still requires maintenance: after the package maintainers have done cvs add or import for a directory, they need to change the ACLs on the created `#cvs.locks' directory. But even that is better than nothing... I would most prefer to see a solution where there was a directory CVSROOT/locks (definable in `modules'), with a persistent directory structure that replicates the repository's structure (remember, we need to be able to set the ACLs on the directory; it is no good if the locks in this special directory come and go) Any comments on the patch in the attachment? Any chance of getting this into CVS quickly? We would very much prefer to have the standard sources and avoid locally modified versions. If there was another release, we can press everybody to upgrade, but to get everybody to use local versions is much more difficult... Cheers, and thanks for the great tool! //lat -- Lassi.Tuura@cern.ch There's no sunrise without a night --------------22DAC26AD3CEC0FC3C0A8951 Content-Type: text/plain; charset=us-ascii; name="cvs-locks-patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="cvs-locks-patch" diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/add.c ./src/add.c --- /asis/src/GNU.LANG/cvs-1.9.2/src/add.c Wed Aug 7 23:46:04 1996 +++ ./src/add.c Mon Sep 15 19:01:50 1997 @@ -478,6 +478,8 @@ if (!noexec) { + char lockdir[PATH_MAX]; + omask = umask (cvsumask); if (CVS_MKDIR (rcsdir, 0777) < 0) { @@ -485,6 +487,14 @@ (void) umask (omask); goto out; } + + (void) sprintf (lockdir, "%s/%s", rcsdir, CVSLCKDIR); + if (CVS_MKDIR (lockdir, 0777) < 0) + { + error (0, errno, "cannot mkdir %s", lockdir); + (void) umask (omask); + goto out; + } (void) umask (omask); } diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/cvs.h ./src/cvs.h --- /asis/src/GNU.LANG/cvs-1.9.2/src/cvs.h Wed Oct 2 08:03:59 1996 +++ ./src/cvs.h Mon Sep 15 19:01:51 1997 @@ -189,10 +189,11 @@ some operations (such as main branch checkouts and updates). */ #define CVSATTIC "Attic" -#define CVSLCK "#cvs.lock" -#define CVSRFL "#cvs.rfl" -#define CVSWFL "#cvs.wfl" -#define CVSRFLPAT "#cvs.rfl.*" /* wildcard expr to match read locks */ +#define CVSLCKDIR "#cvs.locks" +#define CVSLCK "lock" +#define CVSRFL "rfl" +#define CVSWFL "wfl" +#define CVSRFLPAT "rfl.*" /* wildcard expr to match read locks */ #define CVSEXT_LOG ",t" #define CVSPREFIX ",," #define CVSDOTIGNORE ".cvsignore" diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/filesubr.c ./src/filesubr.c --- /asis/src/GNU.LANG/cvs-1.9.2/src/filesubr.c Wed Sep 25 16:42:48 1996 +++ ./src/filesubr.c Mon Sep 15 19:01:50 1997 @@ -274,7 +274,13 @@ return; if (mkdir (name, 0777) == 0 || errno == EEXIST) - return; + { + cp = xmalloc (strlen (name) + 1 + strlen (CVSLCKDIR) + 1); + (void) sprintf (cp, "%s/%s", name, CVSLCKDIR); + name = cp; + if (mkdir (name, 0777) == 0 || errno == EEXIST) + return; + } if (! existence_error (errno)) { error (0, errno, "cannot make path to %s", name); @@ -287,7 +293,13 @@ *cp++ = '/'; if (*cp == '\0') return; - (void) mkdir (name, 0777); + if (mkdir (name, 0777) == 0) + { + cp = xmalloc (strlen (name) + 1 + strlen (CVSLCKDIR) + 1); + (void) sprintf (cp, "%s/%s", name, CVSLCKDIR); + name = cp; + (void) mkdir (name, 0777); + } } /* Create directory NAME if it does not already exist; fatal error for diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/find_names.c ./src/find_names.c --- /asis/src/GNU.LANG/cvs-1.9.2/src/find_names.c Wed Aug 7 23:46:26 1996 +++ ./src/find_names.c Mon Sep 15 19:01:50 1997 @@ -305,7 +305,7 @@ if (strcmp (dp->d_name, ".") == 0 || strcmp (dp->d_name, "..") == 0 || strcmp (dp->d_name, CVSATTIC) == 0 || - strcmp (dp->d_name, CVSLCK) == 0 || + strcmp (dp->d_name, CVSLCKDIR) == 0 || strcmp (dp->d_name, CVSREP) == 0) continue; diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/import.c ./src/import.c --- /asis/src/GNU.LANG/cvs-1.9.2/src/import.c Wed Oct 2 08:04:00 1996 +++ ./src/import.c Mon Sep 15 19:01:49 1997 @@ -1211,8 +1211,10 @@ #endif { char rcs[PATH_MAX]; + char lockdir[PATH_MAX]; (void) sprintf (rcs, "%s%s", repository, RCSEXT); + (void) sprintf (lockdir, "%s/%s", repository, CVSLCKDIR); if (isfile (repository) || isfile(rcs)) { fperror (logfp, 0, 0, "ERROR: %s is a file, should be a directory!", @@ -1230,6 +1232,18 @@ error (0, ierrno, "ERROR: cannot mkdir %s -- not added", repository); err = 1; + goto out; + } + if (noexec == 0 && CVS_MKDIR (lockdir, 0777) < 0) + { + ierrno = errno; + fperror (logfp, 0, ierrno, + "ERROR: cannot mkdir %s -- not added", repository); + error (0, ierrno, + "ERROR: cannot mkdir %s -- not added", repository); + err = 1; + /* Ignore error status -- not much we can do about it. */ + CVS_RMDIR (repository); goto out; } } diff -ur /asis/src/GNU.LANG/cvs-1.9.2/src/lock.c ./src/lock.c --- /asis/src/GNU.LANG/cvs-1.9.2/src/lock.c Tue Sep 17 06:30:08 1996 +++ ./src/lock.c Mon Sep 15 19:49:43 1997 @@ -140,14 +140,14 @@ if (readlock[0] != '\0') { - (void) sprintf (tmp, "%s/%s", repository, readlock); + (void) sprintf (tmp, "%s/%s/%s", repository, CVSLCKDIR, readlock); if ( CVS_UNLINK (tmp) < 0 && ! existence_error (errno)) error (0, errno, "failed to remove lock %s", tmp); } if (writelock[0] != '\0') { - (void) sprintf (tmp, "%s/%s", repository, writelock); + (void) sprintf (tmp, "%s/%s/%s", repository, CVSLCKDIR, writelock); if ( CVS_UNLINK (tmp) < 0 && ! existence_error (errno)) error (0, errno, "failed to remove lock %s", tmp); } @@ -159,7 +159,7 @@ */ if (writelock[0] != '\0' || (readlock[0] != '\0' && cleanup_lckdir)) { - (void) sprintf (tmp, "%s/%s", repository, CVSLCK); + (void) sprintf (tmp, "%s/%s/%s", repository, CVSLCKDIR, CVSLCK); if (Check_Owner(tmp)) { #ifdef AFSCVS @@ -248,7 +248,7 @@ } /* write a read-lock */ - (void) sprintf (tmp, "%s/%s", xrepository, readlock); + (void) sprintf (tmp, "%s/%s/%s", xrepository, CVSLCKDIR, readlock); if ((fp = CVS_FOPEN (tmp, "w+")) == NULL || fclose (fp) == EOF) { error (0, errno, "cannot create read lock in repository `%s'", @@ -385,7 +385,7 @@ } /* write the write-lock file */ - (void) sprintf (tmp, "%s/%s", repository, writelock); + (void) sprintf (tmp, "%s/%s/%s", repository, CVSLCKDIR, writelock); if ((fp = CVS_FOPEN (tmp, "w+")) == NULL || fclose (fp) == EOF) { int xerrno = errno; @@ -419,6 +419,7 @@ readers_exist (repository) char *repository; { + char *lockdir; char *line; DIR *dirp; struct dirent *dp; @@ -429,8 +430,11 @@ again: #endif - if ((dirp = CVS_OPENDIR (repository)) == NULL) - error (1, 0, "cannot open directory %s", repository); + lockdir = xmalloc (strlen (repository) + 1 + strlen (CVSLCKDIR) + 1); + (void) sprintf (lockdir, "%s/%s", repository, CVSLCKDIR); + + if ((dirp = CVS_OPENDIR (lockdir)) == NULL) + error (1, 0, "cannot open directory %s", lockdir); errno = 0; while ((dp = readdir (dirp)) != NULL) @@ -442,8 +446,8 @@ (void) time (&now); #endif - line = xmalloc (strlen (repository) + strlen (dp->d_name) + 5); - (void) sprintf (line, "%s/%s", repository, dp->d_name); + line = xmalloc (strlen (lockdir) + strlen (dp->d_name) + 5); + (void) sprintf (line, "%s/%s", lockdir, dp->d_name); if ( CVS_STAT (line, &sb) != -1) { #ifdef CVS_FUDGELOCKS @@ -478,7 +482,7 @@ errno = 0; } if (errno != 0) - error (0, errno, "error reading directory %s", repository); + error (0, errno, "error reading directory %s", lockdir); closedir (dirp); return (ret); @@ -520,7 +524,31 @@ time_t now; #endif - (void) sprintf (masterlock, "%s/%s", repository, CVSLCK); + (void) sprintf (masterlock, "%s/%s", repository, CVSLCKDIR); + + /* Create the lock directory if it doesn't exist. This can + happen if the repository has been created with an older + version of cvs. Note that this wreak havoc if the same + repository is used with older and newer versions of cvs + concurrently. */ + if (CVS_STAT (masterlock, &sb) < 0) + { + int status = -1; + + omask = umask (cvsumask); + SIG_beginCrSect (); + if (CVS_MKDIR (masterlock, 0777) != 0) { + error (0, errno, "couldn't create lock directory `%s'", masterlock); + status = L_ERROR; + } + SIG_endCrSect (); + (void) umask (omask); + + if (status != -1) + return status; + } + + (void) sprintf (masterlock, "%s/%s/%s", repository, CVSLCKDIR, CVSLCK); /* * Note that it is up to the callers of set_lock() to arrange for signal --------------22DAC26AD3CEC0FC3C0A8951--