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

Re: Bug report against SVN 1.6.13

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 17 Oct 2010 05:09:37 +0200

Stefan Sperling wrote on Sat, Oct 16, 2010 at 15:40:46 +0200:
> On Fri, Oct 15, 2010 at 10:13:48PM +0200, Paul Maier wrote:
> > svn cp should always make the target file read-write in the
> > working copy.
>
> No. A plain copy should carry file permissions of its source along,
> just like a normal (operating system) copy command does.
>

+1

> Only in the special case where an uncommitted file has an svn:needs-lock
> property it should be read-write, because:
> 1) There is no way to get the lock to make the file read-write,
> so there's no point in having the file read-only
> 2) Noone else can see the file yet, anyway
>

+1

> > Sounds like one command somewhere near the end of svn cp, isn't it?
>
> I'd poke around in the code to see where the svn:needs-lock property
> is handled, and try to add a special case there for files which are
> locally added or copied.

I looked at svn_wc__maybe_set_read_only(), discovered it wasn't even
called during 'svn cp', and came up with the following patch:

[[[
Index: subversion/tests/cmdline/lock_tests.py
===================================================================
--- subversion/tests/cmdline/lock_tests.py (revision 1023400)
+++ subversion/tests/cmdline/lock_tests.py (working copy)
@@ -37,6 +37,23 @@ XFail = svntest.testcase.XFail
 Item = svntest.wc.StateItem
 
 ######################################################################
+# Helpers
+
+def check_writability(path, writable)
+ "Raise if PATH is not writable."
+ bits = stat.S_IWGRP | stat.S_IWOTH | stat.S_IWRITE
+ mode = os.stat(path)[0]
+ if bool(mode & bits) != writable:
+ raise svntest.Failure("path '%s' is unexpectedly %s (mode %o)"
+ % (path, ["writable", "read-only"][writable], mode))
+
+def is_writable(path):
+ check_writability(path, True)
+
+def is_readonly(path):
+ check_writability(path, False)
+
+######################################################################
 # Tests
 
 #----------------------------------------------------------------------
@@ -1573,6 +1590,39 @@ def replace_and_propset_locked_path(sbox):
                                      'commit', '-m', '', G_path)
 
 
+#----------------------------------------------------------------------
+def cp_isnt_ro(sbox):
+ "uncommitted svn:needs-lock add/cp not read-only"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+ mu2_path = os.path.join(wc_dir, 'A', 'mu2')
+ kappa_path = os.path.join(wc_dir, 'kappa')
+ open(kappa_path, 'w').write("This is the file 'kappa'.\n")
+
+ # added file
+ sbox.simple_add(kappa_path)
+ svntest.actions.set_prop('svn:needs-lock', 'yes', kappa_path)
+ is_writable(kappa_path)
+ sbox.simple_commit(kappa_path)
+ is_readonly(kappa_path)
+
+ # versioned file
+ svntest.actions.set_prop('svn:needs-lock', 'yes', mu_path)
+ is_writable(mu_path)
+ sbox.simple_commit(mu_path)
+ is_readonly(mu_path)
+
+ # added-with-history file
+ svntest.actions.set_prop('svn:needs-lock', 'yes', mu_path)
+ svntest.main.run_svn(None, 'copy', mu_path, mu2_path)
+ is_writable(mu2_path)
+ sbox.simple_commit(mu2_path)
+ is_readonly(mu2_path)
+
+
 ########################################################################
 # Run the tests
 
@@ -1619,6 +1669,7 @@ test_list = [ None,
               verify_path_escaping,
               XFail(replace_and_propset_locked_path,
                     svntest.main.is_ra_type_dav),
+ cp_isnt_ro,
             ]
 
 if __name__ == '__main__':
Index: subversion/libsvn_wc/copy.c
===================================================================
--- subversion/libsvn_wc/copy.c (revision 1023400)
+++ subversion/libsvn_wc/copy.c (working copy)
@@ -238,6 +238,17 @@ copy_versioned_file(svn_wc__db_t *db,
                              tmpdir_abspath,
                              TRUE, /* recursive */
                              cancel_func, cancel_baton, scratch_pool));
+
+ /* Remove 'read-only' from the copied file. */
+ {
+ const svn_string_t *needs_lock;
+ SVN_ERR(svn_wc__internal_propget(&needs_lock, db, src_abspath,
+ SVN_PROP_NEEDS_LOCK, scratch_pool,
+ scratch_pool));
+ if (needs_lock)
+ svn_io_set_file_read_write(tmp_dst_abspath, FALSE, scratch_pool);
+ }
+
       if (tmp_dst_abspath)
         {
           svn_skel_t *work_item;
]]]

Please review the regression test :-)

Thanks,

Daniel
Received on 2010-10-17 05:11:07 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.