experiment to get Mac resource-forks under version control [with PATCH]

Howdy,

I'm looking for advice and review :-)

Projects I build under OS X destined for the Palm or for OS 9 still
have resources. I use subversion for non-resource stuff with no
problem, but it would be a win for me to use subversion for these
projects as well.

The basic idea in this experiment is to represent a Macintosh resource
fork as a special property. The property is a write-through reflection
of the resource fork data. I can explain the reasoning that led me to
this choice if anyone cares. I've hooked in within libsvn_wc. I added
a wcprop to remember the last time the resource-fork on disk and the
value of the resource-fork property were known to be identical. I
added no new options, commands, or syntax. I exploit OS X's magical
file notation for addressing the resource fork as though it were a
distinct but ordinary data file. You can put a file's resources under
version control like so

    svn propset -F x/rsrc x

In a working copy, this code successfully manages: changes to the
resource fork from outside subversion; changes through propset; and can
get the resource property checked into the repository.

I wanted to get general opinions on whether I'm on the right track.
This code is in no way ready for prime-time. Checkout doesn't work,
because I don't have sufficient grasp of the order of operations in a
checkout (or, I guess, file installation in general) to make sure my
reflection code is hooked up in the right place. I use the properties

    svn:resource-fork
    svn:wc:last-resource-reflect-time

My tests aren't really tests yet, I just wanted to make sure I could
build tests and get them executed.

Is this approach a fit for subversion's architecture? Is there a
better way?

Am I hooking in in the right place?

Should I be using properties like this? Is it right to use a wcprop
here?

The code is cross-platform, but its execution isn't. How best to make
this fit with other clients? Config tests and ifdefs? Conditional
execution? Something else?

And of course, advice about where I got coding guidelines, idiom wrong,
etc. Patch below ---
__________
Scott Collins <http://ScottCollins.net/>

Index: svn/build.conf
===================================================================
--- svn/build.conf (revision 5862)
+++ svn/build.conf (working copy)
@@ -408,6 +408,14 @@
  install = test
  libs = libsvn_test libsvn_delta libsvn_wc libsvn_subr
$(SVN_APRUTIL_LIBS) $(SVN_APR_LIBS)

+# test resource-fork manipulation routines
+[rsrcfork-test]
+type = exe
+path = subversion/tests/libsvn_wc
+sources = rsrcfork-test.c
+install = test
+libs = libsvn_test libsvn_wc $(SVN_APRUTIL_LIBS) $(SVN_APR_LIBS)
+
  # test svn_config utilities
  [config-test]
  type = exe
@@ -590,7 +598,7 @@
         fs-test skel-test key-test strings-reps-test changes-test
         md5args repos-test
         config-test hashdump-test stringtest path-test stream-test
time-test
- translate-test
+ translate-test rsrcfork-test
         random-test diff-diff3-test
         target-test svndiff-test vdelta-test diff-test diff3-test
diff4-test

Index: svn/subversion/include/svn_props.h
===================================================================
--- svn/subversion/include/svn_props.h (revision 5862)
+++ svn/subversion/include/svn_props.h (working copy)
@@ -160,6 +160,9 @@
  /** The value to force the executable property to when set */
  #define SVN_PROP_EXECUTABLE_VALUE "*"

+/** The (binary) resource data for a given file iff the user elects to
manage it */
+#define SVN_PROP_RESOURCE_FORK SVN_PROP_PREFIX "resource-fork"
+
  /** Describes external items to check out into this directory.
   *
   * The format is a series of lines, such as:
@@ -198,6 +201,10 @@
   * when committing.
   */
  #define SVN_PROP_WC_PREFIX SVN_PROP_PREFIX "wc:"
