Re: Bug report against SVN 1.6.13

['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

['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

['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