Hi Stefan,
Are you sure there are no behavior changes by changing this?
If you copy nodes at every level the resulting nodes are the same as when
you only copy a root. (We had a lot of this in the wc-1.0 code). Tweaking
between copyfrom_* and copyroot_* is quite easy to get not exactly right,
without visible side effects. The repos_layer will hide a lot of those
differences to the client layer, while there can be quite a different
behavior in the storage layer.
With just the snippets in this thread I can't find a true answer and I need
more time to look at the actual change itself, to determine if there are
possible side effects.
Bert
From: Stefan Fuhrmann [mailto:[email protected]]
Sent: zondag 29 september 2013 18:39
To: Bill Tutt
Cc: Branko Čibej; Subversion Development
Subject: Re: Moves in FSFS
On Sun, Sep 29, 2013 at 2:08 AM, Bill Tutt <[email protected]
<mailto:[email protected]> > wrote:
On a slightly different note I noticed this oddity in lib_fs_fs/dag.c from
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?
annotate=1517479
703
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539
if (is_parent_copyroot)
704
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849531&r2=849532&> 849532
{
705
danielsh
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=1305395&r2=1305396&> 1305396
SVN_ERR(get_node_revision(&parent_noderev, parent));
706
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539
noderev->copyroot_rev = parent_noderev->copyroot_rev;
707
kfogel
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=858544&r2=858545&> 858545
noderev->copyroot_path = apr_pstrdup(pool,
708
parent_noderev->copyroot_path);
709
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849531&r2=849532&> 849532
}
710
hwright
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=866390&r2=866391&> 866391
711
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539
noderev->copyfrom_path = NULL;
712
noderev->copyfrom_rev = SVN_INVALID_REVNUM;
713
hwright
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=866390&r2=866391&> 866391
Sure looks like a useless memory allocation from the APR pool for
copyroot_path as well as dead code rotting away quietly.
As well as this earlier in the same file:
394
kfogel
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=858544&r2=858545&> 858545
new_noderev.copyroot_path = apr_pstrdup(pool,
395
parent_noderev->copyroot_path);
396
jpieper
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539
new_noderev.copyroot_rev = parent_noderev->copyroot_rev;
397
new_noderev.copyfrom_rev = SVN_INVALID_REVNUM;
398
new_noderev.copyfrom_path = NULL;
Wow, this has been here for ages... :)
Thanks Bill for spotting this. Fixed in r1527344.
-- Stefan^2.
