You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Paul Maier <sv...@web.de> on 2010/10/13 23:42:36 UTC

Bug report against SVN 1.6.13

Hi there! 
 
file b should be read-write here; what do you think: 
 
 
Reproduction script: 
-------------------- 
svn --version 
svnadmin create xx 
svn co "file:///C:/[...]/xx" yy 
cd yy 
echo a > a 
svn add a 
svn propset svn:needs-lock "*" a 
svn ci -m "" 
svn up 
svn cp a b 
ls -lA 
 
 
Observed behaviour: 
------------------- 
File b is read-only. 
No svn command is available to make file b read-write. 
svn lock won't do, because file b is not in the repository yet. 
Even a "svn lock a" before the copy doesn't help out. 
 
 
Expected behaviour: 
------------------- 
File b is read-write. 
Files without representation in the repository (files that come from a
uncommitted svn cp or svn mv) should be read-write regardless
svn:needs-lock setting. 
 
 
Full console output follows: 
---------------------------- 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue>svn --version 
svn, Version 1.6.13 (r1002816) 
   Übersetzt Oct  3 2010, 23:19:41 
[...] 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue>svnadmin create xx 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue>svn co "file:///C:/Dokumente und Einstellungen/Test2/Desktop/SvnIssue/xx" yy 
Ausgecheckt, Revision 0. 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue>cd yy 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>echo a  1>a 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>svn add a 
A         a 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>svn propset svn:needs-lock "*" a 
Eigenschaft "svn:needs-lock" für "a" gesetzt 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>svn ci -m "" 
Hinzufügen     a 
Übertrage Daten . 
Revision 1 übertragen. 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>svn up 
Revision 1. 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>svn cp a b 
A         b 
 
C:\Dokumente und Einstellungen\Test2\Desktop\SvnIssue\yy>ls -lA 
total 2 
drwxr-xr-x   6 administ Kein            0 Oct 11 20:10 .svn 
-r--r--r--   1 administ Kein            4 Oct 11 20:10 a 
-r--r--r--   1 administ Kein            4 Oct 11 20:10 b 
 
 
 
What do you think? 
 
With kind regards, 
  Paul 
 

Re: Bug report against SVN 1.6.13

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Sun, Oct 17, 2010 at 20:54:41 +0200:
> On Sun, Oct 17, 2010 at 04:30:41PM +0200, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Sun, Oct 17, 2010 at 14:43:57 +0200:
> > > Your patch seems to handle copies only. What about locally added files?
> > 
> > Does this part of the regression patch cover the scenario you have in mind?
> 
> Yup. It does cover it.
> 
> Does this test already pass?
> 

Yes.  Committed r1023571.  Thanks.

Daniel

Re: Bug report against SVN 1.6.13

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Oct 17, 2010 at 04:30:41PM +0200, Daniel Shahaf wrote:
> Stefan Sperling wrote on Sun, Oct 17, 2010 at 14:43:57 +0200:
> > Your patch seems to handle copies only. What about locally added files?
> 
> Does this part of the regression patch cover the scenario you have in mind?

Yup. It does cover it.

Does this test already pass?

>     Index: subversion/tests/cmdline/lock_tests.py
>     ===================================================================
>     --- subversion/tests/cmdline/lock_tests.py	(revision 1023400)
>     +++ subversion/tests/cmdline/lock_tests.py	(working copy)
>     @@ -1573,6 +1590,39 @@ def replace_and_propset_locked_path(sbox):
>     +def cp_isnt_ro(sbox):
>     ...
>     +  # 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)
>     ...
>     @@ -1619,6 +1669,7 @@ test_list = [ None,
>     +              cp_isnt_ro,
>            ]

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 06:11:56 +0200:
> 'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> > Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > > Or if a whole directory is copied (from wc or from URL) and our file
> > > is contained somewhere in the depth of that directory?
> > > 
> > 
> > Agreed, but haven't tested this.
> 
> This currently fails, presumably somewhere under repos_to_wc_copy_single()
> -> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
> latter function recently.  (also, at the moment that function isn't exactly
> the clearest piece of code we have)
> 
> So there's still work to be done on this part.

Summary:


