Re: [PATCH] v4 Fix #3390 Relative externals not updated during switch

On Mon, Dec 21, 2009 at 02:40:46PM +0000, Philip Martin wrote:
> Daniel Näslund <daniel_at_longitudo.com> writes:
>
> > + /* First try to get the associated url and if there is none as is the case
> > + * for exports then do use the user supplied values. */
> > + err = svn_wc__node_get_url(&url, cb->ctx->wc_ctx,
> > + abs_parent_dir, cb->pool, cb->pool);
> > +
> > + /* We want to check for just SVN_ERR_WC_PATH_NOT_FOUND here but since not
> > + * all callers use absolute paths at the moment we get assertions about
> > + * that, which we must catch. */
> > + if (err)
>
> It would be better to check explicitly for PATH_NOT_FOUND and whatever
> the error the assertion gives. Even better would be to fix the
> callers so that the assertions don't happen. The problem with your
> current code is that it's hiding problems, it means that we have to
> remember this code and come back and fix it.

I was wrong. It wasn't an assertion about abspaths. It was
SVN_ERR_WC_NOT_WORKING_COPY that I for some reason did not get right.
I've replaced the check for WC_PATH_NOT_FOUND with that one.

I admit - I've considered a patch finished when there was no failing
tests. But I'm trying to be better. My New Years resolution will be to
not consider a solution finished until I've understood every part of it.

make check passed.

[[[
Fix issue #3390, relative externals not updated during switch. As a side
effect - allow externals hash diff functionality to be used with abspaths.

* subversion/libsvn_client/switch.c
  (svn_client__switch_internal): Pass in an external_func to
    svn_wc_crawl_revision5().

* subversion/libsvn_client/externals.c
  (handle_externals_desc_change): Get the url for parent_dir with
    svn_wc__node_get_url(). Does not work for export where the
    parent_dir has no url. Then we resort to the old way of adding the
    parent_dir to the from_url.

* subversion/tests/cmdline/externals_tests.py
  (test_list): Remove XFail from switch_relative_external.

Patch by: Daniel Näslund <daniel{_AT_}longitudo.com>
Review by: stsp, philip
]]]

Daniel