[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

r40202 broke commit over ra_neon (Re: svn commit: r40202 - trunk/subversion/libsvn_ra_neon)

From: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 26 Oct 2009 13:57:11 +0100

On Fri, Oct 23, 2009 at 08:59:04AM -0700, Julian Foad wrote:
> Author: julianfoad
> Date: Fri Oct 23 08:59:03 2009
> New Revision: 40202
>
> Log:
> Use new dirent/URI/path functions to resolve some deprecation warnings.
> Also fix a bit of indentation.
>
> * subversion/libsvn_ra_neon/commit.c
> (get_version_url, create_activity, commit_delete_entry, commit_add_dir,
> commit_add_file, commit_close_file): Use `svn_path_url_add_component2()'.
> (add_child): Use `svn_path_url_add_component2()' and `svn_uri_join()'.
> (checkout_resource): Use `svn_relpath_local_style()'.
> (get_child_tokens): Use `svn_uri_is_child()'.

> @@ -325,8 +325,8 @@ static svn_error_t * create_activity(com
> the activity, and create the activity. The URL for our activity
> will be ACTIVITY_COLL/UUID */
> SVN_ERR(get_activity_collection(cc, &activity_collection, FALSE, pool));
> - url = svn_path_url_add_component(activity_collection->data,
> - uuid_buf, pool);
> + url = svn_path_url_add_component2(activity_collection->data,
> + uuid_buf, pool);
> SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
> "MKACTIVITY", url, NULL, NULL,
> 201 /* Created */,

I had to revert the above hunk in r40224, to fix the following
assertion failure during commit over ra_neon:

(gdb) bt
#0 0x0c264451 in kill () from /usr/lib/libc.so.51.1
#1 0x0c2bbdb3 in abort () at /usr/src/lib/libc/stdlib/abort.c:68
#2 0x0c243c5b in __assert2 (file=0x25d608c2 "subversion/libsvn_subr/path.c",
    line=100, func=0x25d60884 "svn_path_join",
    failedexpr=0x25d608a0 "svn_path_is_canonical(base, pool)")
    at /usr/src/lib/libc/gen/assert.c:52
#3 0x05d81be6 in svn_path_join (base=0x7ea78a48 "/repos/svn/!svn/act/",
    component=0x7ea78898 "bb3146ba-c229-11de-9977-6303f13ecd5f",
    pool=0x8b5bd018) at subversion/libsvn_subr/path.c:100
#4 0x05d83115 in svn_path_url_add_component2 (
    url=0x7ea78a48 "/repos/svn/!svn/act/",
    component=0x7ea78898 "bb3146ba-c229-11de-9977-6303f13ecd5f",
    pool=0x8b5bd018) at subversion/libsvn_subr/path.c:927
#5 0x01f331f6 in create_activity (cc=0x7ea78800, pool=0x8b5bd018)
    at subversion/libsvn_ra_neon/commit.c:328
#6 0x01f35400 in svn_ra_neon__get_commit_editor (session=0x8347da38,
    editor=0xcfbe6dcc, edit_baton=0xcfbe6dc8, revprop_table=0x7ea78670,
    callback=0x7a4dcf3 <svn_client__commit_callback>,
    callback_baton=0x7ea787f8, lock_tokens=0x8347d7a8, keep_locks=0,
    pool=0x8b5bd018) at subversion/libsvn_ra_neon/commit.c:1477
#7 0x06a1ce04 in svn_ra_get_commit_editor3 (session=0x8347da38,
    editor=0xcfbe6dcc, edit_baton=0xcfbe6dc8, revprop_table=0x7ea78670,
    callback=0x7a4dcf3 <svn_client__commit_callback>,
    callback_baton=0x7ea787f8, lock_tokens=0x8347d7a8, keep_locks=0,
    pool=0x8b5bd018) at subversion/libsvn_ra/ra_loader.c:586
#8 0x07a48d1e in get_ra_editor (ra_session=0xcfbe6dc4, editor=0xcfbe6dcc,
    edit_baton=0xcfbe6dc8, ctx=0x8a5dc718,
    base_url=0x8347d760 "https://svn.collab.net/repos/svn/trunk/subversion/libsvn_subr", base_dir=0x852ae280 "/home/stsp/svn/svn-trunk",
    log_msg=0x8347d610 "Resolve \"format not a string literal and no format arguments found\" warning.\n\n* subversion/libsvn_subr/io.c\n (do_io_file_wrapper_cleanup): Add the format specifier \"%s\", which\n fixes the warning.\n\nPa"..., commit_items=0x84361fb0, revprop_table=0x0, commit_info_p=0xcfbe6e30,
    is_commit=1, lock_tokens=0x8347d7a8, keep_locks=0, pool=0x8b5bd018)
    at subversion/libsvn_client/commit.c:641
#9 0x07a4a832 in svn_client_commit4 (commit_info_p=0xcfbe6e30,
    targets=0x852ad778, depth=svn_depth_infinity, keep_locks=0,
    keep_changelists=0, changelists=0x8a5dc160, revprop_table=0x0,
    ctx=0x8a5dc718, pool=0x8b5bd018) at subversion/libsvn_client/commit.c:1646
#10 0x1c006a49 in svn_cl__commit (os=0x8b5bd1c0, baton=0xcfbe6f1c,
    pool=0x8b5bd018) at subversion/svn/commit-cmd.c:119
#11 0x1c010925 in main (argc=4, argv=0xcfbe7110) at subversion/svn/main.c:2195
(gdb) up
#1 0x0c2bbdb3 in abort () at /usr/src/lib/libc/stdlib/abort.c:68
68 (void)kill(getpid(), SIGABRT);
(gdb) up
#2 0x0c243c5b in __assert2 (file=0x25d608c2 "subversion/libsvn_subr/path.c",
    line=100, func=0x25d60884 "svn_path_join",
    failedexpr=0x25d608a0 "svn_path_is_canonical(base, pool)")
    at /usr/src/lib/libc/gen/assert.c:52
52 abort();
(gdb) up
#3 0x05d81be6 in svn_path_join (base=0x7ea78a48 "/repos/svn/!svn/act/",
    component=0x7ea78898 "bb3146ba-c229-11de-9977-6303f13ecd5f",
    pool=0x8b5bd018) at subversion/libsvn_subr/path.c:100
100 assert(svn_path_is_canonical(base, pool));
(gdb)

The problem is that base has a trailing slash, so "/repos/svn/!svn/act/"
is not considered canonical.
The old version canonicalised the URL while the new version does not:

  const char *
  svn_path_url_add_component(const char *url,
                             const char *component,
                             apr_pool_t *pool)
  {
    /* URL can have trailing '/' */
    url = svn_path_canonicalize(url, pool);
  
    return svn_path_url_add_component2(url, component, pool);
  }

  const char *
  svn_path_url_add_component2(const char *url,
                              const char *component,
                              apr_pool_t *pool)
  {
    /* = svn_path_uri_encode() but without always copying */
    component = uri_escape(component, uri_char_validity, pool);
  
    return svn_path_join(url, component, pool);
  }

The docstring of the new version documents this difference, so this
was done on purpose.

To fix this properly, we need to canonicalise the base somewhere.
Please also check any other add_component->add_component2 upgrades
made in r40202 for similar issues.

Thanks,
Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411374
Received on 2009-10-26 13:57:55 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.