In the directory case, repos_to_wc_copy_single() makes a temporary
wc (via svn_client__checkout_internal()) and then mass-adds it
into the wc (which is the temporary wc's parent) via svn_wc_add4().


The alternatives are to make the files non-read-only already in the
temporary checkout, or to fix them afterwards:

For the former option, need to follow the tracks from
svn_client__checkout_internal(), through 3-4 libsvn_client helper
functions, through to the libsvn_wc update editor; where, in
close_file(), all the file's properties are present, and a call to
svn_wc__wq_build_file_install() is made.  That call in turn calls
svn_wc__maybe_set_read_only(), which --- by examining STATUS, HAVE_BASE,
and HAVE_WORK from the svn_wc__db_read_info() call --- can determine
that the file in front of it does not have a repository node (crucially,
it does that by consulting with the PARENT wc, not with the temporary
sub-wc), and therefore shouldn't be mark read-only.

For the latter option, svn_wc_add4() (case #1) calls
svn_wc_copy3(metadata_only=TRUE), which could be taught somehow to fix
the permissions as it walks the tree.


To me, that seems like a fair amount of work for little gain.  And just
this archeology has exhausted me.  So I don't think I'll be implementing
this at the moment.  If someone cares enough about this, they can send
a patch (or suggest an easier implementation).


Lastly, much thanks to Philip Martin for his IRC help with the WC parts
of this (in particularly, the wc_db interfacing and observing that the
wc.db being used is the wrong one).


Daniel

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 06:11:56 +0200:
> 'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> > Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > > Or if a whole directory is copied (from wc or from URL) and our file
> > > is contained somewhere in the depth of that directory?
> > > 
> > 
> > Agreed, but haven't tested this.
> 
> This currently fails, presumably somewhere under repos_to_wc_copy_single()
> -> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
> latter function recently.  (also, at the moment that function isn't exactly
> the clearest piece of code we have)
> 
> So there's still work to be done on this part.

Summary:


In the directory case, repos_to_wc_copy_single() makes a temporary
wc (via svn_client__checkout_internal()) and then mass-adds it
into the wc (which is the temporary wc's parent) via svn_wc_add4().


The alternatives are to make the files non-read-only already in the
temporary checkout, or to fix them afterwards:

For the former option, need to follow the tracks from
svn_client__checkout_internal(), through 3-4 libsvn_client helper
functions, through to the libsvn_wc update editor; where, in
close_file(), all the file's properties are present, and a call to
svn_wc__wq_build_file_install() is made.  That call in turn calls
svn_wc__maybe_set_read_only(), which --- by examining STATUS, HAVE_BASE,
and HAVE_WORK from the svn_wc__db_read_info() call --- can determine
that the file in front of it does not have a repository node (crucially,
it does that by consulting with the PARENT wc, not with the temporary
sub-wc), and therefore shouldn't be mark read-only.

For the latter option, svn_wc_add4() (case #1) calls
svn_wc_copy3(metadata_only=TRUE), which could be taught somehow to fix
the permissions as it walks the tree.


To me, that seems like a fair amount of work for little gain.  And just
this archeology has exhausted me.  So I don't think I'll be implementing
this at the moment.  If someone cares enough about this, they can send
a patch (or suggest an easier implementation).


Lastly, much thanks to Philip Martin for his IRC help with the WC parts
of this (in particularly, the wc_db interfacing and observing that the
wc.db being used is the wrong one).


Daniel

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > svn cp svn://...../a b
> > should also leave file b as read-write, doesn't it?
> > 
> 
> Should, but doesn't.
> 

Fixed in r1023647, using the very same approach I originally
(unsuccessfully) tried for the previous patch :-)

Thanks for pointing out that the "source is URL to file" and "source is
URL to dir" cases should both be tested; in this case, they seem to
require separate fixes.

> > Or if a whole directory is copied (from wc or from URL) and our file
> > is contained somewhere in the depth of that directory?
> > 
> 
> Agreed, but haven't tested this.

This currently fails, presumably somewhere under repos_to_wc_copy_single()
-> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
latter function recently.  (also, at the moment that function isn't exactly
the clearest piece of code we have)

So there's still work to be done on this part.

Daniel


Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > svn cp svn://...../a b
> > should also leave file b as read-write, doesn't it?
> > 
> 
> Should, but doesn't.
> 

Fixed in r1023647, using the very same approach I originally
(unsuccessfully) tried for the previous patch :-)

Thanks for pointing out that the "source is URL to file" and "source is
URL to dir" cases should both be tested; in this case, they seem to
require separate fixes.

> > Or if a whole directory is copied (from wc or from URL) and our file
> > is contained somewhere in the depth of that directory?
> > 
> 
> Agreed, but haven't tested this.

This currently fails, presumably somewhere under repos_to_wc_copy_single()
-> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
latter function recently.  (also, at the moment that function isn't exactly
the clearest piece of code we have)

So there's still work to be done on this part.

Daniel

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > svn cp svn://...../a b
> > should also leave file b as read-write, doesn't it?
> > 
> 
> Should, but doesn't.
> 

Fixed in r1023647, using the very same approach I originally
(unsuccessfully) tried for the previous patch :-)

Thanks for pointing out that the "source is URL to file" and "source is
URL to dir" cases should both be tested; in this case, they seem to
require separate fixes.

> > Or if a whole directory is copied (from wc or from URL) and our file
> > is contained somewhere in the depth of that directory?
> > 
> 
> Agreed, but haven't tested this.

This currently fails, presumably somewhere under repos_to_wc_copy_single()
-> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
latter function recently.  (also, at the moment that function isn't exactly
the clearest piece of code we have)

So there's still work to be done on this part.

Daniel


Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
'Daniel Shahaf' wrote on Mon, Oct 18, 2010 at 00:24:13 +0200:
> Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> > svn cp svn://...../a b
> > should also leave file b as read-write, doesn't it?
> > 
> 
> Should, but doesn't.
> 

Fixed in r1023647, using the very same approach I originally
(unsuccessfully) tried for the previous patch :-)

