You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/06/29 16:23:38 UTC

svn commit: r1606514 - in /subversion/trunk: subversion/libsvn_fs_fs/rev_file.c subversion/libsvn_fs_fs/rev_file.h subversion/libsvn_fs_fs/transaction.c tools/server-side/svnfsfs/load-index-cmd.c

Author: stefan2
Date: Sun Jun 29 14:23:38 2014
New Revision: 1606514

URL: http://svn.apache.org/r1606514
Log:
Make 'svnfsfs load-index' work again.

There are two issues solved here. First, we need to make the
pack / rev file writable before re-writing the index in it.

Second, there seems to be a problem with seek(APR_CUR) after
truncating files on Unix.  There is no true workaround similar
to the flush() issue in the same function, however using APR_END
employs a different code path returning the correct values.

* subversion/libsvn_fs_fs/rev_file.h
  (svn_fs_fs__open_pack_or_rev_file_writable): Declare new internal
                                               API function.

* subversion/libsvn_fs_fs/rev_file.c
  (set_read_only_baton_t,
   set_read_onl): New APR cleanup callback function & parameter
                  to switch the file back to read-only.
  (auto_make_writable): New utility that makes a file temporarily
                        writable.
  (open_pack_or_rev_file): Add WRITABLE flag and grant write access
                           to the rev / pack file as requested.
  (svn_fs_fs__open_pack_or_rev_file): Update caller.
  (svn_fs_fs__open_pack_or_rev_file_writable): Implement new function
                                               similar to the r/o variant.

* subversion/libsvn_fs_fs/transaction.c
  (svn_fs_fs__add_index_data): Although the file pointer is known
                               to be at EOF, we need to use APR_END
                               here due to a bug in APR.  Functionally,
                               both approaches should be equivalent.

* tools/server-side/svnfsfs/load-index-cmd.c
  (load_index): Request a writable file.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/rev_file.c
    subversion/trunk/subversion/libsvn_fs_fs/rev_file.h
    subversion/trunk/subversion/libsvn_fs_fs/transaction.c
    subversion/trunk/tools/server-side/svnfsfs/load-index-cmd.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/rev_file.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/rev_file.c?rev=1606514&r1=1606513&r2=1606514&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/rev_file.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/rev_file.c Sun Jun 29 14:23:38 2014
@@ -28,6 +28,7 @@
 
 #include "../libsvn_fs/fs-loader.h"
 
+#include "private/svn_io_private.h"
 #include "svn_private_config.h"
 
 static void
@@ -54,13 +55,76 @@ init_revision_file(svn_fs_fs__revision_f
   file->pool = pool;
 }
 
