You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2010/11/18 10:51:19 UTC
Re: [PATCH] Re: test failure 1.6.14 (svnsync test 29 with bdb;
copyfrom)
On Thu, Nov 18, 2010 at 06:34:30AM +0200, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Thu, Nov 18, 2010 at 06:07:07 +0200:
> > + SVN_ERR(svn_fs_node_history(&history, copyto_root, copyto_path, pool));
> > + SVN_ERR(svn_fs_history_prev(&history_prev, history, TRUE /* cross copies */,
> > + pool));
> > + SVN_ERR(svn_fs_history_location(copyfrom_path_p, copyfrom_rev_p,
> > + history_prev, pool));
>
> Tests running (trunk plus the patch).
>
>
> But perhaps I should have used this function instead:
>
> 1439 svn_error_t *
> 1440 svn_fs_copied_from(svn_revnum_t *rev_p,
> 1441 const char **path_p,
> 1442 svn_fs_root_t *root,
> 1443 const char *path,
> 1444 apr_pool_t *pool);
You mean like this? Seems to work here.
Index: subversion/libsvn_repos/replay.c
===================================================================
--- subversion/libsvn_repos/replay.c (revision 1036389)
+++ subversion/libsvn_repos/replay.c (working copy)
@@ -225,10 +225,14 @@ add_subdir(svn_fs_root_t *source_root,
continue;
else if (change->change_kind == svn_fs_path_change_replace)
{
- /* ### Can this assert fail? */
- SVN_ERR_ASSERT(change->copyfrom_known);
- copyfrom_path = change->copyfrom_path;
- copyfrom_rev = change->copyfrom_rev;
+ if (change->copyfrom_known)
+ {
+ copyfrom_path = change->copyfrom_path;
+ copyfrom_rev = change->copyfrom_rev;
+ }
+ else
+ SVN_ERR(svn_fs_copied_from(©from_rev, ©from_path,
+ target_root, new_path, pool));
}
}
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Thanks!
C. Michael Pilato wrote on Thu, Nov 18, 2010 at 10:29:23 -0500:
> On 11/18/2010 09:57 AM, Daniel Shahaf wrote:
> > C. Michael Pilato wrote on Thu, Nov 18, 2010 at 09:09:57 -0500:
>
> [...]
>
> >> Another subtlety. Say some other change is committed in our under '/Z' in
> >> r3 (but not to '/Z/B/E/alpha'):
> >>
> >> svn_fs_copied_from('/Z', r3) = NULL, SVN_INVALID_REVNUM
> >> svn_fs_copied_from('/Z/B/E/alpha', r3) = NULL, SVN_INVALID_REVNUM
> >>
> >> svn_fs_history_prev('/Z', r3) = '/Z', r2
> >> svn_fs_history_prev('/Z/B/E/alpha', r3) = '/Z/B/E/alpha', r2
> >>
> >
> > Wouldn't the results of the above four calls be the same regardless
> > of whether or not r3 touched /Z/B/E/alpha?
>
> Um... yes. :-)
>
> >> Notice also that svn_fs_history_prev() doesn't return information about the
> >> copy source either, because the previous "interesting history location" is
> >> the copy itself.
> >>
> >
> > Okay; in other words, svn_fs_history_prev() considers a path-revision
> > which are the *result* of a copy operation an "interesting" location.
>
> Exactly.
>
> > From this I understand that, if r20 did not touch '^/trunk', then
> > the two calls:
> > svn_fs_history_prev('^/branches/branch', r21)
> > svn_fs_history_prev('^/trunk', r20)
> > will behave identically.
>
> Yes.
>
> > On the other hand, in your example, I expect that svn_fs_copied_from()
> > will return /trunk@20?
>
> Yes. svn_fs_copied_from('^/branches/branch', r21) = '/trunk', r20
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/18/2010 09:57 AM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Thu, Nov 18, 2010 at 09:09:57 -0500:
[...]
>> Another subtlety. Say some other change is committed in our under '/Z' in
>> r3 (but not to '/Z/B/E/alpha'):
>>
>> svn_fs_copied_from('/Z', r3) = NULL, SVN_INVALID_REVNUM
>> svn_fs_copied_from('/Z/B/E/alpha', r3) = NULL, SVN_INVALID_REVNUM
>>
>> svn_fs_history_prev('/Z', r3) = '/Z', r2
>> svn_fs_history_prev('/Z/B/E/alpha', r3) = '/Z/B/E/alpha', r2
>>
>
> Wouldn't the results of the above four calls be the same regardless
> of whether or not r3 touched /Z/B/E/alpha?
Um... yes. :-)
>> Notice also that svn_fs_history_prev() doesn't return information about the
>> copy source either, because the previous "interesting history location" is
>> the copy itself.
>>
>
> Okay; in other words, svn_fs_history_prev() considers a path-revision
> which are the *result* of a copy operation an "interesting" location.
Exactly.
> From this I understand that, if r20 did not touch '^/trunk', then
> the two calls:
> svn_fs_history_prev('^/branches/branch', r21)
> svn_fs_history_prev('^/trunk', r20)
> will behave identically.
Yes.
> On the other hand, in your example, I expect that svn_fs_copied_from()
> will return /trunk@20?
Yes. svn_fs_copied_from('^/branches/branch', r21) = '/trunk', r20
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Thu, Nov 18, 2010 at 09:09:57 -0500:
> On 11/18/2010 09:02 AM, C. Michael Pilato wrote:
> > On 11/18/2010 08:43 AM, Daniel Shahaf wrote:
> >> Yes, that's what I meant, but I see that Philip already committed a fix.
> >>
> >> Question to FS people:
> >>
> >> Is the svn_fs_history_location(svn_fs_history_prev()) approach
> >> equivalent to the svn_fs_copied_from() approach?
> >
> > No.
> >
> > svn_fs_copied_from() will only return information about copies when you ask
> > it of the actual copy target.
> >
> > svn_fs_history_prev() will traverse history thru copies for any subtree of
> > the copy.
> >
> > For example, in our Greek tree, if r2 is a move of '/A' to '/Z', then:
> >
> > svn_fs_copied_from('/Z', r2) = '/A', r1
> >
> > svn_fs_copied_from('/Z/B/E/alpha', r2) = NULL, SVN_INVALID_REVNUM
> >
> > but:
> >
> > svn_fs_history_prev('/Z', r2) = '/A', r1
> >
> > svn_fs_history_prev('/Z/B/E/alpha', r2) = '/A/B/E/alpha', r1
>
Okay.
> Another subtlety. Say some other change is committed in our under '/Z' in
> r3 (but not to '/Z/B/E/alpha'):
>
> svn_fs_copied_from('/Z', r3) = NULL, SVN_INVALID_REVNUM
> svn_fs_copied_from('/Z/B/E/alpha', r3) = NULL, SVN_INVALID_REVNUM
>
> svn_fs_history_prev('/Z', r3) = '/Z', r2
> svn_fs_history_prev('/Z/B/E/alpha', r3) = '/Z/B/E/alpha', r2
>
Wouldn't the results of the above four calls be the same regardless
of whether or not r3 touched /Z/B/E/alpha?
> Notice that svn_fs_copied_from() doesn't see the copy because the
> node-revision for /Z@3 was not the exact target of a copy.
>
Understood.
> Notice also that svn_fs_history_prev() doesn't return information about the
> copy source either, because the previous "interesting history location" is
> the copy itself.
>
Okay; in other words, svn_fs_history_prev() considers a path-revision
which are the *result* of a copy operation an "interesting" location.
(So the first call will return the copy target, /Z@2, and only the next
call will return /A@rN with N<2.)
> Another nuance not demonstrated by my simple example is that
> svn_fs_history_prev() might not really return "the copy source", even when
> you query the exact copy target. Why? Because the source of the copy might
> not be an "interesting history location". Maybe you did 'svn copy
> ^/trunk@20 ^/branches/branch' (creating r21), but prior to your commit, the
> last time ^/trunk changed was r10. svn_fs_history_prev('^/branches/branch',
> r21) will return '^/trunk', r10, *not* '^/trunk', r20.
>
Okay; in other words, "being copied to" isn't a "interesting" history location.
>From this I understand that, if r20 did not touch '^/trunk', then
the two calls:
svn_fs_history_prev('^/branches/branch', r21)
svn_fs_history_prev('^/trunk', r20)
will behave identically.
On the other hand, in your example, I expect that svn_fs_copied_from()
will return /trunk@20?
(not because you said so, but because I expect /something/ to return /trunk@20)
> Confused yet?
>
I believe that I am not confused.
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
Thank you very much for the thorough explanations. :-)
Daniel
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Thu, Nov 18, 2010 at 09:09:57 -0500:
> On 11/18/2010 09:02 AM, C. Michael Pilato wrote:
> > On 11/18/2010 08:43 AM, Daniel Shahaf wrote:
> >> Yes, that's what I meant, but I see that Philip already committed a fix.
> >>
> >> Question to FS people:
> >>
> >> Is the svn_fs_history_location(svn_fs_history_prev()) approach
> >> equivalent to the svn_fs_copied_from() approach?
> >
> > No.
> >
> > svn_fs_copied_from() will only return information about copies when you ask
> > it of the actual copy target.
> >
> > svn_fs_history_prev() will traverse history thru copies for any subtree of
> > the copy.
> >
> > For example, in our Greek tree, if r2 is a move of '/A' to '/Z', then:
> >
> > svn_fs_copied_from('/Z', r2) = '/A', r1
> >
> > svn_fs_copied_from('/Z/B/E/alpha', r2) = NULL, SVN_INVALID_REVNUM
> >
> > but:
> >
> > svn_fs_history_prev('/Z', r2) = '/A', r1
> >
> > svn_fs_history_prev('/Z/B/E/alpha', r2) = '/A/B/E/alpha', r1
>
Okay.
> Another subtlety. Say some other change is committed in our under '/Z' in
> r3 (but not to '/Z/B/E/alpha'):
>
> svn_fs_copied_from('/Z', r3) = NULL, SVN_INVALID_REVNUM
> svn_fs_copied_from('/Z/B/E/alpha', r3) = NULL, SVN_INVALID_REVNUM
>
> svn_fs_history_prev('/Z', r3) = '/Z', r2
> svn_fs_history_prev('/Z/B/E/alpha', r3) = '/Z/B/E/alpha', r2
>
Wouldn't the results of the above four calls be the same regardless
of whether or not r3 touched /Z/B/E/alpha?
> Notice that svn_fs_copied_from() doesn't see the copy because the
> node-revision for /Z@3 was not the exact target of a copy.
>
Understood.
> Notice also that svn_fs_history_prev() doesn't return information about the
> copy source either, because the previous "interesting history location" is
> the copy itself.
>
Okay; in other words, svn_fs_history_prev() considers a path-revision
which are the *result* of a copy operation an "interesting" location.
(So the first call will return the copy target, /Z@2, and only the next
call will return /A@rN with N<2.)
> Another nuance not demonstrated by my simple example is that
> svn_fs_history_prev() might not really return "the copy source", even when
> you query the exact copy target. Why? Because the source of the copy might
> not be an "interesting history location". Maybe you did 'svn copy
> ^/trunk@20 ^/branches/branch' (creating r21), but prior to your commit, the
> last time ^/trunk changed was r10. svn_fs_history_prev('^/branches/branch',
> r21) will return '^/trunk', r10, *not* '^/trunk', r20.
>
Okay; in other words, "being copied to" isn't a "interesting" history location.
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/18/2010 09:02 AM, C. Michael Pilato wrote:
> On 11/18/2010 08:43 AM, Daniel Shahaf wrote:
>> Yes, that's what I meant, but I see that Philip already committed a fix.
>>
>> Question to FS people:
>>
>> Is the svn_fs_history_location(svn_fs_history_prev()) approach
>> equivalent to the svn_fs_copied_from() approach?
>
> No.
>
> svn_fs_copied_from() will only return information about copies when you ask
> it of the actual copy target.
>
> svn_fs_history_prev() will traverse history thru copies for any subtree of
> the copy.
>
> For example, in our Greek tree, if r2 is a move of '/A' to '/Z', then:
>
> svn_fs_copied_from('/Z', r2) = '/A', r1
>
> svn_fs_copied_from('/Z/B/E/alpha', r2) = NULL, SVN_INVALID_REVNUM
>
> but:
>
> svn_fs_history_prev('/Z', r2) = '/A', r1
>
> svn_fs_history_prev('/Z/B/E/alpha', r2) = '/A/B/E/alpha', r1
Another subtlety. Say some other change is committed in our under '/Z' in
r3 (but not to '/Z/B/E/alpha'):
svn_fs_copied_from('/Z', r3) = NULL, SVN_INVALID_REVNUM
svn_fs_copied_from('/Z/B/E/alpha', r3) = NULL, SVN_INVALID_REVNUM
svn_fs_history_prev('/Z', r3) = '/Z', r2
svn_fs_history_prev('/Z/B/E/alpha', r3) = '/Z/B/E/alpha', r2
Notice that svn_fs_copied_from() doesn't see the copy because the
node-revision for /Z@3 was not the exact target of a copy.
Notice also that svn_fs_history_prev() doesn't return information about the
copy source either, because the previous "interesting history location" is
the copy itself.
Another nuance not demonstrated by my simple example is that
svn_fs_history_prev() might not really return "the copy source", even when
you query the exact copy target. Why? Because the source of the copy might
not be an "interesting history location". Maybe you did 'svn copy
^/trunk@20 ^/branches/branch' (creating r21), but prior to your commit, the
last time ^/trunk changed was r10. svn_fs_history_prev('^/branches/branch',
r21) will return '^/trunk', r10, *not* '^/trunk', r20.
Confused yet?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn_fs_history_prev() and svn_fs_copied_from() question
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/18/2010 08:43 AM, Daniel Shahaf wrote:
> Yes, that's what I meant, but I see that Philip already committed a fix.
>
> Question to FS people:
>
> Is the svn_fs_history_location(svn_fs_history_prev()) approach
> equivalent to the svn_fs_copied_from() approach?
No.
svn_fs_copied_from() will only return information about copies when you ask
it of the actual copy target.
svn_fs_history_prev() will traverse history thru copies for any subtree of
the copy.
For example, in our Greek tree, if r2 is a move of '/A' to '/Z', then:
svn_fs_copied_from('/Z', r2) = '/A', r1
svn_fs_copied_from('/Z/B/E/alpha', r2) = NULL, SVN_INVALID_REVNUM
but:
svn_fs_history_prev('/Z', r2) = '/A', r1
svn_fs_history_prev('/Z/B/E/alpha', r2) = '/A/B/E/alpha', r1
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
svn_fs_history_prev() and svn_fs_copied_from() question (was: Re:
[PATCH] Re: test failure 1.6.14 (svnsync test 29 with bdb;
copyfrom))
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yes, that's what I meant, but I see that Philip already committed a fix.
Question to FS people:
Is the svn_fs_history_location(svn_fs_history_prev()) approach
equivalent to the svn_fs_copied_from() approach?
Stefan Sperling wrote on Thu, Nov 18, 2010 at 11:51:19 +0100:
> On Thu, Nov 18, 2010 at 06:34:30AM +0200, Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Thu, Nov 18, 2010 at 06:07:07 +0200:
> > > + SVN_ERR(svn_fs_node_history(&history, copyto_root, copyto_path, pool));
> > > + SVN_ERR(svn_fs_history_prev(&history_prev, history, TRUE /* cross copies */,
> > > + pool));
> > > + SVN_ERR(svn_fs_history_location(copyfrom_path_p, copyfrom_rev_p,
> > > + history_prev, pool));
> >
> > Tests running (trunk plus the patch).
> >
> >
> > But perhaps I should have used this function instead:
> >
> > 1439 svn_error_t *
> > 1440 svn_fs_copied_from(svn_revnum_t *rev_p,
> > 1441 const char **path_p,
> > 1442 svn_fs_root_t *root,
> > 1443 const char *path,
> > 1444 apr_pool_t *pool);
>
> You mean like this? Seems to work here.
>
> Index: subversion/libsvn_repos/replay.c
> ===================================================================
> --- subversion/libsvn_repos/replay.c (revision 1036389)
> +++ subversion/libsvn_repos/replay.c (working copy)
> @@ -225,10 +225,14 @@ add_subdir(svn_fs_root_t *source_root,
> continue;
> else if (change->change_kind == svn_fs_path_change_replace)
> {
> - /* ### Can this assert fail? */
> - SVN_ERR_ASSERT(change->copyfrom_known);
> - copyfrom_path = change->copyfrom_path;
> - copyfrom_rev = change->copyfrom_rev;
> + if (change->copyfrom_known)
> + {
> + copyfrom_path = change->copyfrom_path;
> + copyfrom_rev = change->copyfrom_rev;
> + }
> + else
> + SVN_ERR(svn_fs_copied_from(©from_rev, ©from_path,
> + target_root, new_path, pool));
> }
> }
>