Author: stefan2
Date: Mon Jun 29 17:34:28 2015
New Revision: 1688270

URL: http://svn.apache.org/r1688270
Log:
Add a notification if an svn_fs_pack call is a no-op.  Use that to warn
users if they try to pack a non-sharded FSFS repo.

Extending the svn_fs_pack_notify_action_t enum seems to be the only way
to implement this without bumping the svn_fs_pack API:  We can't use the
warning function because the svn_fs_t instance is created temporarily
inside the API function such that the user can't set warning function.
No-op packs are not an error condition either, therefore returning an
svn_error_t would be inappropriate.

To get the most milage out of this feature, we also notify if there is
no complete shard that can be packed.  'svnadmin pack' does not report
that condition to minimize UI changes.  Other API users might be interested
in that information, though.

* subversion/include/svn_fs.h
  (svn_fs_pack_notify_action_t): Add notification for no-op "action".

* subversion/include/svn_repos.h
  (svn_repos_notify_action_t): Same in outer API layer.

* subversion/libsvn_fs_fs/pack.c
  (pack_body): Notify if there was no shard to be packed.
  (svn_fs_fs__pack): Same.  Also warn if the repo is non-sharded.

* subversion/libsvn_repos/fs-wrap.c
  (pack_notify_func): Translate the new notification as well.

* subversion/svnadmin/svnadmin.c
  (repos_notify_handler): Print a warning if the repo is non-sharded.

* subversion/tests/cmdline/svnadmin_tests.py
  (fsfs_pack_non_sharded): New test case.
  (test_list): Register the new test.

Modified:
    subversion/trunk/subversion/include/svn_fs.h
    subversion/trunk/subversion/include/svn_repos.h
    subversion/trunk/subversion/libsvn_fs_fs/pack.c
    subversion/trunk/subversion/libsvn_repos/fs-wrap.c
    subversion/trunk/subversion/svnadmin/svnadmin.c
    subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py

Modified: subversion/trunk/subversion/include/svn_fs.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Mon Jun 29 17:34:28 2015
@@ -2962,7 +2962,13 @@ typedef enum svn_fs_pack_notify_action_t
 
   /** packing of the shard revprops has completed
       @since New in 1.7. */
-  svn_fs_pack_notify_end_revprop
+  svn_fs_pack_notify_end_revprop,
+
+  /** pack has been a no-op for this repository.  The next / future packable
+      shard will be given.  If the shard is -1, then the repository does not
+      support packing at all.
+      @since New in 1.10. */
+  svn_fs_pack_notify_noop
 
 } svn_fs_pack_notify_action_t;
 

Modified: subversion/trunk/subversion/include/svn_repos.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_repos.h?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_repos.h (original)
+++ subversion/trunk/subversion/include/svn_repos.h Mon Jun 29 17:34:28 2015
@@ -244,7 +244,10 @@ typedef enum svn_repos_notify_action_t
   svn_repos_notify_format_bumped,
 
   /** A revision range was copied. @since New in 1.9. */
-  svn_repos_notify_hotcopy_rev_range
+  svn_repos_notify_hotcopy_rev_range,
+
+  /** The repository pack did not do anything. @since New in 1.10. */
+  svn_repos_notify_pack_noop
 } svn_repos_notify_action_t;
 
 /** The type of warning occurring.

Modified: subversion/trunk/subversion/libsvn_fs_fs/pack.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/pack.c?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/pack.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/pack.c Mon Jun 29 17:34:28 2015
@@ -1987,7 +1987,14 @@ pack_body(void *baton,
      we need to re-read the pack status. */
   SVN_ERR(get_pack_status(&fully_packed, pb->fs, pool));
   if (fully_packed)
-    return SVN_NO_ERROR;
+    {
+      if (pb->notify_func)
+        (*pb->notify_func)(pb->notify_baton,
+                           ffd->min_unpacked_rev / ffd->max_files_per_dir,
+                           svn_fs_pack_notify_noop, pool);
+
+      return SVN_NO_ERROR;
+    }
 
   completed_shards = (ffd->youngest_rev_cache + 1) / ffd->max_files_per_dir;
   pb->revs_dir = svn_dirent_join(pb->fs->path, PATH_REVS_DIR, pool);
