Jay Freeman (saurik) wrote:
>Branko:
>
>OK, here is the new version.
>
Jay, this is almost fine now. Please see the comments below:
>* ./subversion/include/svn_io.h (svn_io_set_file_read_write): Added
>prototype for a new svn_io_set_file_read_write() function that removes
>the read-only flag from the specified file. Changed prototype on
>existing svn_io_set_file_read_only() to take IGNORE_ENOENT, which asks
>the function to ignore file not found errors.
>
>* ./subversion/libsvn_wc/log.c (log_do_file_readonly): Updated usage of
>svn_io_set_file_read_only() to use the new IGNORE_ENOENT argument.
>
No need to be so verbose in the log. It's enough to say, e.g.,
(svn_io_set_file_read_only): Added new parameter IGNORE_ENOENT. All
callers changed.
Then you don't have to mention each call later on.
>(log_do_committed): Updated usage of svn_io_set_file_read_only() to use
>the new IGNORE_ENOENT argument.
>
>* ./subversion/libsvn_wc/adm_files.c (sync_adm_file): Updated usage of
>svn_io_set_file_read_only() to use the new IGNORE_ENOENT argument.
>
>(close_adm_file): When a file is overwritten by another file on Win32,
>it is important that the file not be read-only. Before performing the
>rename, close_adm_file() now removes the read-only flag on the
>destination. Also updated usage of svn_io_set_file_read_only() to use
>the new IGNORE_ENOENT argument.
>
>(svn_wc__remove_adm_file): On Win32, files that are marked read-only
>cannot be deleted. Before performing the delete,
>svn_wc__remove_adm_file now removes the read-only flag on the
>destination.
>
>* ./subversion/libsvn_wc/get_editor.c (svn_wc_install_file): There was a
>small bug where the fresh_keywords_value pointer was failed to be
>initialized to NULL.
>
This bit (plus the associated diff) really belongs to a separate patch.
One patch should concentrate on one change, for clarity.
(I'll go ahead and commit this change now, anyway.)
>* ./subversion/libsvn_subr/io.c (svn_io_set_file_read_only): Modified
>function to except the new IGNORE_ENOENT argument for ignoring errors
>related to files that aren't found.
>
>(svn_io_set_file_read_write): Implemented a new function that removes
>the read-only flag from the specified file using apr_file_attrs_set().
>
>
>
>Index: .\subversion\include\svn_io.h
>===================================================================
>--- .\subversion\include\svn_io.h
>+++ .\subversion\include\svn_io.h Mon Feb 11 15:36:38 2002
>@@ -142,7 +142,16 @@
> /* Make a file as read-only as the operating system allows.
> * PATH is the path to the file.
> */
>-svn_error_t *svn_io_set_file_read_only (const char *path, apr_pool_t
>*pool);
>+svn_error_t *svn_io_set_file_read_only (const char *path,
>+ svn_boolean_t ignore_enoent,
>+ apr_pool_t *pool);
>+
>+/* Make a file as writable as the operating system allows.
>+ * PATH is the path to the file.
>+ */
>+svn_error_t *svn_io_set_file_read_write (const char *path,
>+ svn_boolean_t ignore_enoent,
>+ apr_pool_t *pool);
>
>
The docstrings need fixing here, now that there's a new parameter.
>
>
> /* Read a line from FILE into BUF, but not exceeding *LIMIT bytes.
>Index: .\subversion\libsvn_wc\log.c
>===================================================================
>--- .\subversion\libsvn_wc\log.c
>+++ .\subversion\libsvn_wc\log.c Mon Feb 11 15:39:09 2002
>@@ -397,7 +397,7 @@
> full_path = svn_stringbuf_dup (loggy->path, loggy->pool);
> svn_path_add_component_nts (full_path, name);
>
>- SVN_ERR (svn_io_set_file_read_only (full_path->data, loggy->pool));
>+ SVN_ERR (svn_io_set_file_read_only (full_path->data, FALSE,
>loggy->pool));
>
> return SVN_NO_ERROR;
> }
>@@ -890,6 +890,7 @@
> prop_base_path->data);
>
> SVN_ERR (svn_io_set_file_read_only (prop_base_path->data,
>+ FALSE,
> loggy->pool));
> }
>
>Index: .\subversion\libsvn_wc\adm_files.c
>===================================================================
>--- .\subversion\libsvn_wc\adm_files.c
>+++ .\subversion\libsvn_wc\adm_files.c Mon Feb 11 15:43:54 2002
>@@ -329,7 +329,7 @@
> /* Rename. */
> apr_err = apr_file_rename (tmp_path->data, path->data, pool);
> if (APR_STATUS_IS_SUCCESS (apr_err))
>- SVN_ERR (svn_io_set_file_read_only (path->data, pool));
>+ SVN_ERR (svn_io_set_file_read_only (path->data, FALSE, pool));
>
> /* Unconditionally restore path. */
> chop_admin_name (path, components_added);
>@@ -681,10 +681,13 @@
> v_extend_with_adm_name (tmp_path, extension, 1, pool, ap);
> va_end (ap);
>
>+ /* Remove read-only flag on destination. */
>+ SVN_ERR(svn_io_set_file_read_write (path->data, TRUE, pool));
>+
> /* Rename. */
> apr_err = apr_file_rename (tmp_path->data, path->data, pool);
> if (APR_STATUS_IS_SUCCESS(apr_err))
>- SVN_ERR (svn_io_set_file_read_only (path->data, pool));
>+ SVN_ERR (svn_io_set_file_read_only (path->data, FALSE, pool));
>
> /* Unconditionally restore path. */
> chop_admin_name (path, components_added);
>@@ -998,6 +1001,9 @@
> va_start (ap, pool);
> components_added = v_extend_with_adm_name (path, NULL, 0, pool, ap);
> va_end (ap);
>+
>+ /* Remove read-only flag on path. */
>+ SVN_ERR(svn_io_set_file_read_write (path->data, FALSE, pool));
>
> apr_err = apr_file_remove (path->data, pool);
> if (apr_err)
>Index: .\subversion\libsvn_wc\get_editor.c
>===================================================================
>--- .\subversion\libsvn_wc\get_editor.c
>+++ .\subversion\libsvn_wc\get_editor.c Mon Feb 11 03:34:56 2002
>@@ -1524,7 +1524,7 @@
> {
> /* Did we get a new keywords value passed into this routine? */
> int i;
>- svn_stringbuf_t *fresh_keywords_value;
>+ svn_stringbuf_t *fresh_keywords_value = NULL;
>
> /* Rats, here's one case where it would be *nice* to have a
> hash instead of an array. */
>Index: .\subversion\libsvn_subr\io.c
>===================================================================
>--- .\subversion\libsvn_subr\io.c
>+++ .\subversion\libsvn_subr\io.c Mon Feb 11 15:36:42 2002
>@@ -351,7 +351,9 @@
> /*** Permissions and modes. ***/
>
> svn_error_t *
>-svn_io_set_file_read_only (const char *path, apr_pool_t *pool)
>+svn_io_set_file_read_only (const char *path,
>+ svn_boolean_t ignore_enoent,
>+ apr_pool_t *pool)
> {
> apr_status_t status;
> status = apr_file_attrs_set (path,
>@@ -359,8 +361,27 @@
> APR_FILE_ATTR_READONLY,
> pool);
> if (status && status != APR_ENOTIMPL)
>- return svn_error_createf (status, 0, NULL, pool,
>- "failed to set file '%s' read-only",
>path);
>+ if (!ignore_enoent || !APR_STATUS_IS_ENOENT(status))
>+ return svn_error_createf (status, 0, NULL, pool,
>+ "failed to set file '%s' read-only",
>path);
>+
>+ return SVN_NO_ERROR;
>+}
>+
>+svn_error_t *
>+svn_io_set_file_read_write (const char *path,
>+ svn_boolean_t ignore_enoent,
>+ apr_pool_t *pool)
>+{
>+ apr_status_t status;
>+ status = apr_file_attrs_set (path,
>+ 0,
>+ APR_FILE_ATTR_READONLY,
>+ pool);
>+ if (status && status != APR_ENOTIMPL)
>+ if (!ignore_enoent || !APR_STATUS_IS_ENOENT(status))
>+ return svn_error_createf (status, 0, NULL, pool,
>+ "failed to set file '%s' read-write",
>path);
>
> return SVN_NO_ERROR;
> }
>
And: Your mailer seems to wrap lines in the patch. Can you persuade it
not to?
Apart from that, it's fine. I hope you don't mind the nitpicking. :-)
--
Brane Čibej <brane_at_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
Received on Sat Oct 21 14:37:06 2006