+/* Baton type for set_read_only() */
+typedef struct set_read_only_baton_t
+{
+  /* File to set to read-only. */
+  const char *file_path;
+
+  /* Scratch pool sufficient life time.
+   * Ideally the pool that we registered the cleanup on. */
+  apr_pool_t *pool;
+} set_read_only_baton_t;
+
+/* APR pool cleanup callback taking a set_read_only_baton_t baton and then
+ * (trying to) set the specified file to r/o mode. */
+static apr_status_t
+set_read_only(void *baton)
+{
+  set_read_only_baton_t *ro_baton = baton;
+  apr_status_t status;
+  svn_error_t *err;
+
+  err = svn_io_set_file_read_only(ro_baton->file_path, TRUE, ro_baton->pool);
+  if (err)
+    {
+      status = err->apr_err;
+      svn_error_clear(err);
+    }
+
+  return status;
+}
+
+/* If the file at PATH is read-only, attempt to make it writable.  The
+ * original state will be restored with RESULT_POOL gets cleaned up.
+ * SCRATCH_POOL is for temporary allocations. */
+static svn_error_t *
+auto_make_writable(const char *path,
+                   apr_pool_t *result_pool,
+                   apr_pool_t *scratch_pool)
+{
+  svn_boolean_t is_read_only;
+  apr_finfo_t finfo;
+
+  SVN_ERR(svn_io_stat(&finfo, path, SVN__APR_FINFO_READONLY, scratch_pool));
+  SVN_ERR(svn_io__is_finfo_read_only(&is_read_only, &finfo, scratch_pool));
+
+  if (is_read_only)
+    {
+      /* Tell the pool to restore the r/o state upon cleanup
+         (assuming the file will still exist, failing silently otherwise). */
+      set_read_only_baton_t *baton = apr_pcalloc(result_pool,
+                                                  sizeof(*baton));
+      baton->pool = result_pool;
+      baton->file_path = apr_pstrdup(result_pool, path);
+      apr_pool_cleanup_register(result_pool, baton,
+                                set_read_only, apr_pool_cleanup_null);
+
+      /* Finally, allow write access (undoing it has already been scheduled
+         and is idempotent). */
+      SVN_ERR(svn_io_set_file_read_write(path, FALSE, scratch_pool));
+    }
+}
+
 /* Core implementation of svn_fs_fs__open_pack_or_rev_file working on an
- * existing, initialized FILE structure.
+ * existing, initialized FILE structure.  If WRITABLE is TRUE, give write
+ * access to the file - temporarily resetting the r/o state if necessary.
  */
 static svn_error_t *
 open_pack_or_rev_file(svn_fs_fs__revision_file_t *file,
                       svn_fs_t *fs,
                       svn_revnum_t rev,
+                      svn_boolean_t writable,
                       apr_pool_t *result_pool,
                       apr_pool_t *scratch_pool)
 {
@@ -72,11 +136,19 @@ open_pack_or_rev_file(svn_fs_fs__revisio
     {
       const char *path = svn_fs_fs__path_rev_absolute(fs, rev, scratch_pool);
       apr_file_t *apr_file;
+      apr_int32_t flags = writable
+                        ? APR_READ | APR_WRITE | APR_BUFFERED
+                        : APR_READ | APR_BUFFERED;
+
+      /* We may have to *temporarily* enable write access. */
+      err = writable ? auto_make_writable(path, result_pool, scratch_pool)
+                     : SVN_NO_ERROR; 
+
+      /* open the revision file in buffered r/o or r/w mode */
+      if (!err)
+        err = svn_io_file_open(&apr_file, path, flags, APR_OS_DEFAULT,
+                               result_pool);
 
-      /* open the revision file in buffered r/o mode */
-      err = svn_io_file_open(&apr_file, path,
-                             APR_READ | APR_BUFFERED, APR_OS_DEFAULT,
-                             result_pool);
       if (!err)
         {
           file->file = apr_file;
@@ -132,7 +204,21 @@ svn_fs_fs__open_pack_or_rev_file(svn_fs_
   *file = apr_palloc(result_pool, sizeof(**file));
   init_revision_file(*file, fs, rev, result_pool);
 
-  return svn_error_trace(open_pack_or_rev_file(*file, fs, rev,
+  return svn_error_trace(open_pack_or_rev_file(*file, fs, rev, FALSE,
+                                               result_pool, scratch_pool));
+}
+
+svn_error_t *
+svn_fs_fs__open_pack_or_rev_file_writable(svn_fs_fs__revision_file_t** file,
+                                          svn_fs_t* fs,
+                                          svn_revnum_t rev,
+                                          apr_pool_t* result_pool,
+                                          apr_pool_t *scratch_pool)
+{
+  *file = apr_palloc(result_pool, sizeof(**file));
+  init_revision_file(*file, fs, rev, result_pool);
+
+  return svn_error_trace(open_pack_or_rev_file(*file, fs, rev, TRUE,
                                                result_pool, scratch_pool));
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/rev_file.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/rev_file.h?rev=1606514&r1=1606513&r2=1606514&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/rev_file.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/rev_file.h Sun Jun 29 14:23:38 2014
@@ -98,6 +98,20 @@ svn_fs_fs__open_pack_or_rev_file(svn_fs_
                                  apr_pool_t *result_pool,
                                  apr_pool_t *scratch_pool);
 
+/* Open the correct revision file for REV with read and write access.
+ * If necessary, temporarily reset the file's read-only state.  If the
+ * filesystem FS has been packed, *FILE will be set to the packed file;
+ * otherwise, set *FILE to the revision file for REV.
+ *
+ * Return SVN_ERR_FS_NO_SUCH_REVISION if the file doesn't exist.
+ * Allocate *FILE in RESULT_POOL and use SCRATCH_POOLfor temporaries. */
+svn_error_t *
+svn_fs_fs__open_pack_or_rev_file_writable(svn_fs_fs__revision_file_t **file,
+                                          svn_fs_t *fs,
+                                          svn_revnum_t rev,
+                                          apr_pool_t *result_pool,
+                                          apr_pool_t *scratch_pool);
+
 /* If the footer data in FILE has not been read, yet, do so now.
  * Index locations will only be read upon request as we assume they get
  * cached and the FILE is usually used for REP data access only.

Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1606514&r1=1606513&r2=1606514&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Sun Jun 29 14:23:38 2014
@@ -3587,12 +3587,12 @@ svn_fs_fs__add_index_data(svn_fs_t *fs,
 
   /* Append the actual index data to the pack file. */
   l2p_offset = 0;
-  SVN_ERR(svn_io_file_seek(file, APR_CUR, &l2p_offset, pool));
+  SVN_ERR(svn_io_file_seek(file, APR_END, &l2p_offset, pool));
   SVN_ERR(svn_fs_fs__l2p_index_append(fs, file, l2p_proto_index, revision,
                                       pool));
 
   p2l_offset = 0;
-  SVN_ERR(svn_io_file_seek(file, APR_CUR, &p2l_offset, pool));
+  SVN_ERR(svn_io_file_seek(file, APR_END, &p2l_offset, pool));
   SVN_ERR(svn_fs_fs__p2l_index_append(fs, file, p2l_proto_index, revision,
                                       pool));
 

Modified: subversion/trunk/tools/server-side/svnfsfs/load-index-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/server-side/svnfsfs/load-index-cmd.c?rev=1606514&r1=1606513&r2=1606514&view=diff
==============================================================================
--- subversion/trunk/tools/server-side/svnfsfs/load-index-cmd.c (original)
+++ subversion/trunk/tools/server-side/svnfsfs/load-index-cmd.c Sun Jun 29 14:23:38 2014
@@ -363,8 +363,9 @@ load_index(const char *path,
       svn_fs_fs__revision_file_t *rev_file;
 
       /* Open rev / pack file & trim indexes + footer off it. */
-      SVN_ERR(svn_fs_fs__open_pack_or_rev_file(&rev_file, fs, revision,
-                                               iterpool, iterpool));
+      SVN_ERR(svn_fs_fs__open_pack_or_rev_file_writable(&rev_file, fs,
+                                                        revision, iterpool,
+                                                        iterpool));
       SVN_ERR(svn_fs_fs__auto_read_footer(rev_file));
       SVN_ERR(svn_io_file_trunc(rev_file->file, rev_file->l2p_offset,
                                 iterpool));