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

[PATCH] have ra_dav understand new prop namespaces

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2002-08-07 20:47:15 CEST

Here's my patch for #840; I've run out of time before having to leave
for the rest of the week. :-(

The patch is "finished" AFAICT, but I'm getting weird regression bugs
in basic_tests.py #10 and #12 over DAV. cmpilato has graciously
agreed to review it and debug it for me when I'm gone. The hope here
is that this patch makes it into 0.14.1, so that we can get the
intelligent clients "out there" as early as possible.

(Looking at the ethereal log on basic_test #10, it seems that the
client does a PROPFIND to get a baseline collection, and then
immediately turns around and tries to GET the baseline collection! I
have a feeling this is related to my seemingly small changes to
_get_baseline_info()... like somehow the relative path isn't being
tacked on to the GET of the bc or something. Not sure.)

One other note: ideally, somebody should write a mod_dav_svn patch
that *sends* the new property namespaces, just to make sure that this
ra_dav patch works with new as well as old! We'd then shelf the
mod_dav_svn patch for a month or so.

---------------------------------------------------------------------

Implement the first half of issue #840: teach ra_dav to understand
both old and new property namespaces.

* svn_dav.h (SVN_DAV_PROP_NS_SVN, SVN_DAV_PROP_NS_DAV,
  SVN_DAV_PROP_NS_CUSTOM): define three new property namespaces.

* ra_dav.h (SVN_RA_DAV__PROP_BASELINE_RELPATH_OLD,
  SVN_RA_DAV__PROP_BASELINE_RELPATH): define an "old" version of the
  baseline-relative-path property, to contrast against the correct,
  new one. It turns out that this is actually the *only* renegade
  network-generated property that is "polluting" the svn: namespace.

  (ELEM_baseline_relpath_old): new enum.

* props.c (elem_definitions, neon_descriptions): mention old and new
  relpath props in these lists.

  (validate_element): allow either old- or new- style baseline_relpath.

  (starting_props): try to fetch *both* old and new baseline_relpath props.

  (process_results, add_to_hash): when parsing a multistatus PROPFIND
  response, only store the properties that were actually found. At
  this point, we know that at least one of the baseline_relpath props
  will be a 404, and that's totally fine.

  (svn_ra_dav__get_baseline_info): try to pull the new-style
  baseline-relpath property from the hash; if that fails, query for
  the old-style one instead. Only die if both are unavailable.

* fetch.c (add_props): this is the main filter that pushes props at
  libsvn_wc. Recognize both old and new 'custom' namespaces.
  Recognize both old and new 'svn' namespaces, filtering out the
  baseline-rel-path only in the old namespace.

  (svn_ra_dav__get_file, svn_ra_dav__get_dir): apply the same logic
  here. This fixes a huge bug: can you believe that these functions
  were completely ignoring 'svn:' properties up till now?! Oy. This
  might fix issue #591. Also, while we're here, fix the bug in 'svn
  ls', whereby __get_dir() wasn't looking in the correct namespaces to
  decide if an entry had props or not.

Index: ./subversion/include/svn_dav.h
===================================================================
--- ./subversion/include/svn_dav.h
+++ ./subversion/include/svn_dav.h Tue Aug 6 19:45:54 2002
@@ -49,6 +49,24 @@
 #define SVN_DAV_ERROR_TAG "error"
 
 
+
+/* General property (xml) namespaces that will be used by both ra_dav
+ and mod_dav_svn for marshalling properties. */
+
+/* A property stored in the fs and wc, begins with 'svn:', and is
+ interpreted either by client or server. */
+#define SVN_DAV_PROP_NS_SVN "http://subversion.tigris.org/xmlns/svn"
+
+/* A property stored in the fs and wc, but totally ignored by svn
+ client and server. (Simply invented by the users.) */
+#define SVN_DAV_PROP_NS_CUSTOM "http://subversion.tigris.org/xmlns/custom"
+
+/* A property purely generated and consumed by the network layer, not
+ seen by either fs or wc. */
+#define SVN_DAV_PROP_NS_DAV "http://subversion.tigris.org/xmlns/dav"
+
+
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: ./subversion/libsvn_ra_dav/ra_dav.h
===================================================================
--- ./subversion/libsvn_ra_dav/ra_dav.h
+++ ./subversion/libsvn_ra_dav/ra_dav.h Tue Aug 6 19:55:23 2002
@@ -199,9 +199,15 @@
 #define SVN_RA_DAV__PROP_CREATOR_DISPLAYNAME "DAV:creator-displayname"
 #define SVN_RA_DAV__PROP_GETCONTENTLENGTH "DAV:getcontentlength"
 
-#define SVN_RA_DAV__PROP_BASELINE_RELPATH \
+/* ### REMOVE ME SOMEDAY: Here for backward compatibility */
+#define SVN_RA_DAV__PROP_BASELINE_RELPATH_OLD \
     SVN_PROP_PREFIX "baseline-relative-path"
 
+#define SVN_RA_DAV__PROP_BASELINE_RELPATH \
+ SVN_DAV_PROP_NS_DAV "baseline-relative-path"
+
+
+
 typedef struct {
   /* what is the URL for this resource */
   const char *url;
@@ -357,7 +363,8 @@
   /* SVN elements */
   ELEM_add_directory,
   ELEM_add_file,
- ELEM_baseline_relpath,
+ ELEM_baseline_relpath,
+ ELEM_baseline_relpath_old, /* ### REMOVE ME someday. */
   ELEM_deleted_path, /* used in log reports */
   ELEM_replaced_path, /* used in log reports */
   ELEM_added_path, /* used in log reports */
Index: ./subversion/libsvn_ra_dav/props.c
===================================================================
--- ./subversion/libsvn_ra_dav/props.c
+++ ./subversion/libsvn_ra_dav/props.c Wed Aug 7 12:44:36 2002
@@ -32,6 +32,7 @@
 #include "svn_delta.h"
 #include "svn_ra.h"
 #include "svn_path.h"
+#include "svn_dav.h"
 
 #include "ra_dav.h"
 
@@ -61,6 +62,8 @@
 
   /* SVN elements */
   { ELEM_baseline_relpath, SVN_RA_DAV__PROP_BASELINE_RELPATH, 1 },
+ /* ### REMOVE the old version of this someday: */
+ { ELEM_baseline_relpath_old, SVN_RA_DAV__PROP_BASELINE_RELPATH_OLD, 1 },
 
   { 0 }
 };
@@ -78,7 +81,10 @@
   { "DAV:", "getcontentlength", ELEM_get_content_length, NE_XML_CDATA },
 
   /* SVN elements */
- { SVN_PROP_PREFIX, "baseline-relative-path", ELEM_baseline_relpath,
+ { SVN_DAV_PROP_NS_DAV, "baseline-relative-path", ELEM_baseline_relpath,
+ NE_XML_CDATA },
+ /* ### REMOVE this someday: */
+ { SVN_PROP_PREFIX, "baseline-relative-path", ELEM_baseline_relpath_old,
     NE_XML_CDATA },
 
   { NULL }
@@ -101,6 +107,7 @@
 {
   { "DAV:", "version-controlled-configuration" },
   { SVN_PROP_PREFIX, "baseline-relative-path" },
+ { SVN_DAV_PROP_NS_DAV, "baseline-relative-path" },
   { "DAV:", "resourcetype" },
   { NULL }
 };
@@ -170,6 +177,12 @@
 {
   svn_ra_dav_resource_t *r = userdata;
   const char *name;
+
+ if (value == NULL)
+ /* According to neon's docstrings, this means that there was an
+ error fetching this property. We don't care about the exact
+ error status code, though. */
+ return 0;
   
   name = apr_pstrcat(r->pool, pname->nspace, pname->name, NULL);
   value = apr_pstrdup(r->pool, value);
@@ -186,8 +199,7 @@
   /* prop_ctx_t *pc = userdata; */
   svn_ra_dav_resource_t *r = ne_propset_private(rset);
 
- /* ### should use ne_propset_status(rset) to determine whether the
- * ### PROPFIND failed for the properties we're interested in. */
+ /* Only call iterate() on the 200-status properties. */
   (void) ne_propset_iterate(rset, add_to_hash, r);
 }
 
@@ -200,6 +212,7 @@
           {
           case ELEM_baseline_coll:
           case ELEM_baseline_relpath:
+ case ELEM_baseline_relpath_old: /* ### REMOVE ME someday */
           case ELEM_checked_in:
           case ELEM_resourcetype:
           case ELEM_vcc:
@@ -553,11 +566,19 @@
                                  APR_HASH_KEY_STRING);
     if (relative_path == NULL)
       {
- /* ### better error reporting... */
- /* ### need an SVN_ERR here */
- return svn_error_create(APR_EGENERAL, 0, NULL, pool,
- "The relative-path property was not "
- "found on the resource.");
+ /* ### REMOVE ME someday: */
+ relative_path = apr_hash_get(rsrc->propset,
+ SVN_RA_DAV__PROP_BASELINE_RELPATH_OLD,
+ APR_HASH_KEY_STRING);
+
+ if (relative_path == NULL)
+ {
+ /* ### better error reporting... */
+ /* ### need an SVN_ERR here */
+ return svn_error_create(APR_EGENERAL, 0, NULL, pool,
+ "The relative-path property was not "
+ "found on the resource.");
+ }
       }
     
     /* don't forget to tack on the parts we lopped off in order
Index: ./subversion/libsvn_ra_dav/fetch.c
===================================================================
--- ./subversion/libsvn_ra_dav/fetch.c
+++ ./subversion/libsvn_ra_dav/fetch.c Wed Aug 7 12:44:24 2002
@@ -374,13 +374,28 @@
       char *val;
 
       apr_hash_this(hi, (const void **)&key, NULL, (void *)&val);
-
+
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_CUSTOM) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_CUSTOM, NSLEN) == 0)
+ {
+ /* for props in the 'custom' namespace, we strip the
+ namespace and just use whatever name the user gave the
+ property. */
+
+ svn_stringbuf_set(skey, key + NSLEN);
+
+ /* ### urk. this value isn't binary-safe... */
+ svn_stringbuf_set(sval, val);
+
+ (*setter)(baton, skey, sval);
+ }
+#undef NSLEN
 #define NSLEN (sizeof(SVN_PROP_CUSTOM_PREFIX) - 1)
       if (strncmp(key, SVN_PROP_CUSTOM_PREFIX, NSLEN) == 0)
         {
- /* for custom props, we strip the namespace, and just use whatever
- name the user gave the property. */
-
+ /* ### Backwards compatibility: look for old 'svn:custom:'
+ namespace and strip it away, instead of the good URI
+ namespace. REMOVE this block someday! */
           svn_stringbuf_set(skey, key + NSLEN);
 
           /* ### urk. this value isn't binary-safe... */
@@ -389,14 +404,30 @@
           (*setter)(baton, skey, sval);
         }
 #undef NSLEN
+
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_SVN) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_SVN, NSLEN) == 0)
+ {
+ /* This property is an 'svn:' prop, recognized by client, or
+ server, or both. Convert the URI namespace into normal
+ 'svn:' prefix again before pushing it at the wc.*/
+ svn_stringbuf_set(skey, "svn:");
+ svn_stringbuf_appendcstr (skey, key + NSLEN);
+
+ /* ### urk. this value isn't binary-safe... */
+ svn_stringbuf_set(sval, val);
+
+ (*setter)(baton, skey, sval);
+ }
+#undef NSLEN
 #define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
       else if (strncmp(key, SVN_PROP_PREFIX, NSLEN) == 0)
         {
- /* this is one of our properties. pass it straight through. */
-
- /* ### oops. watch out for props that the server sets, which we
- ### don't want reflected in the WC. we should put these into
- ### a server-prop namespace. */
+ /* ### Backwards compatibility: if the property is already
+ in the deprecated 'svn:' namespace, pass it straight
+ through without change. But filter out the
+ baseline-relative-path property. REMOVE this block
+ someday.*/
           if (strcmp(key + NSLEN, "baseline-relative-path") == 0)
             continue;
 
@@ -411,11 +442,11 @@
       else
         {
           /* If we get here, then we have a property that is neither
- in the 'svn:custom:' space, nor in the plain old 'svn:'
- space. There are a few properties like this. Some of
- them, like 'checked-in', 'version-controlled-configuration',
- and 'getetag' seem to be handled in other places
- (get_vsn_url()). For all others, we filter them here. */
+ in the 'custom' space, nor in the 'svn' space. So it
+ must be either in the 'network' space or 'DAV:' space.
+ The following routine converts a handful of DAV: props
+ into 'svn:wc:' or 'svn:entry:' props that libsvn_wc
+ wants. */
           set_special_wc_prop (key, val, setter, baton, pool);
         }
     }
