File Locking on Win9x, and FSFS repositories

I've just has someone ask me on the Cygwin list I could get FSFS
repositories working on Win9x.

I don't have any Win9x machines available to test, but based on list mails,
the problem is that Win9x doesn't support shared file locking (!). FSFS
doesn't need this (correct?), but currently libsvn_repos is doing the
locking for all repositories, not just BDB repositories, which actually need
it, to ensure recovery isn't run whilst the FS is open.

Anyway, based on this hypothesis, here is a possible patch, which simply
avoids using <repos>/locks/db.lock for FSFS repositories. To do this, I had
to create a new API so that the type of a FS can be discovered from outside
libsvn_fs.

I'd appreciate comments from anyone who understands the situation better,
and if anyone has a Win9x machine on which they could check that this makes
file:/// access to FSFS repositories work, that would be great - although
please take note that this is just quick hack, and my assumption that FSFS
doesn't need this locking might possibly be incorrect.

Max.

[[[
Do not take the repository db.lock for FSFS repositories.
This might let FSFS repositories work on Win9x.

* subversion/include/svn_fs.h (svn_fs_type): New API. Get the type of a FS.
* subversion/libsvn_fs/fs-loader.c (svn_fs_type): New API, functionality
    extracted from fs_library_vtable().
  (fs_library_vtable): Use svn_fs_type().
* subversion/libsvn_repos/repos.c (get_repos):
    Skip locking for FSFS repositories.
]]]

Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h (revision 15801)
+++ subversion/include/svn_fs.h (working copy)
@@ -169,6 +169,15 @@
                           apr_hash_t *config, apr_pool_t *pool);

 /**
+ * Discover the type of a Subversion filesystem located in the directory
+ * @a path.
+ *
+ * @since New in 1.3.
+ */
+svn_error_t *svn_fs_type (const char **fs_type, const char *path,
+ apr_pool_t *pool);
+
+/**
  * Return the path to @a fs's repository, allocated in @a pool.
  * @note This is just what was passed to svn_fs_create() or
  * svn_fs_open() -- might be absolute, might not.
Index: subversion/libsvn_fs/fs-loader.c
===================================================================
--- subversion/libsvn_fs/fs-loader.c (revision 15801)
+++ subversion/libsvn_fs/fs-loader.c (working copy)
@@ -163,12 +163,10 @@
                             _("Unknown FS type '%s'"), fs_type);
 }

-/* Fetch the library vtable for an existing FS. */
-static svn_error_t *
-fs_library_vtable (fs_library_vtable_t **vtable, const char *path,
- apr_pool_t *pool)
+svn_error_t *
+svn_fs_type (const char **fs_type, const char *path, apr_pool_t *pool)
 {
- const char *filename, *fs_type;
+ const char *filename, *local_fs_type;
   char buf[128];
   svn_error_t *err;
   apr_file_t *file;
@@ -180,7 +178,7 @@
   if (err && APR_STATUS_IS_ENOENT (err->apr_err))
     {
       svn_error_clear (err);
- fs_type = SVN_FS_TYPE_BDB;
+ local_fs_type = SVN_FS_TYPE_BDB;
     }
   else if (err)
     return err;
@@ -189,10 +187,24 @@
       len = sizeof(buf);
       SVN_ERR (svn_io_read_length_line (file, buf, &len, pool));
       SVN_ERR (svn_io_file_close (file, pool));
- fs_type = buf;
+ local_fs_type = buf;
     }

   /* Fetch the library vtable by name, now that we've chosen one. */
+ *fs_type = apr_pstrdup (pool, local_fs_type);
+ return SVN_NO_ERROR;
+}
+
+/* Fetch the library vtable for an existing FS. */
+static svn_error_t *
+fs_library_vtable (fs_library_vtable_t **vtable, const char *path,
+ apr_pool_t *pool)
+{
+ const char *fs_type;
+
+ SVN_ERR (svn_fs_type (&fs_type, path, pool));
+
+ /* Fetch the library vtable by name, now that we've chosen one. */
   return get_library_vtable (vtable, fs_type, pool);
 }

Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c (revision 15801)
+++ subversion/libsvn_repos/repos.c (working copy)
@@ -1646,16 +1646,30 @@

   /* Locking. */
   {
- const char *lockfile_path;
- svn_error_t *err;
+ const char *fs_type;

- /* Get a filehandle for the repository's db lockfile. */
- lockfile_path = svn_repos_db_lockfile (repos, pool);
+ SVN_ERR (svn_fs_type (&fs_type, repos->db_path, pool));

- err = svn_io_file_lock2 (lockfile_path, exclusive, nonblocking, pool);
- if (err != NULL && APR_STATUS_IS_EAGAIN (err->apr_err))
- return err;
- SVN_ERR_W (err, _("Error opening db lockfile"));
+ if (!strcmp (fs_type, SVN_FS_TYPE_FSFS))
+ {
+ /* Skip locking for FSFS repositories, because it is not necessary,
+ * and because Win9x is lame, and doesn't support shared locks,
+ * locking anyway actually kills our ability to host repositories
+ * on Win9x. */
+ }
+ else
+ {
+ const char *lockfile_path;
+ svn_error_t *err;
+
+ /* Get a filehandle for the repository's db lockfile. */
+ lockfile_path = svn_repos_db_lockfile (repos, pool);
+
+ err = svn_io_file_lock2 (lockfile_path, exclusive, nonblocking,
pool);
+ if (err != NULL && APR_STATUS_IS_EAGAIN (err->apr_err))
+ return err;
+ SVN_ERR_W (err, _("Error opening db lockfile"));
+ }
   }

   /* Open up the filesystem only after obtaining the lock. */

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org