Re: PATCH: backport of symlink fix for issue 142 to cvs 1.11.9.1

[Derek Robert Price]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark D. Baushke wrote:

| The bug is documented in
|
|     http://ccvs.cvshome.org/issues/show_bug.cgi?id=142
|
| In brief, the use of the CVSROOT/config LockDir=/some/directory option
| has problems if the path in the $CVSROOT contains symbolic links.


In addition to the destabilizing potential of the patch, I was inclined
to consider this a new feature and not a bug fix since it has been
broken for so long that few long-time users of CVS expected it to work.

Run the question by info-cvs and be sure to point out the destabilizing
potential (which we do not generally inflict _intentionally_ on the
"stable" branch, Paul - regardless of how much one or two users would
like to see a feature in the stable branch, I don't like intentionally
violating the contract we created when we advertised the branch as
"stable" - noting the potential destabilization in NEWS and in the
release announcements might be a reasonable compromise) and see what
happens.  As I said, depending on the result, I might change my vote.
Unless Larry decides to register a vote on Mark's side, then the point
is moot.

| Another issue is the interaction of LockDir=/some/directory with
| symbolic links in the $CVSROOT/. tree pointing to various directories.
| To be honest, I do not have such a thing in my repository, so I don't
| know if it is a problem or not in either the feature or the stable
| branches of cvs.


This problem must exist since there is no handling of symlinks in the
locking code.  If a symlink to a directory exists in the repository, for
example:

dir
~ |
~ |---sdir1
~ |
~ |---sdir2
~ |
~ |---sdir3->sdir1

Where you will note that sdir3 is a link to sdir1, then in lockdir, the
following directory structure will be created:

lockdir
~ |
~ |---sdir1
~ |
~ |---sdir2
~ |
~ |---sdir3

The salient point being that despite sdir1 and sdir3 pointing to the
same directory in the repository, two lock directories were created, so
two processes could have write locks for the sdir1 directory at the same
time if they had requested the locks using different names for sdir1.

This problem should be repairable by making the locking functions copy
symlinks into LockDir or perhaps normalizing the symlinks out of
existance, though normalization might bump heads with the current
symlinked CVSROOT fix.

A similar problem exists with links to files within the repository.
This would be slightly harder to fix since the locking code doesn't
currently have access to any file names - as things are currently
implemented the functions which call the locking code would each need to
be aware of whether any files about to be accessed are symlinked and
make separate directory lock requests.  In the case of the
lock_tree_for_write calls, where the caller doesn't have direct access
to the filenames either, this could be painful, if not impossible, to
accomplish without race conditions or the implementation of intent locks.

I also don't know that other problems _don't_ exist.

| >Cool as it might be that feature does work with
| >one now, I am against including the backport on 1.11.x for this reason.
| >
| >If you guys can raise enough relative clamor and few objections here and
| >on info-cvs, I will consider changing my vote.
|
| I'll try to get to that this week after I verify the state of the second
| condition of symbolic links in the repository itself rather than just in
| the path to the repository... after I figure out an easy to to test
it. :-)


That should just be a matter of adding a test that introduces symlinks.
You could just check the directory structure in LockDir, or better yet,
put a write lock into lockdir/sdir1 in the example above by hand and
then commit to sdir1 files via the sdir3 link and observe the potential
carnage.

Of course, the failure case would block indefinately, unless a
background process started before the test slept for 10 seconds before
removing the artificially introduced write lock.

Derek
- --
~                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
- --
I will not prescribe medication.
I will not prescribe medication.
I will not prescribe medication...

~          - Bart Simpson on chalkboard, _The Simpsons_
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)
Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org

iD8DBQE/noAuLD1OTBfyMaQRAggUAJ4yh5CETDcBIlN0/58TDv/kKLebigCgqF1q
b47F9U8ZbFVA1qki12WRCnk=
=JCmM
-----END PGP SIGNATURE-----