@@ -919,15 +950,52 @@
 
           apr_hash_this(hi, &key, NULL, &val);
 
- /* If the property starts with "svn:custom:", then it's a
+ /* If the property is in the 'custom' namespace, then it's a
              normal user-controlled property coming from the fs. Just
              strip off this prefix and add to the hash. */
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_CUSTOM) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_CUSTOM, NSLEN) == 0)
+ apr_hash_set(*props, &((const char *)key)[NSLEN],
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+#undef NSLEN
+ /* ### Backwards compatibility: look for old 'svn:custom:'
+ namespace and strip it away, instead of the good URI
+ namespace. REMOVE this block someday! */
 #define NSLEN (sizeof(SVN_PROP_CUSTOM_PREFIX) - 1)
           if (strncmp(key, SVN_PROP_CUSTOM_PREFIX, NSLEN) == 0)
             apr_hash_set(*props, &((const char *)key)[NSLEN],
                          APR_HASH_KEY_STRING,
                          svn_string_create(val, ras->pool));
 #undef NSLEN
+
+ /* If the property is in the 'svn' namespace, then it's a
+ normal user-controlled property coming from the fs. Just
+ strip off the URI prefix, add an 'svn:', and add to the hash. */
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_SVN) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_SVN, NSLEN) == 0)
+ {
+ const char *newkey = apr_pstrcat(ras->pool,
+ SVN_PROP_PREFIX,
+ (const char *)key + NSLEN);
+ apr_hash_set(*props, newkey,
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+ }
+#undef NSLEN
+ /* ### Backwards compatibility: look for old 'svn:' instead
+ of the good URI namespace. Filter out
+ baseline-rel-path. REMOVE this block someday! */
+#define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
+ if (strncmp(key, SVN_PROP_PREFIX, NSLEN) == 0)
+ {
+ if (strcmp((const char *)key + NSLEN,
+ "baseline-relative-path") != 0)
+ apr_hash_set(*props, key,
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+ }
+#undef NSLEN
           
           else if (strcmp(key, SVN_RA_DAV__PROP_CHECKED_IN) == 0)
             {
@@ -961,7 +1029,7 @@
             }
           else
             /* If it's one of the 'entry' props, this func will
- recognize the DAV name & add it to the hash mapped to a
+ recognize the DAV: name & add it to the hash mapped to a
                new name recognized by libsvn_wc. */
             SVN_ERR( set_special_wc_prop ((const char *) key,
                                           (const char *) val,
@@ -1062,18 +1130,32 @@
       else
         entry->size = (apr_off_t) atol(propval); /* ### FIXME? */
 
- /* does this resource contain any 'svn:custom:' properties, i.e.
- ones actually created and set by the user? */
+ /* does this resource contain any 'svn' or 'custom' properties,
+ i.e. ones actually created and set by the user? */
       for (h = apr_hash_first (ras->pool, resource->propset);
            h; h = apr_hash_next (h))
         {
           const void *kkey;
           void *vval;
           apr_hash_this (h, &kkey, NULL, &vval);
+
+ if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_CUSTOM,
+ sizeof(SVN_DAV_PROP_NS_CUSTOM)) == 0)
+ entry->has_props = TRUE;
 
- if (strncmp((const char *)kkey, SVN_PROP_CUSTOM_PREFIX,
- sizeof(SVN_PROP_CUSTOM_PREFIX)) == 0)
+ else if (strncmp((const char *)kkey, SVN_PROP_CUSTOM_PREFIX,
+ sizeof(SVN_PROP_CUSTOM_PREFIX)) == 0)
             entry->has_props = TRUE;
+
+ else if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_SVN,
+ sizeof(SVN_DAV_PROP_NS_SVN)) == 0)
+ entry->has_props = TRUE;
+
+ else if (strncmp((const char *)kkey, SVN_PROP_PREFIX,
+ sizeof(SVN_PROP_PREFIX)) == 0)
+ if (strcmp((const char *)kkey + sizeof(SVN_PROP_PREFIX),
+ "baseline-relative-path") != 0)
+ entry->has_props = TRUE;
         }
 
       /* created_rev & friends */
