Last week, Samuel GĂ©lineau reported problems with:
$ svn propdel svn:executable symlink-to-directory
(See http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=130081.)
Deleting the svn:executable property causes 'chmod -x'. However, that
follows links, so it changes the x-bit on the link target -- meaning
it can remove the executable bit on the directory target of the
symlink, which usually isn't the desired result!
There are a couple of ways to solve the problem. This requires some
background; please bear with me.
In io_set_file_perms() -- the guts of svn_io_set_file_executable() --
we don't distinguish between links and their targets. The call to
apr_stat()
status = apr_stat(&finfo, path_apr, APR_FINFO_PROT, pool);
requests only APR_FINFO_PROT information; note how we don't also set
the APR_FINFO_LINK bit for the 'wanted' mask. Because we don't set
that bit, apr_stat() gets info about the target of the link rather
than about the link itself. Here's the relevant code from apr_stat():
if (wanted & APR_FINFO_LINK)
srv = lstat(fname, &info);
else
srv = stat(fname, &info);
And here's the result in the 'finfo' structure we get back:
(gdb) p finfo
$4 = { ...
filetype = APR_DIR,
...
fname = 0x808b350 "wc/symlink"
... }
Yes, that's right: the filetype is APR_DIR, the name is "wc/symlink" :-).
IOW, io_set_file_perms() doesn't even know if it's dealing with a link
or not when it starts banging on the permissions of 'path'... But that
doesn't matter so much, because apr_file_perms_set() also resolves
links and operates on their targets. Thus, the real issue isn't
io_set_file_perms() not knowing if 'path' is a link, but rather not
*caring* whether 'path' ultimately resolves to a file or directory
(even though that information is readily available).
I can think of two solutions, given the above. I'm not sure which is
more correct.
Simple Solution #1:
Make svn_io_set_file_executable() a no-op when invoked on a directory
(even via a link).
In other words, in io_set_file_perms(), if only 'change_executable'
is set and finfo.filetype == APR_DIR, then simply don't do
anything. We'd document that svn_io_set_file_executable() is a
no-op on non-files; change io_set_file_perms()'s name to
io_set_path_perms() (because all its *other* operations would still
work on non-files); and document the exception.
By the way, I don't think this would be an important enough API
change to warrant svn_io_set_file_executable2(), but we could rev
it if people are worried about that.
Not-So-Simple Solution #2:
Have io_set_file_perms() detect when its operand is a link, and
have it set the perms on the link instead of the link's target.
The thing is, I'm not sure that's even possible. The APR function
we use to tweak perms (or anything else about a file) is this:
APR_DECLARE(apr_status_t) apr_file_perms_set(const char *fname,
apr_fileperms_t perms)
{
mode_t mode = apr_unix_perms2mode(perms);
if (chmod(fname, mode) == -1)
return errno;
return APR_SUCCESS;
}
As you can see, it takes the path as a string argument. In other
words, it starts and ends at the symlink too! And it's just
passing off to the chmod(). (apr_unix_perms2mode() doesn't do
anything interesting, by the way -- that is, it doesn't affect any
bits that might mean "act on the link not its target".)
As far as I know, the only way to affect the bits on the link
itself is via the system call lchmod() (which is to chmod() as
lstat() is to stat()). However, APR doesn't even offer an
interface to lchmod():
$ find apr -type f | xargs grep lchmod
$ find apr-util -type f | xargs grep lchmod
$
So Solution #2 would be quite involved. And I'm not even sure it's
better than #1. In Subversion, when would we ever want to change
permissions on a link, as opposed to its target?
I guess I lean toward Solution #1 right now, but would like to hear
others' thoughts.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 17 22:22:24 2007