You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/04/06 18:25:21 UTC

svn commit: r931209 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

Author: cmpilato
Date: Tue Apr  6 16:25:21 2010
New Revision: 931209

URL: http://svn.apache.org/viewvc?rev=931209&view=rev
Log:
Add compat code to work around pre-1.7 servers' messy handling of
mergeinfo paths.

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_get_mergeinfo): Strip leading slashes from catalog keys
    returned by the RA provider.

Modified:
    subversion/trunk/subversion/libsvn_ra/ra_loader.c

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=931209&r1=931208&r2=931209&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Tue Apr  6 16:25:21 2010
@@ -689,6 +689,8 @@ svn_error_t *svn_ra_get_mergeinfo(svn_ra
 {
   svn_error_t *err;
   int i;
+  apr_hash_index_t *hi;
+  svn_mergeinfo_catalog_t tmp_catalog;
 
   /* Validate path format. */
   for (i = 0; i < paths->nelts; i++)
@@ -705,9 +707,40 @@ svn_error_t *svn_ra_get_mergeinfo(svn_ra
       return err;
     }
 
-  return session->vtable->get_mergeinfo(session, catalog, paths,
-                                        revision, inherit,
-                                        include_descendants, pool);
+  SVN_ERR(session->vtable->get_mergeinfo(session, &tmp_catalog, paths,
+                                         revision, inherit,
+                                         include_descendants, pool));
+  
+  if (tmp_catalog == NULL)
+    {
+      *catalog = NULL;
+      return SVN_NO_ERROR;
+    }
+
+  /* Work around a bug in pre-1.7 servers that caused CATALOG's keys
+     to be a mix of absolute and relative paths (when they were all
+     supposed to be relative.  */
+  *catalog = apr_hash_make(pool);
+  for (hi = apr_hash_first(pool, tmp_catalog); hi; hi = apr_hash_next(hi))
+    {
+      const void *key;
+      apr_ssize_t klen;
+      void *val;
+      const char *path;
+
+      apr_hash_this(hi, &key, &klen, &val);
+      path = key;
+      if (path[0] == '/')
+        {
+          apr_hash_set(*catalog, path + 1, klen - 1, val);
+        }
+      else
+        {
+          apr_hash_set(*catalog, path, klen, val);
+        }
+    }
+
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *svn_ra_do_update2(svn_ra_session_t *session,



Re: svn commit: r931209 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Bert Huijben wrote:
>> Shouldn't we handle this in the specific RA layers instead of globally for all ra-layers?
>>
>> That would allow removing the code for future code paths (like we did for HTTPv2), while this code would hide the issues if we ever reintroduce the bug?
>>
>> I'm pretty sure you fixed ra_local, so that is at least one ra layer that doesn't need this fix.
> 
> So, I made this change with 1.6.x in mind.  1.6.x benefits universally from
> this change because a) there's only codepath to debug if something is wrong,
> b) 1.6.x code will never be able to optimize out the correction (because we
> won't likely teach it to recognize newer servers), and c) I'm not proposing
> the server fix for 1.6.x.
> 
> But sure, I can move the corrective behavior into specific RA layers for trunk.

r931581.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r931209 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
> 
>> -----Original Message-----
>> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
>> Sent: dinsdag 6 april 2010 18:25
>> To: commits@subversion.apache.org
>> Subject: svn commit: r931209 -
>> /subversion/trunk/subversion/libsvn_ra/ra_loader.c
>>
>> Author: cmpilato
>> Date: Tue Apr  6 16:25:21 2010
>> New Revision: 931209
>>
>> URL: http://svn.apache.org/viewvc?rev=931209&view=rev
>> Log:
>> Add compat code to work around pre-1.7 servers' messy handling of
>> mergeinfo paths.
>>
>> * subversion/libsvn_ra/ra_loader.c
>>   (svn_ra_get_mergeinfo): Strip leading slashes from catalog keys
>>     returned by the RA provider.
> 
> Shouldn't we handle this in the specific RA layers instead of globally for all ra-layers?
> 
> That would allow removing the code for future code paths (like we did for HTTPv2), while this code would hide the issues if we ever reintroduce the bug?
> 
> I'm pretty sure you fixed ra_local, so that is at least one ra layer that doesn't need this fix.

So, I made this change with 1.6.x in mind.  1.6.x benefits universally from
this change because a) there's only codepath to debug if something is wrong,
b) 1.6.x code will never be able to optimize out the correction (because we
won't likely teach it to recognize newer servers), and c) I'm not proposing
the server fix for 1.6.x.

But sure, I can move the corrective behavior into specific RA layers for trunk.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: svn commit: r931209 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> Sent: dinsdag 6 april 2010 18:25
> To: commits@subversion.apache.org
> Subject: svn commit: r931209 -
> /subversion/trunk/subversion/libsvn_ra/ra_loader.c
> 
> Author: cmpilato
> Date: Tue Apr  6 16:25:21 2010
> New Revision: 931209
> 
> URL: http://svn.apache.org/viewvc?rev=931209&view=rev
> Log:
> Add compat code to work around pre-1.7 servers' messy handling of
> mergeinfo paths.
> 
> * subversion/libsvn_ra/ra_loader.c
>   (svn_ra_get_mergeinfo): Strip leading slashes from catalog keys
>     returned by the RA provider.

Shouldn't we handle this in the specific RA layers instead of globally for all ra-layers?

That would allow removing the code for future code paths (like we did for HTTPv2), while this code would hide the issues if we ever reintroduce the bug?

I'm pretty sure you fixed ra_local, so that is at least one ra layer that doesn't need this fix.

	Bert