Re: ra_serf not providing a valid revision to delete_entry() upon update

On Mon, Mar 19, 2012 at 3:48 PM, Hyrum K Wright
<hyrum.wright_at_wandisco.com> wrote:
> [ warning: investigation is still ongoing, but I thought I'd report this here.]
>
> I'm trying to debug the Ev2 shims over ra_dav.  In doing so, I've
> discovered an inconsistency between ra_serf and ra_neon (surprise!)
>
> ra_neon provides a valid argument for the replaced_rev parameter of
> the Ev1 delete_entry() function.  The relevant code is in
> libsvn_ra_neon/replay.c:
>
> 169         const char *path = svn_xml_get_attr_value("name", atts);
> 170         const char *crev = svn_xml_get_attr_value("rev", atts);
> 171
> 172         if (! path)
> 173           return MISSING_ATTR(nspace, elt_name, "name");
> 174         else if (! crev)
> 175           return MISSING_ATTR(nspace, elt_name, "rev");
> 176         else
> 177           {
> 178             dir_item_t *di = &TOP_DIR(rb);
> 179
> 180             SVN_ERR(rb->editor->delete_entry(path, SVN_STR_TO_REV(crev),
> 181                                              di->baton, di->pool));
> 182           }
> 183       }
> 184       break;
>
> Apparently, ra_neon expects and finds a "rev" attribute on the
> relevant XML element.  However, the same code in ra_serf (update.c)
> doesn't:
>
> 1556   else if ((state == OPEN_DIR || state == ADD_DIR) &&
> 1557            strcmp(name.name, "delete-entry") == 0)
> 1558     {
> 1559       const char *file_name;
> 1560       report_info_t *info;
> 1561       apr_pool_t *tmppool;
> 1562       const char *full_path;
> 1563
> 1564       file_name = svn_xml_get_attr_value("name", attrs);
> 1565
> 1566       if (!file_name)
> 1567         {
> 1568           return svn_error_create(
> 1569             SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
> 1570             _("Missing name attr in delete-entry element"));
> 1571         }
> 1572
> 1573       info = parser->state->private;
> 1574
> 1575       SVN_ERR(open_dir(info->dir));
> 1576
> 1577       tmppool = svn_pool_create(info->dir->dir_baton_pool);
> 1578
> 1579       full_path = svn_relpath_join(info->dir->name, file_name, tmppool);
> 1580
> 1581       SVN_ERR(info->dir->update_editor->delete_entry(full_path,
> 1582                                                      SVN_INVALID_REVNUM,
> 1583                                                      info->dir->dir_baton,
> 1584                                                      tmppool));
> 1585
> 1586       svn_pool_destroy(tmppool);
> 1587     }
>
> As you can see, ra_serf is unconditionally passing SVN_INVALID_REVNUM
> to delete_entry(), which is what is causing the shims to fail over
> ra_dav (when run with ra_serf).
>
> I tried inserting analogous code to fetch the "rev" attribute in
> ra_serf, but it appears that said attribute isn't available.  I'm not
> familiar enough with the protocol to know why it would be for ra_neon,
> but not ra_serf, and I'm asking here in the hopes that somebody can
> enlighten me. :)

For the record, the patch I tried was:

[[[
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 1302583)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -1557,11 +1557,13 @@
            strcmp(name.name, "delete-entry") == 0)
     {
       const char *file_name;
+ const char *crev;
       report_info_t *info;
       apr_pool_t *tmppool;
       const char *full_path;

       file_name = svn_xml_get_attr_value("name", attrs);
+ crev = svn_xml_get_attr_value("rev", attrs);

       if (!file_name)
         {
@@ -1579,7 +1581,7 @@
       full_path = svn_relpath_join(info->dir->name, file_name, tmppool);

       SVN_ERR(info->dir->update_editor->delete_entry(full_path,
- SVN_INVALID_REVNUM,
+ SVN_STR_TO_REV(crev),
                                                      info->dir->dir_baton,
                                                      tmppool));
 ]]]

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/