On Fri, Apr 2, 2010 at 5:22 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Wed, Mar 31, 2010 at 4:29 PM, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> On Wed, Mar 31, 2010 at 1:01 PM, Hyrum K. Wright <
>> hyrum_wright_at_mail.utexas.edu> wrote:
>>
>>> 1.6.10 tarballs are up, the magic revision is r929659:
>>>
>>> http://orac.ece.utexas.edu/pub/svn/1.6.10/
>>>
>>> Download, test, sign and send your sigs back to me. (And don't even think
>>> about declaring this as "released" until I say so, for reasons I won't
>>> expound upon here.)
>>>
>>
>> Update: Somebody (read: me) didn't run the bindings tests before rolling the
>> tarball. There are a few binding test failures, one in the ruby bindings,
>> another couple in the JavaHL tests. We're investigating those (and help is
>> of course appreciated), but the upshot is: there may be a 1.6.11 shortly.
>
> The issue-3242 partial backport is responsible for the 3 JavaHL test failures.
>
> r879762 on the issue-3242 branch removed this session reparenting code
> from libsvn_client/mergeinfo.c:svn_client__get_repos_mergeinfo_catalog()...
>
> [[[
> {
> svn_error_t *err;
> svn_mergeinfo_t repos_mergeinfo;
> - const char *old_session_url;
> apr_array_header_t *rel_paths = apr_array_make(scratch_pool, 1,
> sizeof(rel_path));
>
> APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
>
> - /* Temporarily point the session at the root of the repository.
> -
> - ### BH: This is called from 'svn cp URL1 [URL2..] TOURL' and causes issue
> - #3242. As far as I can tell this is the only place in this
> - scenario that really needs access to the repository root instead
> - of the common parent. If there is any way to handle this via the
> - common parent, we should implement this here and we reduce the
> - problems caused by issue #3242. */
> - SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session,
> - NULL, scratch_pool));
> -
> /* Fetch the mergeinfo. */
> err = svn_ra_get_mergeinfo(ra_session, &repos_mergeinfo, rel_paths, rev,
> inherit, include_descendants, result_pool);
>
> ]]]
>
> ...and effectively moved it to the one caller of
> svn_client__get_repos_mergeinfo_catalog() that still needs access to
> the root of the repository: libsvn_client/mergeinfo.c:get_mergeinfo():
>
> [[[
> @@ -960,6 +956,7 @@
> {
> const char *repos_rel_path;
> const char *local_abspath;
> + const char *old_session_url;
>
> SVN_ERR(svn_dirent_get_absolute(&local_abspath, "", scratch_pool));
> SVN_ERR(svn_client__open_ra_session_internal(&ra_session, path_or_url,
> @@ -974,6 +971,8 @@
> FALSE, NULL,
> scratch_pool,
> scratch_pool));
> + SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session,
> + *repos_root, scratch_pool));
> SVN_ERR(svn_client__get_repos_mergeinfo_catalog(mergeinfo_catalog,
> ra_session,
> repos_rel_path, rev,
> ]]]
>
> The problem is, when the issue-3242-dev backport branch was created
> and r879762 backported to it, the changes to get_mergeinfo() were not
> included -- see
> http://svn.apache.org/viewvc/subversion/branches/1.6.x-issue-3242-partial/subversion/libsvn_client/mergeinfo.c?r1=916089&r2=916088&pathrev=916089.
>
> I assume this was mistake(?), the change getting lost in conflict resolution.
>
> With the remainder of rr879762 backported to mergeinfo.c like so,
>
> [[[
> Index: subversion/libsvn_client/mergeinfo.c
> ===================================================================
> --- subversion/libsvn_client/mergeinfo.c (revision 923779)
> +++ subversion/libsvn_client/mergeinfo.c (working copy)
> @@ -862,6 +862,7 @@
> if (svn_path_is_url(path_or_url))
> {
> const char *repos_rel_path;
> + const char *old_session_url;
>
> SVN_ERR(svn_client__open_ra_session_internal(&ra_session, path_or_url,
> NULL, NULL, NULL, FALSE,
> @@ -872,6 +873,8 @@
> SVN_ERR(svn_client__path_relative_to_root(&repos_rel_path, path_or_url,
> *repos_root, FALSE, NULL,
> NULL, subpool));
> + SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session,
> + *repos_root, subpool));
> SVN_ERR(svn_client__get_repos_mergeinfo(ra_session, mergeinfo,
> repos_rel_path, rev,
> svn_mergeinfo_inherited, FALSE,
>
>
> ]]]
>
> the JavaHL tests pass. Running the rest of the tests right now.
Created a backport branch for this missing piece of r879762 from
^/subversion/branches/issue-3242-dev:
http://svn.apache.org/viewvc?view=revision&revision=930865
http://svn.apache.org/viewvc?view=revision&revision=930880
Nominated it for backport as well.
~~~~~
While this only affects the JavaHL tests in our suite, you can
replicate the problem from the command line with a svn mergeinfo
subcommand that needs to find inherited mergeinfo on a URL, e.g.:
1.6.10-client>svn mergeinfo --show-revs merged
https://svn.apache.org/repos/asf/subversion/trunk/subversion
https://svn.apache.org/repos/asf/subversion/branches/1.6.x/subversion
svn: '/repos/asf/!svn/bc/930863/subversion/branches/1.6.x/subversion'
path not found
Paul
P.S. There are clearly some opportunities in this code to be more
conservative in requesting access to the repos root. If
REPOS_REL_PATH has explicit mergeinfo we only need access to that
path. Only if it has none and we need to find inherited mergeinfo do
we need access further up the repos tree...and even then we'd never
need access to the root (since how can it have an mergeinfo?).
Ideally we could check REPOS_REL_PATH for mergeinfo, if we found none
then reparent to REPOS_REL_PATH's immediate parent and check that, and
repeat as needed. Of course that is not going to do any favors to
performance. Regardless, these are changes that need to be made on
trunk; libsvn_client:mergeinfo.c:get_mergeinfo() on 1.6.x is
completely broken right now when operating on a URL.
Received on 2010-04-05 17:50:49 CEST