[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn_io_file_rename preserves *destination* file permission on Windows

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2005-11-15 21:03:23 CET

On 11/15/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> On 11/15/05, Branko Čibej <brane@xbc.nu> wrote:
> > Ivan Zhakov wrote:
> >
> > >On 11/15/05, Erik Huelsmann <ehuels@gmail.com> wrote:
> > >
> > >
> > >>On 11/15/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> > >>
> > >>
> > >>>On 11/15/05, Branko Čibej <brane@xbc.nu> wrote:
> > >>>
> > >>>
> > >>>>Erik Huelsmann wrote:
> > >>>>
> > >>>>
> > >>>>>So, I suppose I should be testing trunk on a windows box and see what
> > >>>>>happens if I change svn_io_file_rename...
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>Something bad, I'm pretty sure, or I wouldn't have made the original
> > >>>>commit. :)
> > >>>>
> > >>>>
> > >>>I have just removed setting destination read-only after rename and all
> > >>>tests passes on Windows (except propt_tests.py 16 which is failling
> > >>>before change).
> > >>>
> > >>>
> > >>Could you reduce svn_wc__prep_file_for_replacement to 'return
> > >>SVN_NO_ERROR;' and see if tests still pass? (Yes, with the changed
> > >>svn_io_file_rename please.)
> > >>
> > >>Before deciding to really rip that specific part of the code, it would
> > >>be good to find out if any tests were created to test for the
> > >>semantics Branko is referring to (not necessarily asking you to do
> > >>that though).
> > >>
> > >>
> > >All tests pass with no-op svn_wc__prep_file_for_replacement and
> > >changed svn_io_file_rename. What do you think about this?
> > >
> > >
> > Please donćt even consider ripping that code out before you're 100% sure
> > it's not needed (any more). I certainly didn't make those changes for
> > fun. They were needed at the time.
> Of course, I am understand that you changed it for something. I want
> get to know for what this done. For present time I haven't ideas.

Considering what brane and I discussed elsewhere in this thread, I
propose to fix the bug in svn_io_file_rename with the following patch.
Could you test and tell me what your findings are?

Thanks!

Log:
[[[
Fix issue #2306.

Note:
  After applying this patch, svn_io_file_rename behaves the
  same on Windows and Unix: overwrite readonly files while retaining
  readonlyness on the source file.

* subversion/libsvn_subr/io.c
  (svn_io_file_rename): Retain readonlyness on the *source* instead of
  the target.

]]]

Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 17362)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -2484,14 +2484,14 @@
   svn_boolean_t was_read_only;
   apr_finfo_t finfo;

- SVN_ERR (svn_io_set_file_read_write (from_path, FALSE, pool));
+ SVN_ERR (svn_io_set_file_read_write (to_path, FALSE, pool));
 #endif /* WIN32 */

   SVN_ERR (svn_path_cstring_from_utf8 (&from_path_apr, from_path, pool));
   SVN_ERR (svn_path_cstring_from_utf8 (&to_path_apr, to_path, pool));

 #ifdef WIN32
- status = apr_stat (&finfo, to_path_apr, APR_FINFO_PROT, pool);
+ status = apr_stat (&finfo, from_path_apr, APR_FINFO_PROT, pool);
   if (APR_STATUS_IS_ENOENT (status))
     {
       was_read_only = FALSE;
@@ -2507,7 +2507,7 @@
       was_read_only = !(finfo.protection
                         & (APR_UWRITE | APR_GWRITE | APR_WWRITE));
       if (was_read_only)
- SVN_ERR (svn_io_set_file_read_write (to_path, FALSE, pool));
+ SVN_ERR (svn_io_set_file_read_write (from_path, FALSE, pool));
     }
 #endif /* WIN32 */
Received on Tue Nov 15 21:06:07 2005

This is an archived mail posted to the Subversion Dev mailing list.