Content-type: text/plain This is for Subversion issue #532, about making .svn/text-base/ files read only. Apply this patch to your working tree, rebuild, run make check, and start debugging the test failures. --------------------8-<-------cut-here---------8-<----------------------- Set text-base files and prop-base files read-only. This resolves issue #532 (OR WILL RESOLVE IT, WHEN THIS PATCH IS COMPLETED). * subversion/libsvn_subr/io.c (svn_io_set_file_read_only): New func. * subversion/include/svn_io.h (svn_io_set_file_read_only): Prototype for new func. * subversion/libsvn_wc/adm_files.c: Include strcmp via apr_want.h. (v_extend_with_adm_name): If setting a text_base or prop_base path, notify caller by reference via new parameter `is_base'. All callers changed. (extend_with_adm_name): Same. (sync_adm_file): If sync'ing a base file, set it read-only when done. Also, use APR_STATUS_IS_SUCCESS to check return value. (close_adm_file): Same. (svn_wc__sync_text_base): Fix grammar in comment; the incorrectness of the popular "had better" was convincingly demonstrated by Noel Taylor . I'm sure Brane has something to say about this. * subversion/libsvn_wc/log.c (log_do_committed): Set prop base read-only. * subversion/libsvn_wc/copy.c (copy_file_administratively): Set prop base read-only. Index: ./subversion/include/svn_io.h =================================================================== --- ./subversion/include/.svn/text-base/svn_io.h Thu Oct 25 10:40:12 2001 +++ ./subversion/include/svn_io.h Mon Oct 29 11:54:15 2001 @@ -142,6 +142,19 @@ svn_io_read_length_line (apr_file_t *file, char *buf, apr_size_t *limit); +/* Make a file as read-only as the operating system allows. + * + * For example, under Unix, this will make the file read-only for + * `user,' `group', and `other', leaving the file's other mode bits + * untouched. + * + * ### todo: someone care to document exactly how this behaves under + * Windows? + */ +svn_error_t *svn_io_set_file_read_only (const char *fname, apr_pool_t *pool); + + + /* Set *APR_TIME to the later of PATH's (a regular file) mtime or ctime. * * Unix traditionally distinguishes between "mod time", which is when Index: ./subversion/libsvn_wc/copy.c =================================================================== --- ./subversion/libsvn_wc/.svn/text-base/copy.c Thu Oct 25 10:40:12 2001 +++ ./subversion/libsvn_wc/copy.c Mon Oct 29 12:03:25 2001 @@ -99,7 +99,8 @@ /* Copy the text-base over unconditionally. */ SVN_ERR (svn_io_copy_file (src_txtb, dst_txtb, pool)); - + SVN_ERR (svn_io_set_file_read_only (dst_txtb->data, pool)); + /* Copy the props over if they exist. */ SVN_ERR (svn_io_check_path (src_wprop, &kind, pool)); if (kind == svn_node_file) @@ -108,7 +109,10 @@ /* Copy the base-props over if they exist */ SVN_ERR (svn_io_check_path (src_bprop, &kind, pool)); if (kind == svn_node_file) - SVN_ERR (svn_io_copy_file (src_bprop, dst_bprop, pool)); + { + SVN_ERR (svn_io_copy_file (src_bprop, dst_bprop, pool)); + SVN_ERR (svn_io_set_file_read_only (dst_bprop->data, pool)); + } } Index: ./subversion/libsvn_wc/log.c =================================================================== --- ./subversion/libsvn_wc/.svn/text-base/log.c Thu Oct 25 18:53:21 2001 +++ ./subversion/libsvn_wc/log.c Mon Oct 29 12:03:56 2001 @@ -958,6 +958,10 @@ "error renaming %s to %s", tmp_prop_path->data, prop_base_path->data); + else + SVN_ERR (svn_io_set_file_read_only (prop_base_path->data, + loggy->pool)); + } Index: ./subversion/libsvn_wc/adm_files.c =================================================================== --- ./subversion/libsvn_wc/.svn/text-base/adm_files.c Thu Oct 25 18:53:22 2001 +++ ./subversion/libsvn_wc/adm_files.c Mon Oct 29 15:25:29 2001 @@ -22,6 +22,9 @@ +#define APR_WANT_STRFUNC +#include + #include #include #include @@ -57,7 +60,9 @@ /* Extend PATH to the name of something in PATH's administrative area. - * Returns the number of path components added to PATH. + * Returns the number of path components added to PATH. If the + * administrative area is the text-base or prop-base and IS_BASE is + * non-null, set *IS_BASE to true, else set it to false. * * First, the adm subdir is appended to PATH as a component, then the * "tmp" directory is added iff USE_TMP is set, then each of the @@ -76,13 +81,20 @@ * always have exactly one return statement, occurring *after* an * unconditional call to chop_admin_name(). */ static int -v_extend_with_adm_name (svn_stringbuf_t *path, +v_extend_with_adm_name (svn_boolean_t *is_base_p, + svn_stringbuf_t *path, svn_boolean_t use_tmp, apr_pool_t *pool, va_list ap) { const char *this; int components_added = 0; + int first_extension = 1; /* True until we've passed the first + extension component. */ + + /* Default to false; set to true later if appropriate. */ + if (is_base_p) + *is_base_p = 0; /* Tack on the administrative subdirectory. */ svn_path_add_component_nts (path, adm_subdir (), svn_path_local_style); @@ -102,6 +114,18 @@ if (this[0] == '\0') continue; + if (first_extension) + { + first_extension = 0; + + if (is_base_p + && (! use_tmp) + && (((strcmp (this, SVN_WC__ADM_TEXT_BASE)) == 0) + || ((strcmp (this, SVN_WC__ADM_PROP_BASE)) == 0) + || ((strcmp (this, SVN_WC__ADM_DIR_PROP_BASE)) == 0))) + *is_base_p = 1; + } + svn_path_add_component_nts (path, this, svn_path_local_style); components_added++; } @@ -112,7 +136,8 @@ /* See v_extend_with_adm_name() for details. */ static int -extend_with_adm_name (svn_stringbuf_t *path, +extend_with_adm_name (svn_boolean_t *is_base_p, + svn_stringbuf_t *path, svn_boolean_t use_tmp, apr_pool_t *pool, ...) @@ -121,7 +146,8 @@ int components_added; va_start (ap, pool); - components_added = v_extend_with_adm_name (path, + components_added = v_extend_with_adm_name (is_base_p, + path, use_tmp, pool, ap); @@ -141,7 +167,7 @@ va_list ap; va_start (ap, pool); - v_extend_with_adm_name (newpath, tmp, pool, ap); + v_extend_with_adm_name (NULL, newpath, tmp, pool, ap); va_end (ap); return newpath; @@ -159,7 +185,7 @@ va_list ap; va_start (ap, pool); - v_extend_with_adm_name (newpath, tmp, pool, ap); + v_extend_with_adm_name (NULL, newpath, tmp, pool, ap); va_end (ap); svn_io_check_path (newpath, &kind, pool); @@ -206,7 +232,7 @@ apr_status_t apr_err = 0; int components_added; - components_added = extend_with_adm_name (path, tmp, pool, thing, NULL); + components_added = extend_with_adm_name (NULL, path, tmp, pool, thing, NULL); if (type == svn_node_file) { @@ -302,6 +328,7 @@ /* Some code duplication with close_adm_file() seems unavoidable, given how C va_lists work. */ + svn_boolean_t is_base; svn_stringbuf_t *tmp_path = svn_stringbuf_dup (path, pool); apr_status_t apr_err; int components_added; @@ -309,31 +336,34 @@ /* Extend real name. */ va_start (ap, pool); - components_added = v_extend_with_adm_name (path, 0, pool, ap); + components_added = v_extend_with_adm_name (&is_base, path, 0, pool, ap); va_end (ap); /* Extend tmp name. */ va_start (ap, pool); - v_extend_with_adm_name (tmp_path, 1, pool, ap); + v_extend_with_adm_name (NULL, tmp_path, 1, pool, ap); va_end (ap); /* Rename. */ apr_err = apr_file_rename (tmp_path->data, path->data, pool); + if (is_base && (APR_STATUS_IS_SUCCESS (apr_err))) + SVN_ERR (svn_io_set_file_read_only (path->data, pool)); + /* Unconditionally restore path. */ chop_admin_name (path, components_added); - if (apr_err) + if (! (APR_STATUS_IS_SUCCESS (apr_err))) return svn_error_createf (apr_err, 0, NULL, pool, "error renaming %s to %s", tmp_path->data, path->data); - else + else return SVN_NO_ERROR; } /* Rename a tmp text-base file to its real text-base name. - The file had better already be closed. */ + The file should already be closed. */ svn_error_t * svn_wc__sync_text_base (svn_stringbuf_t *path, apr_pool_t *pool) { @@ -356,7 +386,8 @@ svn_stringbuf_t *newpath, *basename; svn_path_split (path, &newpath, &basename, svn_path_local_style, pool); - extend_with_adm_name (newpath, + extend_with_adm_name (NULL, + newpath, 0, pool, tmp ? SVN_WC__ADM_TMP : "", @@ -409,7 +440,8 @@ { *prop_path = svn_stringbuf_dup (path, pool); extend_with_adm_name - (*prop_path, + (NULL, + *prop_path, 0, pool, tmp ? SVN_WC__ADM_TMP : "", @@ -432,7 +464,8 @@ "svn_wc__prop_path: %s is not a working copy directory", (*prop_path)->data); - extend_with_adm_name (*prop_path, + extend_with_adm_name (NULL, + *prop_path, 0, pool, tmp ? SVN_WC__ADM_TMP : "", @@ -478,7 +511,8 @@ { *wcprop_path = svn_stringbuf_dup (path, pool); extend_with_adm_name - (*wcprop_path, + (NULL, + *wcprop_path, 0, pool, tmp ? SVN_WC__ADM_TMP : "", @@ -499,7 +533,8 @@ "wcprop_path: %s is not a working copy directory", (*wcprop_path)->data); - extend_with_adm_name (*wcprop_path, + extend_with_adm_name (NULL, + *wcprop_path, 0, pool, tmp ? SVN_WC__ADM_TMP : "", @@ -570,11 +605,11 @@ tmp_path = svn_stringbuf_dup (path, pool); va_start (ap, pool); - v_extend_with_adm_name (opath, 0, pool, ap); + v_extend_with_adm_name (NULL, opath, 0, pool, ap); va_end (ap); va_start (ap, pool); - v_extend_with_adm_name (tmp_path, 1, pool, ap); + v_extend_with_adm_name (NULL, tmp_path, 1, pool, ap); va_end (ap); /* Copy the original thing to the tmp location. */ @@ -586,7 +621,7 @@ /* Extend with tmp name. */ va_start (ap, pool); components_added - = v_extend_with_adm_name (path, 1, pool, ap); + = v_extend_with_adm_name (NULL, path, 1, pool, ap); va_end (ap); } else @@ -594,7 +629,7 @@ /* Extend with regular adm name. */ va_start (ap, pool); components_added - = v_extend_with_adm_name (path, 0, pool, ap); + = v_extend_with_adm_name (NULL, path, 0, pool, ap); va_end (ap); } @@ -628,13 +663,14 @@ apr_pool_t *pool, ...) { + svn_boolean_t is_base; apr_status_t apr_err = 0; int components_added; va_list ap; /* Get the full name of the thing we want. */ va_start (ap, pool); - components_added = v_extend_with_adm_name (path, sync, pool, ap); + components_added = v_extend_with_adm_name (NULL, path, sync, pool, ap); va_end (ap); apr_err = apr_file_close (fp); @@ -656,21 +692,24 @@ /* Extend real name. */ va_start (ap, pool); - components_added = v_extend_with_adm_name (path, 0, pool, ap); + components_added = v_extend_with_adm_name (&is_base, path, 0, pool, ap); va_end (ap); /* Extend tmp name. */ va_start (ap, pool); - v_extend_with_adm_name (tmp_path, 1, pool, ap); + v_extend_with_adm_name (NULL, tmp_path, 1, pool, ap); va_end (ap); /* Rename. */ apr_err = apr_file_rename (tmp_path->data, path->data, pool); + if (is_base && (APR_STATUS_IS_SUCCESS (apr_err))) + SVN_ERR (svn_io_set_file_read_only (path->data, pool)); + /* Unconditionally restore path. */ chop_admin_name (path, components_added); - if (apr_err) + if (! APR_STATUS_IS_SUCCESS (apr_err)) return svn_error_createf (apr_err, 0, NULL, pool, "error renaming %s to %s", tmp_path->data, path->data); @@ -944,7 +983,7 @@ va_list ap; va_start (ap, pool); - components_added = v_extend_with_adm_name (path, 0, pool, ap); + components_added = v_extend_with_adm_name (NULL, path, 0, pool, ap); va_end (ap); apr_err = apr_file_remove (path->data, pool); @@ -978,7 +1017,7 @@ /** Step 1: check that the directory exists. **/ - components_added = extend_with_adm_name (path, 0, pool, NULL); + components_added = extend_with_adm_name (NULL, path, 0, pool, NULL); err = svn_io_check_path (path, &kind, pool); if (!err) @@ -1041,7 +1080,7 @@ apr_status_t apr_err; int components_added; - components_added = extend_with_adm_name (path, 0, pool, NULL); + components_added = extend_with_adm_name (NULL, path, 0, pool, NULL); apr_err = apr_dir_make (path->data, APR_OS_DEFAULT, pool); if (apr_err) @@ -1272,7 +1311,7 @@ /* Get the path to the tmp area, and blow it away. */ tmp_path = svn_stringbuf_dup (path, pool); - extend_with_adm_name (tmp_path, 0, pool, SVN_WC__ADM_TMP, NULL); + extend_with_adm_name (NULL, tmp_path, 0, pool, SVN_WC__ADM_TMP, NULL); apr_err = apr_dir_remove_recursively (tmp_path->data, pool); if (apr_err) return svn_error_createf Index: ./subversion/libsvn_subr/io.c =================================================================== --- ./subversion/libsvn_subr/.svn/text-base/io.c Thu Oct 25 10:40:13 2001 +++ ./subversion/libsvn_subr/io.c Mon Oct 29 11:52:15 2001 @@ -488,6 +488,39 @@ +/*** Permissions and modes. ***/ + +svn_error_t * +svn_io_set_file_read_only (const char *fname, apr_pool_t *pool) +{ + apr_status_t apr_err; + apr_finfo_t finfo; + + apr_err = apr_stat (&finfo, fname, APR_FINFO_PROT, pool); + if (! (APR_STATUS_IS_SUCCESS (apr_err))) + return svn_error_createf + (apr_err, 0, NULL, pool, + "svn_io_set_file_read_only: problem stat'ing \"%s\"", fname); + + finfo.protection &= ~APR_UREAD; + finfo.protection &= ~APR_GREAD; + finfo.protection &= ~APR_WREAD; + + apr_err = apr_file_perms_set (fname, finfo.protection); + if ((! (APR_STATUS_IS_SUCCESS (apr_err))) + /* These two kinds of errors are okay, it just means the OS + doesn't support everything we're trying to do. */ + && (! (APR_STATUS_IS_INCOMPLETE (apr_err))) + && (! (APR_STATUS_IS_ENOTIMPL (apr_err)))) + return svn_error_createf + (apr_err, 0, NULL, pool, + "svn_io_set_file_read_only: problem setting \"%s\" read only", fname); + + return SVN_NO_ERROR; +} + + + /*** Generic streams. ***/ svn_stream_t *