Thanks for pointing out that the "source is URL to file" and "source is
URL to dir" cases should both be tested; in this case, they seem to
require separate fixes.

> > Or if a whole directory is copied (from wc or from URL) and our file
> > is contained somewhere in the depth of that directory?
> > 
> 
> Agreed, but haven't tested this.

This currently fails, presumably somewhere under repos_to_wc_copy_single()
-> svn_wc_add4().  I've pinged Julian on IRC, since he was refactoring the
latter function recently.  (also, at the moment that function isn't exactly
the clearest piece of code we have)

So there's still work to be done on this part.

Daniel

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> Hi Daniel,
> 
> thanks for having taken over this idea. 
> 

No problem.

> One question: 
> Does your solution code and testing code also cover the case when the copy source is a URL?
> 
> svn cp svn://...../a b
> should also leave file b as read-write, doesn't it?
> 

Should, but doesn't.

> Or if a whole directory is copied (from wc or from URL) and our file is contained somewhere in the depth of that directory?
> 

Agreed, but haven't tested this.

> Thanks & regards,
>   Paul
>  

Well, you know the drill...  Would you like to patch that yourself?  If
not, could you file an issue (please include a link to this thread and
add 'danielsh' to the CC list), and I or someone else will get to it.

Re: Bug report against SVN 1.6.13

Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Paul Maier wrote on Sun, Oct 17, 2010 at 22:30:23 +0200:
> Hi Daniel,
> 
> thanks for having taken over this idea. 
> 

No problem.

> One question: 
> Does your solution code and testing code also cover the case when the copy source is a URL?
> 
> svn cp svn://...../a b
> should also leave file b as read-write, doesn't it?
> 

Should, but doesn't.

> Or if a whole directory is copied (from wc or from URL) and our file is contained somewhere in the depth of that directory?
> 

Agreed, but haven't tested this.

> Thanks & regards,
>   Paul
>  

Well, you know the drill...  Would you like to patch that yourself?  If
not, could you file an issue (please include a link to this thread and
add 'danielsh' to the CC list), and I or someone else will get to it.


Re: Bug report against SVN 1.6.13

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Sun, Oct 17, 2010 at 14:43:57 +0200:
> Your patch seems to handle copies only. What about locally added files?

Does this part of the regression patch cover the scenario you have in mind?

    Index: subversion/tests/cmdline/lock_tests.py
    ===================================================================
    --- subversion/tests/cmdline/lock_tests.py	(revision 1023400)
    +++ subversion/tests/cmdline/lock_tests.py	(working copy)
    @@ -1573,6 +1590,39 @@ def replace_and_propset_locked_path(sbox):
    +def cp_isnt_ro(sbox):
    ...
    +  # 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)
    ...
    @@ -1619,6 +1669,7 @@ test_list = [ None,
    +              cp_isnt_ro,
           ]

Re: Bug report against SVN 1.6.13

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Oct 17, 2010 at 05:24:49AM +0200, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Sun, Oct 17, 2010 at 05:09:37 +0200:
> > 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;
> 
> Probably not a good idea to set_file_read_write(tmp_dst_abspath)
> before the check that it's non-NULL.  I'll have to look into that.

Thanks for looking into this, Daniel!

Your patch seems to handle copies only. What about locally added files?

Re: Bug report against SVN 1.6.13

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Sun, Oct 17, 2010 at 05:09:37 +0200:
> 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;

Probably not a good idea to set_file_read_write(tmp_dst_abspath)
before the check that it's non-NULL.  I'll have to look into that.

Re: Bug report against SVN 1.6.13

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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

Re: Bug report against SVN 1.6.13

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Oct 15, 2010 at 10:13:48PM +0200, Paul Maier wrote:
> Hi Stefan!
> 
> Thank you for having responded.
> 
> Of course, after the commit, the file is read-only.
> 
> Sorry. I don't know how to implement this. But it pains me a lot.

