You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Justin Erenkrantz <je...@apache.org> on 2002/08/21 06:02:48 UTC
[PATCH] Add svn:permissions support
As posted earlier, this patch allows file permissions to become a
property of the repository that the client can then respect.
local:
svn propset svn:permissions 600 <file>
svn ci
somewhere else:
svn co <repos-path>
*Poof, permissions are set on the working copy as specified.*
(Of course, on Win32 or other platforms, this would be a no-op.)
I'm happy and would love to see this make it into the main tree.
As it is, my tree is slowly coming out-of-sync with the trunk
as I have a lot of local changes. I should attempt to resync
with trunk if I get a chance (but I'm not sure how many of the
changes would be accepted by the rest of the group). -- justin
* subversion/include/svn_props.h:
Add SVN_PROP_PERMISSIONS define.
* subversion/libsvn_wc/translate.h, subversion/libsvn_wc/translate.c:
Add svn_wc__maybe_set_permissions
* subversion/include/svn_io.h, subversion/libsvn_subr/io.c:
Add svn_io_set_file_permissions
* subversion/libsvn_wc/props.c (svn_wc_prop_set):
If encounter SVN_PROP_PERMISSIONS, call svn_io_set_file_permissions.
* subversion/libsvn_wc/log.c (file_xfer_under_path, install_committed_file),
subversion/libsvn_wc/adm_ops.c (revert_admin_things),
subversion/libsvn_wc/merge.c (svn_wc_merge),
subversion/libsvn_wc/adm_crawler.c (restore_file):
Call svn_wc__maybe_set_permissions
Index: subversion/include/svn_props.h
===================================================================
--- subversion/include/svn_props.h
+++ subversion/include/svn_props.h Tue Aug 20 18:38:30 2002
@@ -126,6 +126,9 @@
/* The character set of a given file. */
#define SVN_PROP_CHARSET SVN_PROP_PREFIX "charset"
+/* Defines a numeric permissions mode of a given file. */
+#define SVN_PROP_PERMISSIONS SVN_PROP_PREFIX "permissions"
+
/* --------------------------------------------------------------------- */
/** INVISIBLE PROPERTIES **/
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h
+++ subversion/include/svn_io.h Tue Aug 20 19:32:37 2002
@@ -193,6 +193,15 @@
svn_boolean_t ignore_enoent,
apr_pool_t *pool);
+/* Set a file's permissions, as much as the operating system
+ allows. PATH is the utf8-encoded path to the file. PERMS is the
+ string-encoded permissions. If IGNORE_ENOENT is TRUE, don't fail if
+ the target file doesn't exist. */
+svn_error_t *svn_io_set_file_permissions (const char *path,
+ const char *perms,
+ svn_boolean_t ignore_enoent,
+ apr_pool_t *pool);
+
/* Read a line from FILE into BUF, but not exceeding *LIMIT bytes.
* Does not include newline, instead '\0' is put there.
Index: subversion/libsvn_wc/merge.c
===================================================================
--- subversion/libsvn_wc/merge.c
+++ subversion/libsvn_wc/merge.c Tue Aug 20 22:48:08 2002
@@ -332,7 +332,9 @@
} /* end of binary conflict handling */
/* Merging is complete. Regardless of text or binariness, we might
- need to tweak the executable bit on the new working file. */
+ need to tweak the permissions on the new working file. */
+ SVN_ERR (svn_wc__maybe_set_permissions (&toggled, merge_target, pool));
+
SVN_ERR (svn_wc__maybe_toggle_working_executable_bit (&toggled,
merge_target, pool));
Index: subversion/libsvn_wc/translate.h
===================================================================
--- subversion/libsvn_wc/translate.h
+++ subversion/libsvn_wc/translate.h Tue Aug 20 22:46:38 2002
@@ -110,6 +110,14 @@
const char *path,
apr_pool_t *pool);
+/* If the SVN_PROP_PERMISSIONS property is present, then set PATH to the
+ right permissions. Set *TOGGLED to TRUE if this happened, or FALSE
+ if not. */
+svn_error_t *
+svn_wc__maybe_set_permissions (svn_boolean_t *toggled,
+ const char *path,
+ apr_pool_t *pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c
+++ subversion/libsvn_wc/props.c Tue Aug 20 19:35:58 2002
@@ -1060,6 +1060,10 @@
{
SVN_ERR (svn_validate_mime_type (value->data, pool));
}
+ else if ((strcmp (name, SVN_PROP_PERMISSIONS) == 0) && value)
+ {
+ SVN_ERR (svn_io_set_file_permissions (path, value->data, TRUE, pool));
+ }
err = svn_wc_prop_list (&prophash, path, pool);
if (err)
Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c
+++ subversion/libsvn_wc/adm_crawler.c Tue Aug 20 22:46:46 2002
@@ -79,6 +79,9 @@
SVN_ERR (svn_io_remove_file (tmp_text_base_path, pool));
+ /* If necessary, tweak the new working file's permissions bit. */
+ SVN_ERR (svn_wc__maybe_set_permissions (&toggled, file_path, pool));
+
/* If necessary, tweak the new working file's executable bit. */
SVN_ERR (svn_wc__maybe_toggle_working_executable_bit
(&toggled, file_path, pool));
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c
+++ subversion/libsvn_wc/log.c Tue Aug 20 22:48:01 2002
@@ -118,7 +118,12 @@
TRUE,
pool));
- /* After copying, toggle the executable bit if props dictate. */
+ /* After copying, toggle the executable bit or permissions if props
+ * dictate.
+ */
+ SVN_ERR (svn_wc__maybe_set_permissions (&toggled, full_dest_path,
+ pool));
+
return svn_wc__maybe_toggle_working_executable_bit
(&toggled,
full_dest_path,
@@ -265,6 +270,13 @@
}
SVN_ERR (svn_io_remove_file (tmp_wfile, pool));
+
+ /* Set the permissions if props dictate. */
+ SVN_ERR (svn_wc__maybe_set_permissions (&toggled, filepath, pool));
+ if (toggled)
+ /* okay, so we didn't -overwrite- the working file, but we changed
+ its timestamp, which is the point of returning this flag. :-) */
+ *overwrote_working = TRUE;
/* Toggle the working file's execute bit if props dictate. */
SVN_ERR (svn_wc__maybe_toggle_working_executable_bit (&toggled,
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c
+++ subversion/libsvn_wc/adm_ops.c Tue Aug 20 22:46:57 2002
@@ -1105,6 +1105,9 @@
pool)))
return revert_error (err, fullpath, "restoring text", pool);
+ /* If necessary, set the new file's permissions. */
+ SVN_ERR (svn_wc__maybe_set_permissions (&toggled, fullpath, pool));
+
/* If necessary, tweak the new working file's executable bit. */
SVN_ERR (svn_wc__maybe_toggle_working_executable_bit
(&toggled, fullpath, pool));
Index: subversion/libsvn_wc/translate.c
===================================================================
--- subversion/libsvn_wc/translate.c
+++ subversion/libsvn_wc/translate.c Tue Aug 20 22:46:03 2002
@@ -1040,6 +1040,25 @@
return SVN_NO_ERROR;
}
+svn_error_t *
+svn_wc__maybe_set_permissions (svn_boolean_t *toggled,
+ const char *path,
+ apr_pool_t *pool)
+{
+ const svn_string_t *propval;
+ SVN_ERR (svn_wc_prop_get (&propval, SVN_PROP_PERMISSIONS, path, pool));
+
+ if (propval != NULL)
+ {
+ SVN_ERR (svn_io_set_file_permissions (path, propval->data, FALSE, pool));
+ *toggled = TRUE;
+ }
+ else {
+ *toggled = FALSE;
+ }
+
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c
+++ subversion/libsvn_subr/io.c Tue Aug 20 22:24:28 2002
@@ -639,6 +639,36 @@
return SVN_NO_ERROR;
}
+svn_error_t *
+svn_io_set_file_permissions (const char *path,
+ const char *perms,
+ svn_boolean_t ignore_enoent,
+ apr_pool_t *pool)
+{
+ apr_status_t status;
+ const char *path_native;
+ apr_int64_t i;
+
+ SVN_ERR (svn_utf_cstring_from_utf8 (&path_native, path, pool));
+
+ /* APR's permissions flags are hex-equivalents of the octets. */
+ i = apr_strtoi64 (perms, NULL, 16);
+
+ /* This cast is probably safe since we're going from 64-bit to a
+ * 32-bit value.
+ */
+ status = apr_file_perms_set (path, (apr_fileperms_t) i);
+ if (status != APR_SUCCESS && !APR_STATUS_IS_INCOMPLETE(status) &&
+ !APR_STATUS_IS_ENOTIMPL(status) &&
+ (!ignore_enoent || !APR_STATUS_IS_ENOENT(status))) {
+ return svn_error_createf (status, 0, NULL, pool,
+ "svn_io_set_file_permissions: "
+ "failed to set permissions on %s (%s)",
+ path, perms);
+ }
+
+ return SVN_NO_ERROR;
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add svn:permissions support
Posted by Benjamin Pflugmann <be...@pflugmann.de>.
Hello.
[...]
> Justin Erenkrantz <je...@apache.org> writes:
> > Please remember that CVS respects permissions - it's an inherent flaw
> > in SVN's model that it can't. A working copy in CVS has at least
> > the same permissions as its ,v file. So, for true CVS compliance, you
> > need to implement something like this.
> >
> > I doubt I'm the only one who has relied on the permissions feature
> > of CVS. So, here's a case where SVN 1.0 will not be as good as CVS
> > if this feature isn't present. -- justin
>
> How do such CVS working copies behave under Windows, or other non-Unix
> OS's?
Don't know.
> And what does it mean when the same users and groups don't exist on
> one working copy box as on another?
AFAIK, this is only about permissions, but not about user/groups,
i.e. the rwx flags for "ugo" are preserved. The user and group are
simply set to whoever you happen to be at the moment of checkout.
> Can you describe exactly how you depended on this feature of CVS,
> maybe?
I also depend implicitly on this feature... without having thought
much about it until now. It may not be normal (i.e. intended) use, but
out of some need, I happened several times to use a working copy
directly for production use, i.e. without an install procedure to copy
the data into its final location, but checking it out to its final
location directly. Not pretty, but it works.
Regards,
Benjamin.
Re: [PATCH] Add svn:permissions support
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> I'm totally against committing this as-is. We've made far too many
> "oh, this works for me, let's put it in" changes already. Justin,
> please write up a design doc for this feature, like I suggested in the
> other thread, and let's kick it around a bit before deciding if, how
> and when to implement this.
+1
For example:
Justin Erenkrantz <je...@apache.org> writes:
> Please remember that CVS respects permissions - it's an inherent flaw
> in SVN's model that it can't. A working copy in CVS has at least
> the same permissions as its ,v file. So, for true CVS compliance, you
> need to implement something like this.
>
> I doubt I'm the only one who has relied on the permissions feature
> of CVS. So, here's a case where SVN 1.0 will not be as good as CVS
> if this feature isn't present. -- justin
How do such CVS working copies behave under Windows, or other non-Unix
OS's?
And what does it mean when the same users and groups don't exist on
one working copy box as on another?
Can you describe exactly how you depended on this feature of CVS,
maybe?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add svn:permissions support
Posted by Branko Čibej <br...@xbc.nu>.
Justin Erenkrantz wrote:
>As posted earlier, this patch allows file permissions to become a
>property of the repository that the client can then respect.
>
>
I'm totally against committing this as-is. We've made far too many "oh,
this works for me, let's put it in" changes already. Justin, please
write up a design doc for this feature, like I suggested in the other
thread, and let's kick it around a bit before deciding if, how and when
to implement this.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re: [PATCH] Add svn:permissions support
Posted by Daniel Berlin <db...@dberlin.org>.
On Wed, 21 Aug 2002, Justin Erenkrantz wrote:
> On Wed, Aug 21, 2002 at 04:48:26PM -0700, Bill Tutt wrote:
> > If you want more, write a wrapper that sets a property, or a mechanism
> > to record/restore that information on your own. I mean that's what
> > Subversion properties are meant to be for anyway. User and/or project
> > specific properties.
>
> You can't easily manage the permissions of the working copy as SVN
> itself trounces the permissions willy-nilly. It uses APR_OS_DEFAULT
> whenever it does a copy/revert/update, etc, etc. You'd need a notion
> of client-side hooks to even have a shot of implementing this.
We, um, have them.
It's called swig bindings.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re: [PATCH] Add svn:permissions support
Posted by Justin Erenkrantz <je...@apache.org>.
On Wed, Aug 21, 2002 at 04:48:26PM -0700, Bill Tutt wrote:
> If you want more, write a wrapper that sets a property, or a mechanism
> to record/restore that information on your own. I mean that's what
> Subversion properties are meant to be for anyway. User and/or project
> specific properties.
You can't easily manage the permissions of the working copy as SVN
itself trounces the permissions willy-nilly. It uses APR_OS_DEFAULT
whenever it does a copy/revert/update, etc, etc. You'd need a notion
of client-side hooks to even have a shot of implementing this.
So, no, I disagree: the SCM system needs to understand this. If
the SCM left the permissions alone, I'd agree. Subversion doesn't.
And, as my patch hopefully pointed out, the change is minimal.
> We'd have a heck of a lot more work to do for v1 if we thought we needed
> to auto-magically arbitrary OS (and indeed filesystem specific)
> properties. Whether they be NTFS parse points, Unix permissions, NT
> (D)ACLs, etc...
Low-hanging fruit. Of course, the problem is that we need to define
a portable way to define what a permission is.
Please remember that CVS respects permissions - it's an inherent flaw
in SVN's model that it can't. A working copy in CVS has at least
the same permissions as its ,v file. So, for true CVS compliance, you
need to implement something like this.
I doubt I'm the only one who has relied on the permissions feature
of CVS. So, here's a case where SVN 1.0 will not be as good as CVS
if this feature isn't present. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: [PATCH] Add svn:permissions support
Posted by Daniel Berlin <db...@dberlin.org>.
On Wed, 21 Aug 2002, Bill Tutt wrote:
> Indeed. My personal biased opinion on this is that Subversion is mostly
> for managing source code repositories, and not for managing completely
> re-creatable directory images. So things like auto-magically handling
> complex multi-OS permission reproduction is just out of scope. (Aside
> from the executable bit, and readonly-ness.)
>
> If you want more, write a wrapper that sets a property, or a mechanism
> to record/restore that information on your own. I mean that's what
> Subversion properties are meant to be for anyway. User and/or project
> specific properties.
This is what i was gonna say.
It's not like we don't have python bindings, and theoretically
java/perl/etc bindings (through SWIG. I've left out the hand written java
bindings because i have no clue what state they are in).
What is so difficult about integrating support for your custom permissions
into whatever client you are writing to do the directory images (assuming
you are not going to be doing lots of image updating by hand on the
machines you are building the images for)?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: [PATCH] Add svn:permissions support
Posted by Bill Tutt <ra...@lyra.org>.
Indeed. My personal biased opinion on this is that Subversion is mostly
for managing source code repositories, and not for managing completely
re-creatable directory images. So things like auto-magically handling
complex multi-OS permission reproduction is just out of scope. (Aside
from the executable bit, and readonly-ness.)
If you want more, write a wrapper that sets a property, or a mechanism
to record/restore that information on your own. I mean that's what
Subversion properties are meant to be for anyway. User and/or project
specific properties.
We'd have a heck of a lot more work to do for v1 if we thought we needed
to auto-magically arbitrary OS (and indeed filesystem specific)
properties. Whether they be NTFS parse points, Unix permissions, NT
(D)ACLs, etc...
Not to mention sharing more representations, and analyzing whether or
not XDelta would be more appropriate for transmitting obnoxiously large
binary files. (i.e. > 5Mb or so) So we might reap any benefits inherent
in an rsync like updating mechanism.
Sorry, this is just one of my pet-peeves. Source code management systems
aren't designed to be arbitrary versioned backup systems, or machine
image deployment repositories.
Bill
> From: Branko Cibej [mailto:brane@xbc.nu]
>
> Justin Erenkrantz wrote:
>
> >On Wed, Aug 21, 2002 at 01:19:31AM -0500, cmpilato@collab.net wrote:
> >
> >
> >>Hm... why can't we at least set the read-only bit on Windows (as,
say,
> >>the negation of the world-writable Unix perm) ?
> >>
> >>
> >
> >That should be the job of APR's apr_file_perms_set().
> >
> No. We decided long ago (and correctly, IMHO) that this is _not_
> apr_file_perms_set's job. There are other interfaces in APR for making
a
> file read-only, and we're already using those in Subversion (in the
> admin area handling thingies).
>
> --
> Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add svn:permissions support
Posted by Branko Čibej <br...@xbc.nu>.
Justin Erenkrantz wrote:
>On Wed, Aug 21, 2002 at 01:19:31AM -0500, cmpilato@collab.net wrote:
>
>
>>Hm... why can't we at least set the read-only bit on Windows (as, say,
>>the negation of the world-writable Unix perm) ?
>>
>>
>
>That should be the job of APR's apr_file_perms_set().
>
No. We decided long ago (and correctly, IMHO) that this is _not_
apr_file_perms_set's job. There are other interfaces in APR for making a
file read-only, and we're already using those in Subversion (in the
admin area handling thingies).
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add svn:permissions support
Posted by Justin Erenkrantz <je...@apache.org>.
On Wed, Aug 21, 2002 at 01:19:31AM -0500, cmpilato@collab.net wrote:
> Hm... why can't we at least set the read-only bit on Windows (as, say,
> the negation of the world-writable Unix perm) ?
That should be the job of APR's apr_file_perms_set(). I have no
Win32 foo, so I have no idea what Windows can and cannot do.
(It just returns ENOTIMPL now.)
But, if a Win32 hacker taught APR how to set permissions, this
code would do the 'right' thing on Win32. My point is simply
that it is a no-op on Win32 now. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add svn:permissions support
Posted by cm...@collab.net.
Justin Erenkrantz <je...@apache.org> writes:
> As posted earlier, this patch allows file permissions to become a
> property of the repository that the client can then respect.
>
> local:
> svn propset svn:permissions 600 <file>
> svn ci
>
> somewhere else:
> svn co <repos-path>
> *Poof, permissions are set on the working copy as specified.*
>
> (Of course, on Win32 or other platforms, this would be a no-op.)
Hm... why can't we at least set the read-only bit on Windows (as, say,
the negation of the world-writable Unix perm) ?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org