+
+/** Last time the actual resource fork and the property were known to
be identical */
+#define SVN_PROP_WC_RESOURCE_FORK_REFLECTED_DATE \
+ SVN_PROP_WC_PREFIX "resource-fork-reflected-date"

  /** Another type of non-user-visible property. "Entry properties" are
   * stored as fields with the administrative 'entries' file.
Index: svn/subversion/libsvn_wc/props.c
===================================================================
--- svn/subversion/libsvn_wc/props.c (revision 5862)
+++ svn/subversion/libsvn_wc/props.c (working copy)
@@ -928,6 +928,163 @@
  }

+static
+svn_error_t *
+set_reflect_time( const char *path,
+ const char *rf_path,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool)
+{
+ apr_time_t rtime;
+ char *rtime_cstring;
+ svn_string_t *rtime_svn_string;
+
+ SVN_ERR (svn_io_file_affected_time (&rtime, rf_path, pool));
+
+ rtime_cstring = svn_time_to_cstring (rtime, pool);
+ rtime_svn_string = svn_string_create (rtime_cstring, pool);
+
+ SVN_ERR (svn_wc__wcprop_set
(SVN_PROP_WC_RESOURCE_FORK_REFLECTED_DATE,
+ rtime_svn_string,
+ path,
+ adm_access,
+ pool));
+
+ return SVN_NO_ERROR;
+}
+
+
+static
+svn_error_t *
+get_reflect_time( const char *path,
+ apr_time_t *rtime,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool)
+{
+ svn_string_t *rtime_svn_string;
+
+ SVN_ERR (svn_wc__wcprop_get (&rtime_svn_string,
+
SVN_PROP_WC_RESOURCE_FORK_REFLECTED_DATE,
+ path,
+ adm_access,
+ pool));
+
+ SVN_ERR (svn_time_from_cstring (rtime, rtime_svn_string->data,
pool));
+
+ return SVN_NO_ERROR;
+}
+
+
+static
+char *
+resource_path_from_path( const char *path,
+ apr_pool_t *pool )
+{
+ return apr_pstrcat(pool, path, "/rsrc", NULL);
+}
+
+
+
+svn_error_t*
+svn_wc__reflect_rsrcfork_to_prop( const char *path,
+ apr_hash_t *props,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool )
+{
+ char *rf_path;
+ apr_time_t rf_mod_time;
+ apr_time_t reflect_time;
+ svn_error_t *error;
+
+
+ /* if there's no svn:resource-fork, then there's nothing to do */
+ if ( NULL == apr_hash_get(props, SVN_PROP_RESOURCE_FORK,
APR_HASH_KEY_STRING) )
+ return SVN_NO_ERROR;
+
+
+ /* 'reflected' here means ``make the resource-fork and the binary
property
+ * be identical''. We remember the last time they were identical
in a wcprop.
+ * If the last-mod-time of the resource fork matches the saved
reflect-time,
+ * then we believe the resource-fork and the binary property are
still identical
+ */
+
+ rf_path = resource_path_from_path(path, pool);
+
+#if 1
+ /* the last time the resource-fork was physically modified on disk
*/
+ error = svn_io_file_affected_time (&rf_mod_time, rf_path, pool);
+ if ( error )
+ return SVN_NO_ERROR;
+ /* if the resource fork doesn't exist yet, that's no problem,
+ maybe we're doing a checkout */
+#else
+ SVN_ERR (svn_io_file_affected_time (&rf_mod_time, rf_path, pool));
+#endif
+
+ /* the last time the resource-fork on disk, and the property were
+ known to be identical */
+ error = get_reflect_time(path, &reflect_time, adm_access, pool);
+ if ( error )
+ return SVN_NO_ERROR;
+ /* if we don't have a working copy reflect timestamp yet, that's
no problem,
+ maybe we're doing a checkout */
+
+ /* if the resource-fork data is newer than the property data... */
+ if ( rf_mod_time > reflect_time )
+ {
+ svn_stringbuf_t *buf = 0;
+ svn_string_t *resource_data = 0;
+
+ /* ...then let's read it into the property */
+ SVN_ERR (svn_stringbuf_from_file (&buf, rf_path, pool));
+
+ resource_data = svn_string_create_from_buf (buf, pool);
+
+ apr_hash_set (props, SVN_PROP_RESOURCE_FORK,
APR_HASH_KEY_STRING, resource_data);
+
+
+ /* That was a reflection, the resource-fork and the property
are
+ * now known to be identical, so lets save the new mod-time as
+ * our reflect-time
+ */
+ SVN_ERR (set_reflect_time (path, rf_path, adm_access, pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
+svn_wc__reflect_prop_to_rsrcfork( const char *path,
+ const svn_string_t *prop_value,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool )
+{
+ apr_status_t apr_err;
+ apr_file_t* f;
+ char *rf_path;
+ apr_size_t len;
+
+ rf_path = resource_path_from_path(path, pool);
+
+ apr_err = apr_file_open(&f,
+ rf_path,
+ (APR_WRITE | APR_CREATE | APR_TRUNCATE |
APR_BINARY),
+ APR_OS_DEFAULT,
+ pool);
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, rf_path);
+
+ len = prop_value->len;
+ apr_err = apr_file_write(f, prop_value->data, &len);
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, rf_path);
+
+ SVN_ERR (set_reflect_time (path, rf_path, adm_access, pool));
+}
+
+
+

  /*------------------------------------------------------------------*/
@@ -960,6 +1117,9 @@

    SVN_ERR (svn_wc__load_prop_file (prop_path, *props, pool));

+ /* update binary resource property from disk, if needed */
+ SVN_ERR (svn_wc__reflect_rsrcfork_to_prop (path, *props, adm_access,
pool));
+
    return SVN_NO_ERROR;
  }