You'd need to be able to read and write C code.

> So I need to look for a crack, who is interested in implementing this?
> Or how is the procedure? Do I have to go through some steps to have
> my idea put into the issue tracker, and then wait until an
> implementor graps it?

You can file an issue describing your idea. Make sure to add a link
to the mailing list archives, so people who find the issue will find
this thread.

You can use this link:
http://svn.haxx.se/users/archive-2010-10/0228.shtml

Then you'll need to wait until someone decides to pick up the task,
if you cannot do it yourself.

Until then, you'll need to work around the problem by manually
marking such files read/write using operating system tools.

> Is it difficult to patch svn cp? How much time would YOU need?

I don't know how long it would take me. And I don't know if or
when I'll find the time to do it.

> 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.

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

> 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.

AW: Bug report against SVN 1.6.13

Posted by Paul Maier <sv...@web.de>.
Hi Stefan!

Thank you for having responded.

Of course, after the commit, the file is read-only.

Sorry. I don't know how to implement this. But it pains me a lot.

So I need to look for a crack, who is interested in implementing this?
Or how is the procedure? Do I have to go through some steps to have
my idea put into the issue tracker, and then wait until an
implementor graps it?

Is it difficult to patch svn cp? How much time would YOU need?

svn cp should always make the target file read-write in the
working copy.
Sounds like one command somewhere near the end of svn cp, isn't it?

What do you think?

Paul
  

> -----Ursprüngliche Nachricht-----
> Von: Stefan Sperling [mailto:stsp@elego.de] 
> Gesendet: Donnerstag, 14. Oktober 2010 12:02
> An: Paul Maier
> Cc: users@subversion.apache.org
> Betreff: Re: Bug report against SVN 1.6.13
> 
> On Thu, Oct 14, 2010 at 01:42:36AM +0200, Paul Maier wrote:
> > Hi there! 
> >  
> > file b should be read-write here; what do you think: 
> >  
> >  
> > Reproduction script: 
> > -------------------- 
> > svn --version 
> > svnadmin create xx 
> > svn co "file:///C:/[...]/xx" yy 
> > cd yy 
> > echo a > a 
> > svn add a 
> > svn propset svn:needs-lock "*" a 
> > svn ci -m "" 
> > svn up 
> > svn cp a b 
> > ls -lA 
> >  
> >  
> > Observed behaviour: 
> > ------------------- 
> > File b is read-only. 
> > No svn command is available to make file b read-write. 
> > svn lock won't do, because file b is not in the repository yet. 
> > Even a "svn lock a" before the copy doesn't help out. 
> >  
> >  
> > Expected behaviour: 
> > ------------------- 
> > File b is read-write. 
> > Files without representation in the repository (files that 
> come from a
> > uncommitted svn cp or svn mv) should be read-write regardless
> > svn:needs-lock setting. 
> 
> I'm fine with this idea if the file is also made read-only 
> after commit.
> 
> Would you like to try to write a patch against Subversion trunk that
> implements this feature? (I wouldn't really call it a "bug". A bug is
> when something doesn't work as designed. In this case, it seems like
> the designers of the lock feature didn't care or overlooked 
> this issue,
> so the desired behaviour is simply "unspecified".)
> 
> Stefan

Re: Bug report against SVN 1.6.13

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 14, 2010 at 01:42:36AM +0200, Paul Maier wrote:
> Hi there! 
>  
> file b should be read-write here; what do you think: 
>  
>  
> Reproduction script: 
> -------------------- 
> svn --version 
> svnadmin create xx 
> svn co "file:///C:/[...]/xx" yy 
> cd yy 
> echo a > a 
> svn add a 
> svn propset svn:needs-lock "*" a 
> svn ci -m "" 
> svn up 
> svn cp a b 
> ls -lA 
>  
>  
> Observed behaviour: 
> ------------------- 
> File b is read-only. 
> No svn command is available to make file b read-write. 
> svn lock won't do, because file b is not in the repository yet. 
> Even a "svn lock a" before the copy doesn't help out. 
>  
>  
> Expected behaviour: 
> ------------------- 
> File b is read-write. 
> Files without representation in the repository (files that come from a
> uncommitted svn cp or svn mv) should be read-write regardless
> svn:needs-lock setting. 

I'm fine with this idea if the file is also made read-only after commit.

Would you like to try to write a patch against Subversion trunk that
implements this feature? (I wouldn't really call it a "bug". A bug is
when something doesn't work as designed. In this case, it seems like
the designers of the lock feature didn't care or overlooked this issue,
so the desired behaviour is simply "unspecified".)

Stefan