@@ -1116,15 +1198,52 @@
 
           apr_hash_this(hi, &key, NULL, &val);
 
- /* If the property starts with "svn:custom:", then it's a
+ /* If the property is in the 'custom' namespace, then it's a
              normal user-controlled property coming from the fs. Just
              strip off this prefix and add to the hash. */
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_CUSTOM) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_CUSTOM, NSLEN) == 0)
+ apr_hash_set(*props, &((const char *)key)[NSLEN],
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+#undef NSLEN
+ /* ### Backwards compatibility: look for old 'svn:custom:'
+ namespace and strip it away, instead of the good URI
+ namespace. REMOVE this block someday! */
 #define NSLEN (sizeof(SVN_PROP_CUSTOM_PREFIX) - 1)
           if (strncmp(key, SVN_PROP_CUSTOM_PREFIX, NSLEN) == 0)
             apr_hash_set(*props, &((const char *)key)[NSLEN],
                          APR_HASH_KEY_STRING,
                          svn_string_create(val, ras->pool));
 #undef NSLEN
+
+ /* If the property is in the 'svn' namespace, then it's a
+ normal user-controlled property coming from the fs. Just
+ strip off the URI prefix, add an 'svn:', and add to the hash. */
+#define NSLEN (sizeof(SVN_DAV_PROP_NS_SVN) - 1)
+ if (strncmp(key, SVN_DAV_PROP_NS_SVN, NSLEN) == 0)
+ {
+ const char *newkey = apr_pstrcat(ras->pool,
+ SVN_PROP_PREFIX,
+ (const char *)key + NSLEN);
+ apr_hash_set(*props, newkey,
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+ }
+#undef NSLEN
+ /* ### Backwards compatibility: look for old 'svn:' instead
+ of the good URI namespace. Filter out
+ baseline-rel-path. REMOVE this block someday! */
+#define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
+ if (strncmp(key, SVN_PROP_PREFIX, NSLEN) == 0)
+ {
+ if (strcmp((const char *)key + NSLEN,
+ "baseline-relative-path") != 0)
+ apr_hash_set(*props, key,
+ APR_HASH_KEY_STRING,
+ svn_string_create(val, ras->pool));
+ }
+#undef NSLEN
           
           else if (strcmp(key, SVN_RA_DAV__PROP_CHECKED_IN) == 0)
             {
@@ -1158,7 +1277,7 @@
             }
           else
             /* If it's one of the 'entry' props, this func will
- recognize the DAV name & add it to the hash mapped to a
+ recognize the DAV: name & add it to the hash mapped to a
                new name recognized by libsvn_wc. */
             SVN_ERR( set_special_wc_prop ((const char *) key,
                                           (const char *) val,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 7 20:48:59 2002

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.