@@ -1240,6 +1400,12 @@
                                           
SVN_WC__ENTRY_MODIFY_TEXT_TIME,
                                           TRUE, pool));
          }
+ }
+
+ /* ... */
+ if (kind == svn_node_file && strcmp (name, SVN_PROP_RESOURCE_FORK)
== 0)
+ {
+ SVN_ERR (svn_wc__reflect_prop_to_rsrcfork (path, value,
adm_access, pool));
      }

    return SVN_NO_ERROR;
Index: svn/subversion/libsvn_wc/props.h
===================================================================
--- svn/subversion/libsvn_wc/props.h (revision 5862)
+++ svn/subversion/libsvn_wc/props.h (working copy)
@@ -138,6 +138,17 @@
  svn_error_t *svn_wc__remove_wcprops (svn_wc_adm_access_t *adm_access,
                                       apr_pool_t *pool);

+/* Reflect contents of a files `resource fork' into a binary property,
+ iff user has elected to let us manage it. */
+svn_error_t *svn_wc__reflect_rsrcfork_to_prop( const char *path,
+ apr_hash_t *props,
+ svn_wc_adm_access_t
*adm_access,
+ apr_pool_t *pool );
+
+svn_error_t *svn_wc__reflect_prop_to_rsrcfork( const char *path,
+ const svn_string_t
*prop_value,
+ svn_wc_adm_access_t
*adm_access,
+ apr_pool_t *pool );

  #ifdef __cplusplus
  }
Index: svn/subversion/tests/libsvn_wc/rsrcfork-test.c
===================================================================
--- svn/subversion/tests/libsvn_wc/rsrcfork-test.c (working copy)
+++ svn/subversion/tests/libsvn_wc/rsrcfork-test.c (working copy)
@@ -0,0 +1,81 @@
+#include <apr_general.h>
+#include <apr_file_io.h>
+#include <apr_time.h>
+#include <svn_wc.h>
+#include "svn_test.h"
+
+static svn_error_t *
+reflect_rsrcfork_to_prop (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ *msg = "reflect resource fork to property";
+ if ( msg_only )
+ return SVN_NO_ERROR;
+
+ apr_status_t apr_err;
+ apr_file_t* f;
+
+ apr_err = apr_file_open(&f,
+ "foo",
+ (APR_WRITE | APR_CREATE),
+ APR_OS_DEFAULT,
+ pool);
+
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, "foo");
+
+ apr_err = apr_file_close(f);
+
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, "foo");
+
+ apr_err = apr_file_open(&f,
+ "foo/rsrc",
+ (APR_WRITE | APR_CREATE | APR_BINARY),
+ APR_OS_DEFAULT,
+ pool);
+
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, "foo/rsrc");
+
+ apr_size_t size = 6;
+ apr_err = apr_file_write(f, "Hello", &size);
+
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, "foo/rsrc");
+
+ apr_err = apr_file_close(f);
+
+ if ( apr_err )
+ return svn_error_create(apr_err, NULL, "foo/rsrc");
+
+ return svn_error_create(SVN_ERR_TEST_FAILED,
+ NULL,
+ "resource fork reflection not yet
implemented");
+}
+
+static svn_error_t *
+reflect_prop_to_rsrcfork (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ *msg = "reflect property to resource fork";
+ if ( msg_only )
+ return SVN_NO_ERROR;
+
+ return svn_error_create(SVN_ERR_TEST_FAILED,
+ NULL,
+ "resource fork reflection not yet
implemented");
+}
+
+
+
+/* The test table. */
+struct svn_test_descriptor_t test_funcs[] =
+{
+ SVN_TEST_NULL,
+ SVN_TEST_PASS(reflect_rsrcfork_to_prop),
+ SVN_TEST_PASS(reflect_prop_to_rsrcfork),
+ SVN_TEST_NULL
+};