@@ -2034,12 +2041,24 @@ svn_fs_fs__pack(svn_fs_t *fs,
 
   /* If we aren't using sharding, we can't do any packing, so quit. */
   if (!ffd->max_files_per_dir)
-    return SVN_NO_ERROR;
+    {
+      if (notify_func)
+        (*notify_func)(notify_baton, -1, svn_fs_pack_notify_noop, pool);
+
+      return SVN_NO_ERROR;
+    }
 
   /* Is there we even anything to do?. */
   SVN_ERR(get_pack_status(&fully_packed, fs, pool));
   if (fully_packed)
-    return SVN_NO_ERROR;
+    {
+      if (notify_func)
+        (*notify_func)(notify_baton,
+                       ffd->min_unpacked_rev / ffd->max_files_per_dir,
+                       svn_fs_pack_notify_noop, pool);
+
+      return SVN_NO_ERROR;
+    }
 
   /* Lock the repo and start the pack process. */
   pb.fs = fs;

Modified: subversion/trunk/subversion/libsvn_repos/fs-wrap.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/fs-wrap.c?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/fs-wrap.c (original)
+++ subversion/trunk/subversion/libsvn_repos/fs-wrap.c Mon Jun 29 17:34:28 2015
@@ -975,15 +975,18 @@ pack_notify_func(void *baton,
 {
   struct pack_notify_baton *pnb = baton;
   svn_repos_notify_t *notify;
+  svn_repos_notify_action_t repos_action;
 
   /* Simple conversion works for these values. */
   SVN_ERR_ASSERT(pack_action >= svn_fs_pack_notify_start
-                 && pack_action <= svn_fs_pack_notify_end_revprop);
+                 && pack_action <= svn_fs_pack_notify_noop);
 
-  notify = svn_repos_notify_create(pack_action
-                                   + svn_repos_notify_pack_shard_start
-                                   - svn_fs_pack_notify_start,
-                                   pool);
+  repos_action = pack_action == svn_fs_pack_notify_noop
+               ? svn_repos_notify_pack_noop
+               : pack_action + svn_repos_notify_pack_shard_start
+                             - svn_fs_pack_notify_start;
+
+  notify = svn_repos_notify_create(repos_action, pool);
   notify->shard = shard;
   pnb->notify_func(pnb->notify_baton, notify, pool);
 

Modified: subversion/trunk/subversion/svnadmin/svnadmin.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/svnadmin.c?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/svnadmin.c (original)
+++ subversion/trunk/subversion/svnadmin/svnadmin.c Mon Jun 29 17:34:28 2015
@@ -1126,6 +1126,18 @@ repos_notify_handler(void *baton,
                                _("* Copied revisions from %ld to %ld.\n"),
                                notify->start_revision, notify->end_revision));
         }
+      return;
+
+    case svn_repos_notify_pack_noop:
+      /* For best backward compatibility, we keep silent if there were just
+         no more shards to pack. */
+      if (notify->shard == -1)
+        {
+          svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
+                                  _("Warning: This repository is not sharded."
+                                    " Packing has no effect.\n")));
+        }
+      return;
 
     default:
       return;

Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1688270&r1=1688269&r2=1688270&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Mon Jun 29 
17:34:28 2015
@@ -3066,6 +3066,22 @@ def hotcopy_read_only(sbox):
     logger.warn("Error: hotcopy failed")
     raise SVNUnexpectedStderr(errput)
 
+@SkipUnless(svntest.main.is_fs_type_fsfs)
+@SkipUnless(svntest.main.fs_has_pack)
+def fsfs_pack_non_sharded(sbox):
+  "'svnadmin pack' on a non-sharded repository"
+
+  # Configure two files per shard to trigger packing.
+  sbox.build(create_wc = False,
+             minor_version = min(svntest.main.options.server_minor_version,3))
+  patch_format(sbox.repo_dir, shard_size=2)
+
+  svntest.actions.run_and_verify_svnadmin(
+      None, [], "upgrade", sbox.repo_dir)
+  svntest.actions.run_and_verify_svnadmin(
+      ['Warning: This repository is not sharded. Packing has no effect.\n'],
+      [], "pack", sbox.repo_dir)
+
 ########################################################################
 # Run the tests
 
@@ -3123,6 +3139,7 @@ test_list = [ None,
               load_txdelta,
               load_no_svndate_r0,
               hotcopy_read_only,
+              fsfs_pack_non_sharded,
              ]
 
 if __name__ == '__main__':


Reply via email to