You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/08/08 21:27:53 UTC
Re: [PATCH] svn_wc_relocate3() on deleted target.
"Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
> How about this?
>
> [[[
>
> Fix the prolbem demonstrated below:
> svn co http://server/greek_tree wc
> cd wc/A
> svn rm D
> svn ci D -m 'rm D'
> svn sw --relocate from to D
>
> These results in exception:
> subversion/libsvn_wc/lock.c:997: (apr_err=155005)
> svn: Directory 'D/C' is missing
>
> * subversion/libsvn_wc/relocate.c
> (svn_wc_relocate3): Relocate deleted entry as a single entry.
>
> ]]]
Below is a new log message, followed by review:
> [[[
> Fix bug in which relocating a deleted entry would give an error.
>
> Patch by: Guo Rui <ti...@mail.ustc.edu.cn>
>
> Reproduction:
>
> $ svn co REPOS/greek_tree wc
> $ cd wc/A
> $ svn rm D
> $ svn ci D -m 'rm D'
> $ svn sw --relocate REPOS/greek_tree/A/D NEW_REPOS/greek_tree/A/D D
> subversion/libsvn_wc/lock.c:993: (apr_err=155005)
> svn: Directory 'D/B' is missing
> subversion/libsvn_wc/lock.c:993: (apr_err=155010)
> svn: Directory 'D/B' is missing
> $
>
> Fix:
>
> * subversion/libsvn_wc/relocate.c
> (svn_wc_relocate3): Handle a deleted entry as a single entry, the
> same way we handle a file.
> ]]]
Okay, I tested, and saw that the patch makes the error go away.
However, immediately after the switch, when I look at A/.svn/entries, I
see that the entry for "D" just says it's a "dir" and is "deleted".
There is no record of any URL change, and of course there is no
A/D/.svn/entries because there is no A/D/ anymore.
So I don't see any difference between handling deleted entries like
single entries, and simply skipping deleted entries entirely. Wouldn't
the end result be the same?
Another question is what happens if we switch A instead of D. I tried
that, both with and without your patch, and it behaved the same: the
switch succeeds, afterwards 'svn st wc' shows 'S wc/A', and the entry
for "D" in A/.svn/entries shows simply that it is deleted (which is
expected, but is also exactly the same is what happens when we
explicitly switched D itself!).
We could apply this patch; it passes 'make check'. But I'd like to
understand better why treating a deleted entry the same way we treat a
file is better than (or different than) just skipping the deleted entry
entirely.
Thanks,
-Karl
> Index: subversion/libsvn_wc/relocate.c
> ===================================================================
> --- subversion/libsvn_wc/relocate.c (revision 32410)
> +++ subversion/libsvn_wc/relocate.c (working copy)
> @@ -128,7 +128,11 @@
> if (! entry)
> return svn_error_create(SVN_ERR_ENTRY_NOT_FOUND, NULL, NULL);
>
> - if (entry->kind == svn_node_file)
> + if (entry->kind == svn_node_file
> + || (entry->deleted
> + && (! (entry->schedule == svn_wc_schedule_add
> + || entry->schedule == svn_wc_schedule_replace)))
> + || entry->absent)
> {
> SVN_ERR(relocate_entry(adm_access, entry, from, to,
> validator, validator_baton, TRUE /* sync */,
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn_wc_relocate3() on deleted target.
Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Fri, Aug 08, 2008 at 05:27:53PM -0400, Karl Fogel wrote:
> "Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
> > How about this?
> >
> > [[[
> >
> > Fix the prolbem demonstrated below:
> > svn co http://server/greek_tree wc
> > cd wc/A
> > svn rm D
> > svn ci D -m 'rm D'
> > svn sw --relocate from to D
> >
> > These results in exception:
> > subversion/libsvn_wc/lock.c:997: (apr_err=155005)
> > svn: Directory 'D/C' is missing
> >
> > * subversion/libsvn_wc/relocate.c
> > (svn_wc_relocate3): Relocate deleted entry as a single entry.
> >
> > ]]]
>
> Below is a new log message, followed by review:
>
> > [[[
> > Fix bug in which relocating a deleted entry would give an error.
> >
> > Patch by: Guo Rui <ti...@mail.ustc.edu.cn>
> >
> > Reproduction:
> >
> > $ svn co REPOS/greek_tree wc
> > $ cd wc/A
> > $ svn rm D
> > $ svn ci D -m 'rm D'
> > $ svn sw --relocate REPOS/greek_tree/A/D NEW_REPOS/greek_tree/A/D D
> > subversion/libsvn_wc/lock.c:993: (apr_err=155005)
> > svn: Directory 'D/B' is missing
> > subversion/libsvn_wc/lock.c:993: (apr_err=155010)
> > svn: Directory 'D/B' is missing
> > $
> >
> > Fix:
> >
> > * subversion/libsvn_wc/relocate.c
> > (svn_wc_relocate3): Handle a deleted entry as a single entry, the
> > same way we handle a file.
> > ]]]
>
> Okay, I tested, and saw that the patch makes the error go away.
> However, immediately after the switch, when I look at A/.svn/entries, I
> see that the entry for "D" just says it's a "dir" and is "deleted".
> There is no record of any URL change, and of course there is no
> A/D/.svn/entries because there is no A/D/ anymore.
>
> So I don't see any difference between handling deleted entries like
> single entries, and simply skipping deleted entries entirely. Wouldn't
> the end result be the same?
>
> Another question is what happens if we switch A instead of D. I tried
> that, both with and without your patch, and it behaved the same: the
> switch succeeds, afterwards 'svn st wc' shows 'S wc/A', and the entry
> for "D" in A/.svn/entries shows simply that it is deleted (which is
> expected, but is also exactly the same is what happens when we
> explicitly switched D itself!).
>
> We could apply this patch; it passes 'make check'. But I'd like to
> understand better why treating a deleted entry the same way we treat a
> file is better than (or different than) just skipping the deleted entry
> entirely.
>
Hi Karl. It's so good to see you are well again. Congratulation!
I did not know much or consider much about the inner logic of relocating an
deleted, absent or excluded entry. The starting point of this patch is simply
get rid of the error message in the situation of such kind of item is
explicitly specified in command line. You can see the code in the later
for-loop as a reference; it in fact does the same filtering on these kind of
entries (I find that a svn_wc_schedule_replace is also needed here).
In the design of subversion, a dir-kind entry only holds minimal information
and full information is only available in the this_dir entry for the sub-dir.
The resolve_to_defaults() function in entries.c will not fill in the missed
values for dir-kind entry. And later in the relocate_entry() function there is
no URL to modify for such kind of entry.
The server code explicitly prohibit reporting an disjointed path as excluded.
As a result, switching an excluded path is also explicitly prohibit. I think
similar logic may also apply to absent or deleted entry. In the situation of
relocation here, if relocate_entry() will always do nothing about hidden
entry, we should not include hidden entry in calls to svn_wc_entry() or
svn_wc_entries_read() at all. I found many suspicious show_hidden=true call in
the code base and marked them as TODO. I would like to turn off these
show_hidden flag and see if something goes wrong.
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org