You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vladimir Berezniker <vm...@tigris.org> on 2003/08/05 18:40:09 UTC

Enhancing and possibly renaming svnadmin archive command

It seems that my emails are gotting lost... so here I try for the last time.

I would like to submit a patch to "svnadmin archive" command and its 
underlying implementation in fs.c.

I. Enhancements:
----------------------------------------------------------

A). Add two new  parameters to  svn_fs_berkeley_archive to look as follows:

     /** Set @a *logfiles to array of <tt>const char *</tt> logfile paths
      * of Berkeley DB-based Subversion filesystem.
      *
      * @a absolute_paths specifies format of the paths:
      * TRUE   - Paths are to be absolute
      * FALSE  - Paths are to be relative to the Berkeley DB logs directory.
      *
      * @a all_log_files specifies which logs to include in the list:
      * TRUE   - All logs are included
     * FALSE  - Only unused logs are included
     *
     * This function wraps the same Berkeley DB 'log_archive' function
     * called by the db_archive binary.  Repository administrators may
     * want to run this function periodically and delete the unused log
     * files, as a way of reclaiming disk space.
     */
     svn_error_t *svn_fs_berkeley_archive (apr_array_header_t **logfiles,
                                            const char *path,
                                           svn_boolean_t absolute_paths,
                                           svn_boolean_t all_log_files,
                                           apr_pool_t *pool);


B) Modify svnadmin's main.c to allow expose the new API to the end user.





II. Questions that came up:
----------------------------------------------------------

A). Name of the "archive" command.

     As Michael Wood pointed out that "archive" is not the best name to 
describe the command that lists Berkeley DB log files. 
http://article.gmane.org/gmane.comp.version-control.subversion.devel/31809

     * How does "svnadmin dbloglist [--all]" sound?

     * If command is renamed does svn_fs_berkeley_archive  get renamed 
as  well or a new function is created (e.g. svn_fs_berkeley_loglist) and 
svn_fs_berkeley_archive calls the new function so that any existing code 
works without a change.?

B). Need for absolute_paths parameter.

     cmpilato@collab.net said: "I don't really see the need to have the 
abs-vs.-rel path parameter", referring to absolute_paths parameter. 
http://article.gmane.org/gmane.comp.version-control.subversion.devel/31725

      * Given that I also want to submit a patch for "svnadmin copy" 
(hot copy) command that will need log file names only, is that a reason 
enough to keep that parameter? You can read about why I wish to compare 
log files at 
http://article.gmane.org/gmane.comp.version-control.subversion.devel/31740
	

I await your comments.

Sincerely,
Vladimir Berezniker


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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Vladimir Berezniker <vm...@tigris.org> writes:

> I am hoping that you will feel better about "svnadmin lsdblogs"
> command, because it does have a tiny bit of brains to it.

Not likely that I will, but not because it lacks brains -- just
because it's Berkeley-specific.  But whatever.

> But I do have to admit I am still a bit confused about the whole
> utf8 stuff and whether I should be converting the path to cstring
> before printing it.  But I am not sure if the path is in utf8 in the
> first place.  The code is in my revised patch that I send to the
> list earlier today.

Hmm... we need to know what Berkeley is handing us so we can decide if
we need to convert to utf8 before returning those paths from libsvn_fs
(and back to native again before printing them from svnadmin).

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
cmpilato@collab.net wrote:
> Michael Wood <mw...@its.uct.ac.za> writes:
> 
> 
[snip]
> 
>>By the way, is svnadmin always only going to operate on Berkeley DB
>>based Subversion repositories?
> 
> 
> Why, got patches?  ;-)
> 
> We'd like to keep a single tool that understands all database backends
> we eventually support.  My personal opinion is that we should have
> never implemented either 'svnadmin recover' or 'svnadmin archive'
> because they are nothing but redundant, mindless wrappers around
> existing functionality provided by the wonderful folks at Sleepycat as
> part of the Berkeley distribution.  If you plan to use software that
> connects to a database (directly, networked, or otherwise), by golly,
> you better know how to take care of that database.
> 

As a user I feel more comfortable running "svnadmin archive" more than running 
"db_archive".  I am not sure exactly why. Maybe because it feels like that 
command got the approval of Subversion developers, I guess.

I am hoping that you will feel better about "svnadmin lsdblogs" command, because 
it does have a tiny bit of brains to it.  But I do have to admit I am still a 
bit confused about the whole utf8 stuff and whether I should be converting the 
path to cstring before printing it.  But I am not sure if the path is in utf8 in 
the first place.  The code is in my revised patch that I send to the list 
earlier today.


[snip]
Sincerely,
Vladimir Berezniker



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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Michael Wood <mw...@its.uct.ac.za> writes:

> What would make *much* more sense is for the default to list only the
> unused logs and a switch to list all of them.  (Which is what Vladimir
> was advocating.)

Maybe you missed the part of my reply to Vladimir which indicated that
at the end of the day, I didn't care what he decided to do UI-wise.

> By the way, is svnadmin always only going to operate on Berkeley DB
> based Subversion repositories?

Why, got patches?  ;-)

We'd like to keep a single tool that understands all database backends
we eventually support.  My personal opinion is that we should have
never implemented either 'svnadmin recover' or 'svnadmin archive'
because they are nothing but redundant, mindless wrappers around
existing functionality provided by the wonderful folks at Sleepycat as
part of the Berkeley distribution.  If you plan to use software that
connects to a database (directly, networked, or otherwise), by golly,
you better know how to take care of that database.

But whatever.  Until we have more than one backend to choose from,
such concerns are premature.

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
I hope this patch is somewhere in the middle of different views of how db log 
files listing should be handled.

Log Message:

* include/svn_fs.h
         (svn_fs_berkeley_logfiles): Renamed from svn_fs_berkeley_archive to be 
more in line with actual purpose of the function. Change return pointer to 
apr_array_header_t type from pointer to NULL-terminated array of const char *. 
Add parameter specifying whether to return all or only unused log file names.

* libsvn_fs/fs.c
         (svn_fs_berkeley_logfiles): Implement the function as specified in 
include/svn_fs.h.

* include/svn_repos.h
         (svn_repos_db_logfiles): Created wrapper function around 
svn_fs_berkeley_logfiles to return log paths relative to the repository root.

* libsvn_repos/repos.c
       (svn_repos_db_logfiles): Implement the function as specified in 
include/svn_repos.h.

* svnadmin/main.c
	Rename "archive" sub command to "lsdblogs" and change default behavior from 
listing only unused logs to listing all log files, to be consistent with the 
name of sub command.  Added --only-unused option to mimic the behavior of 
replaced "archive" sub command.



Patch:
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c	(revision 6656)
+++ subversion/svnadmin/main.c	(working copy)
@@ -55,7 +55,6 @@
  /** Subcommands. **/

  static svn_opt_subcommand_t
-  subcommand_archive,
    subcommand_create,
    subcommand_createtxn,
    subcommand_dump,
@@ -63,6 +62,7 @@
    subcommand_load,
    subcommand_lscr,
    subcommand_lstxns,
+  subcommand_lsdblogs,
    subcommand_recover,
    subcommand_rmtxns,
    subcommand_setlog;
@@ -77,7 +77,8 @@
      svnadmin__ignore_uuid,
      svnadmin__force_uuid,
      svnadmin__parent_dir,
-    svnadmin__bdb_txn_nosync
+    svnadmin__bdb_txn_nosync,
+    svnadmin__only_unused
    };

  /* Option codes and descriptions.
@@ -128,6 +129,9 @@
      {SVN_FS_CONFIG_BDB_TXN_NOSYNC, svnadmin__bdb_txn_nosync, 0,
       "disable fsync at database transaction commit [Berkeley DB]."},

+    {"only-unused", svnadmin__only_unused, 0,
+     "list only used log files, that can be archived."},
+
      {NULL}
    };

@@ -137,11 +141,6 @@
   */
  static const svn_opt_subcommand_desc_t cmd_table[] =
    {
-    {"archive", subcommand_archive, {0},
-     "usage: svnadmin archive REPOS_PATH\n\n"
-     "Ask Berkeley DB which logfiles can be safely deleted.\n\n",
-     {0} },
-
      {"create", subcommand_create, {0},
       "usage: svnadmin create REPOS_PATH\n\n"
       "Create a new, empty repository at REPOS_PATH.\n",
@@ -186,6 +185,12 @@
       "repository.)\n",
       {svnadmin__follow_copies} },

+    {"lsdblogs", subcommand_lsdblogs, {0},
+     "usage: svnadmin lsdblogs REPOS_PATH\n\n"
+     "List all Berkeley DB log files.\n\n",
+     {svnadmin__only_unused} },
+
+
      {"lstxns", subcommand_lstxns, {0},
       "usage: svnadmin lstxns REPOS_PATH\n\n"
       "Print the names of all uncommitted transactions.\n",
@@ -227,6 +232,7 @@
    svn_boolean_t follow_copies;                      /* --copies */
    svn_boolean_t quiet;                              /* --quiet */
    svn_boolean_t bdb_txn_nosync;                     /* --bdb-txn-nosync */
+  svn_boolean_t only_unused;                        /* --only-unused */
    enum svn_repos_load_uuid uuid_action;             /* --ignore-uuid,
                                                         --force-uuid */
    const char *on_disk;
@@ -490,23 +496,27 @@

  /* This implements `svn_opt_subcommand_t'. */
  svn_error_t *
-subcommand_archive (apr_getopt_t *os, void *baton, apr_pool_t *pool)
+subcommand_lsdblogs (apr_getopt_t *os, void *baton, apr_pool_t *pool)
  {
-  svn_repos_t *repos;
    struct svnadmin_opt_state *opt_state = baton;
-  char **logfiles;
-  char **filename;
+  apr_array_header_t *logfiles;
+  int log;

-  SVN_ERR (svn_repos_open (&repos, opt_state->repository_path, pool));
+  SVN_ERR (svn_repos_db_logfiles (&logfiles,
+                                  opt_state->repository_path,
+                                  opt_state->only_unused,
+                                  pool));

-  SVN_ERR (svn_fs_berkeley_archive (&logfiles,
-                                    svn_repos_db_env (repos, pool), pool));
-
    if (logfiles == NULL)
-    return SVN_NO_ERROR;
+    {
+      return SVN_NO_ERROR;
+    }

-  for (filename = logfiles; *filename != NULL; ++filename)
-    printf ("%s\n", *filename);
+  /* Loop, printing log files. */
+  for (log = 0; log < logfiles->nelts; log++)
+    {
+      printf ("%s\n", APR_ARRAY_IDX (logfiles, log, const char *));
+    }

    return SVN_NO_ERROR;
  }
@@ -750,6 +760,9 @@
        case svnadmin__bdb_txn_nosync:
          opt_state.bdb_txn_nosync = TRUE;
          break;
+      case svnadmin__only_unused:
+        opt_state.only_unused = TRUE;
+        break;
        default:
          {
            subcommand_help (NULL, NULL, pool);
Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h	(revision 6656)
+++ subversion/include/svn_fs.h	(working copy)
@@ -193,19 +193,22 @@
                                        apr_pool_t *pool);


-/** Return a NULL-terminated array of absolute paths to @a logfiles
- * that are no longer is use by the DB-based Subversion filesystem,
- * stored in the environment @a path.  Do any necessary allocation
- * within @a pool.
+/** Set @a *logfiles to array of <tt>const char *</tt> log file names
+ * of Berkeley DB-based Subversion filesystem.
   *
- * This function wraps the same Berkeley DB 'log_archive' function
+ * @a only_unused specifies whether to include used log file names in the list:
+ * TRUE   - Only unused logs are included
+ * FALSE  - All logs are included
+ *
+ * This function wraps the Berkeley DB 'log_archive' function
   * called by the db_archive binary.  Repository administrators may
   * want to run this function periodically and delete the unused log
   * files, as a way of reclaiming disk space.
   */
-svn_error_t *svn_fs_berkeley_archive (char ***logfiles,
-                                      const char *path,
-                                      apr_pool_t *pool);
+svn_error_t *svn_fs_berkeley_logfiles (apr_array_header_t **logfiles,
+                                       const char *path,
+                                       svn_boolean_t only_unused,
+                                       apr_pool_t *pool);


  /** @} */
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h	(revision 6656)
+++ subversion/include/svn_repos.h	(working copy)
@@ -108,6 +108,17 @@
  svn_error_t *svn_repos_recover (const char *path, apr_pool_t *pool);


+/** This function is a wrapper around svn_fs_berkeley_logfiles(),
+ * returning log file paths relative to the root of the repository.
+ *
+ * @copydoc svn_fs_berkeley_logfiles()
+ */
+svn_error_t *svn_repos_db_logfiles (apr_array_header_t **logfiles,
+                                    const char *path,
+                                    svn_boolean_t only_unused,
+                                    apr_pool_t *pool);
+
+
  
  /* Repository Paths */

Index: subversion/libsvn_fs/fs.c
===================================================================
--- subversion/libsvn_fs/fs.c	(revision 6656)
+++ subversion/libsvn_fs/fs.c	(working copy)
@@ -651,16 +651,19 @@
  /* Running the 'archive' command on a Berkeley DB-based filesystem.  */


-svn_error_t *
-svn_fs_berkeley_archive (char ***logfiles,
-                         const char *path,
-                         apr_pool_t *pool)
+svn_error_t *svn_fs_berkeley_logfiles (apr_array_header_t **logfiles,
+                                       const char *path,
+                                       svn_boolean_t only_unused,
+                                       apr_pool_t *pool)
  {
    int db_err;
    DB_ENV *env;
    const char *path_native;
    char **filelist;
+  char **filename;

+  *logfiles = NULL;
+
    db_err = db_env_create (&env, 0);
    if (db_err)
      return svn_fs__bdb_dberr (db_err);
@@ -673,16 +676,46 @@
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

-  db_err = env->log_archive (env, &filelist,
-                             DB_ARCH_ABS /* return absolute paths */);
+  {
+    u_int32_t flags = 0;
+
+    if(only_unused == FALSE)
+      {
+        flags |= DB_ARCH_LOG;
+      }
+
+    db_err = env->log_archive (env, &filelist, flags);
+  }
+
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

+  if (filelist == NULL)
+    {
+      db_err = env->close (env, 0);
+      if (db_err)
+        return svn_fs__bdb_dberr (db_err);
+
+      return SVN_NO_ERROR;
+    }
+
+  {
+    *logfiles = apr_array_make (pool, 1, sizeof (const char *));
+
+    for (filename = filelist; *filename != NULL; ++filename)
+      {
+        APR_ARRAY_PUSH (*logfiles, const char *) =  (apr_pstrdup(pool, *filename));
+      }
+
+    /* allocate_env sets malloc and free as the memory management functions,
+       therefore we use free to release memory allocated by DB_ENV */
+    free(filelist);
+  }
+
    db_err = env->close (env, 0);
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

-  *logfiles = filelist;
    return SVN_NO_ERROR;
  }

Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c	(revision 6656)
+++ subversion/libsvn_repos/repos.c	(working copy)
@@ -1261,3 +1261,36 @@

    return SVN_NO_ERROR;
  }
+
+svn_error_t *svn_repos_db_logfiles (apr_array_header_t **logfiles,
+                                    const char *path,
+                                    svn_boolean_t only_unused,
+                                    apr_pool_t *pool)
+{
+  svn_repos_t *repos;
+  int log;
+
+  SVN_ERR (get_repos (&repos, path,
+                      APR_FLOCK_SHARED,
+                      FALSE,     /* Do not open fs. */
+                      pool));
+
+  SVN_ERR (svn_fs_berkeley_logfiles (logfiles,
+                                    svn_repos_db_env (repos, pool),
+                                    only_unused,
+                                    pool));
+
+  if (logfiles == NULL)
+    {
+      return SVN_NO_ERROR;
+    }
+
+  /* Loop, printing log files. */
+  for (log = 0; log < (*logfiles)->nelts; log++)
+    {
+      const char ** log_file = &(APR_ARRAY_IDX (*logfiles, log, const char *));
+      *log_file = svn_path_join(SVN_REPOS__DB_DIR, *log_file, pool);
+    }
+
+  return SVN_NO_ERROR;
+}


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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
cmpilato@collab.net wrote:
[snip]
> 
> I'm making another change, too.  I'm appending the log paths we get
> back from the repository onto the repos_path supplied to svnadmin.
> Any problems with that?

None.


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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Vladimir Berezniker <vm...@tigris.org> writes:

> cmpilato@collab.net wrote:
> > Vladimir Berezniker <vm...@tigris.org> writes:
> >
> [snip]
> > I'll commit this patch as soon as I do a little testing/fixing on it.
> > Already I've found a SEGFAULT -- while svn_fs_berkeley_logfiles
> > promises to return an array, sometimes it returns NULL, and a caller
> > wasn't check for that.  The promise is fine, the code broken, so I'm
> > making it always return an array, even if it's empty (which is
> > consistent with other FS function promises).
> 
> I am still kicking myself for overlooking something so obivious. I
> wanted to make it less work for you guys, not more.  I will try to be
> more diligent in the future.

Alright, I'll call back my boys.  No need to bust any kneecaps today,
I suppose. :-)

I'm making another change, too.  I'm appending the log paths we get
back from the repository onto the repos_path supplied to svnadmin.
Any problems with that?

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
cmpilato@collab.net wrote:
> Vladimir Berezniker <vm...@tigris.org> writes:
> 
[snip]
> 
> I'll commit this patch as soon as I do a little testing/fixing on it.
> 
> Already I've found a SEGFAULT -- while svn_fs_berkeley_logfiles
> promises to return an array, sometimes it returns NULL, and a caller
> wasn't check for that.  The promise is fine, the code broken, so I'm
> making it always return an array, even if it's empty (which is
> consistent with other FS function promises).

I am still kicking myself for overlooking something so obivious. I wanted to 
make it less work for you guys, not more.  I will try to be more diligent in the 
future.

Thank you very much for all you help and guidance on this,
Vladimir Berezniker
(Kicking myself deservingly)


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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Vladimir Berezniker <vm...@tigris.org> writes:

> Found a typo in --only-unused option description.  Updated patch
> follows.

I'll commit this patch as soon as I do a little testing/fixing on it.

Already I've found a SEGFAULT -- while svn_fs_berkeley_logfiles
promises to return an array, sometimes it returns NULL, and a caller
wasn't check for that.  The promise is fine, the code broken, so I'm
making it always return an array, even if it's empty (which is
consistent with other FS function promises).

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
Found a typo in --only-unused option description.  Updated patch follows.

Log Message:

* include/svn_fs.h
         (svn_fs_berkeley_logfiles): Renamed from svn_fs_berkeley_archive to be
more in line with actual purpose of the function. Change return pointer to
apr_array_header_t type from pointer to NULL-terminated array of const char *.
Add parameter specifying whether to return all or only unused log file names.

* libsvn_fs/fs.c
         (svn_fs_berkeley_logfiles): Implement the function as specified in
include/svn_fs.h.

* include/svn_repos.h
         (svn_repos_db_logfiles): Created wrapper function around
svn_fs_berkeley_logfiles to return log paths relative to the repository root.

* libsvn_repos/repos.c
       (svn_repos_db_logfiles): Implement the function as specified in
include/svn_repos.h.

* svnadmin/main.c
	Rename "archive" sub command to "lsdblogs" and change default behavior from
listing only unused logs to listing all log files, to be consistent with the
name of sub command.  Added --only-unused option to mimic the behavior of
replaced "archive" sub command.



Patch:

Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c	(revision 6656)
+++ subversion/svnadmin/main.c	(working copy)
@@ -55,7 +55,6 @@
  /** Subcommands. **/

  static svn_opt_subcommand_t
-  subcommand_archive,
    subcommand_create,
    subcommand_createtxn,
    subcommand_dump,
@@ -63,6 +62,7 @@
    subcommand_load,
    subcommand_lscr,
    subcommand_lstxns,
+  subcommand_lsdblogs,
    subcommand_recover,
    subcommand_rmtxns,
    subcommand_setlog;
@@ -77,7 +77,8 @@
      svnadmin__ignore_uuid,
      svnadmin__force_uuid,
      svnadmin__parent_dir,
-    svnadmin__bdb_txn_nosync
+    svnadmin__bdb_txn_nosync,
+    svnadmin__only_unused
    };

  /* Option codes and descriptions.
@@ -128,6 +129,9 @@
      {SVN_FS_CONFIG_BDB_TXN_NOSYNC, svnadmin__bdb_txn_nosync, 0,
       "disable fsync at database transaction commit [Berkeley DB]."},

+    {"only-unused", svnadmin__only_unused, 0,
+     "list only unused log files, that can be archived."},
+
      {NULL}
    };

@@ -137,11 +141,6 @@
   */
  static const svn_opt_subcommand_desc_t cmd_table[] =
    {
-    {"archive", subcommand_archive, {0},
-     "usage: svnadmin archive REPOS_PATH\n\n"
-     "Ask Berkeley DB which logfiles can be safely deleted.\n\n",
-     {0} },
-
      {"create", subcommand_create, {0},
       "usage: svnadmin create REPOS_PATH\n\n"
       "Create a new, empty repository at REPOS_PATH.\n",
@@ -186,6 +185,12 @@
       "repository.)\n",
       {svnadmin__follow_copies} },

+    {"lsdblogs", subcommand_lsdblogs, {0},
+     "usage: svnadmin lsdblogs REPOS_PATH\n\n"
+     "List all Berkeley DB log files.\n\n",
+     {svnadmin__only_unused} },
+
+
      {"lstxns", subcommand_lstxns, {0},
       "usage: svnadmin lstxns REPOS_PATH\n\n"
       "Print the names of all uncommitted transactions.\n",
@@ -227,6 +232,7 @@
    svn_boolean_t follow_copies;                      /* --copies */
    svn_boolean_t quiet;                              /* --quiet */
    svn_boolean_t bdb_txn_nosync;                     /* --bdb-txn-nosync */
+  svn_boolean_t only_unused;                        /* --only-unused */
    enum svn_repos_load_uuid uuid_action;             /* --ignore-uuid,
                                                         --force-uuid */
    const char *on_disk;
@@ -490,23 +496,27 @@

  /* This implements `svn_opt_subcommand_t'. */
  svn_error_t *
-subcommand_archive (apr_getopt_t *os, void *baton, apr_pool_t *pool)
+subcommand_lsdblogs (apr_getopt_t *os, void *baton, apr_pool_t *pool)
  {
-  svn_repos_t *repos;
    struct svnadmin_opt_state *opt_state = baton;
-  char **logfiles;
-  char **filename;
+  apr_array_header_t *logfiles;
+  int log;

-  SVN_ERR (svn_repos_open (&repos, opt_state->repository_path, pool));
+  SVN_ERR (svn_repos_db_logfiles (&logfiles,
+                                  opt_state->repository_path,
+                                  opt_state->only_unused,
+                                  pool));

-  SVN_ERR (svn_fs_berkeley_archive (&logfiles,
-                                    svn_repos_db_env (repos, pool), pool));
-
    if (logfiles == NULL)
-    return SVN_NO_ERROR;
+    {
+      return SVN_NO_ERROR;
+    }

-  for (filename = logfiles; *filename != NULL; ++filename)
-    printf ("%s\n", *filename);
+  /* Loop, printing log files. */
+  for (log = 0; log < logfiles->nelts; log++)
+    {
+      printf ("%s\n", APR_ARRAY_IDX (logfiles, log, const char *));
+    }

    return SVN_NO_ERROR;
  }
@@ -750,6 +760,9 @@
        case svnadmin__bdb_txn_nosync:
          opt_state.bdb_txn_nosync = TRUE;
          break;
+      case svnadmin__only_unused:
+        opt_state.only_unused = TRUE;
+        break;
        default:
          {
            subcommand_help (NULL, NULL, pool);
Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h	(revision 6656)
+++ subversion/include/svn_fs.h	(working copy)
@@ -193,19 +193,22 @@
                                        apr_pool_t *pool);


-/** Return a NULL-terminated array of absolute paths to @a logfiles
- * that are no longer is use by the DB-based Subversion filesystem,
- * stored in the environment @a path.  Do any necessary allocation
- * within @a pool.
+/** Set @a *logfiles to array of <tt>const char *</tt> log file names
+ * of Berkeley DB-based Subversion filesystem.
   *
- * This function wraps the same Berkeley DB 'log_archive' function
+ * @a only_unused specifies whether to include used log file names in the list:
+ * TRUE   - Only unused logs are included
+ * FALSE  - All logs are included
+ *
+ * This function wraps the Berkeley DB 'log_archive' function
   * called by the db_archive binary.  Repository administrators may
   * want to run this function periodically and delete the unused log
   * files, as a way of reclaiming disk space.
   */
-svn_error_t *svn_fs_berkeley_archive (char ***logfiles,
-                                      const char *path,
-                                      apr_pool_t *pool);
+svn_error_t *svn_fs_berkeley_logfiles (apr_array_header_t **logfiles,
+                                       const char *path,
+                                       svn_boolean_t only_unused,
+                                       apr_pool_t *pool);


  /** @} */
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h	(revision 6656)
+++ subversion/include/svn_repos.h	(working copy)
@@ -108,6 +108,17 @@
  svn_error_t *svn_repos_recover (const char *path, apr_pool_t *pool);


+/** This function is a wrapper around svn_fs_berkeley_logfiles(),
+ * returning log file paths relative to the root of the repository.
+ *
+ * @copydoc svn_fs_berkeley_logfiles()
+ */
+svn_error_t *svn_repos_db_logfiles (apr_array_header_t **logfiles,
+                                    const char *path,
+                                    svn_boolean_t only_unused,
+                                    apr_pool_t *pool);
+
+
  
  /* Repository Paths */

Index: subversion/libsvn_fs/fs.c
===================================================================
--- subversion/libsvn_fs/fs.c	(revision 6656)
+++ subversion/libsvn_fs/fs.c	(working copy)
@@ -651,16 +651,19 @@
  /* Running the 'archive' command on a Berkeley DB-based filesystem.  */


-svn_error_t *
-svn_fs_berkeley_archive (char ***logfiles,
-                         const char *path,
-                         apr_pool_t *pool)
+svn_error_t *svn_fs_berkeley_logfiles (apr_array_header_t **logfiles,
+                                       const char *path,
+                                       svn_boolean_t only_unused,
+                                       apr_pool_t *pool)
  {
    int db_err;
    DB_ENV *env;
    const char *path_native;
    char **filelist;
+  char **filename;

+  *logfiles = NULL;
+
    db_err = db_env_create (&env, 0);
    if (db_err)
      return svn_fs__bdb_dberr (db_err);
@@ -673,16 +676,46 @@
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

-  db_err = env->log_archive (env, &filelist,
-                             DB_ARCH_ABS /* return absolute paths */);
+  {
+    u_int32_t flags = 0;
+
+    if(only_unused == FALSE)
+      {
+        flags |= DB_ARCH_LOG;
+      }
+
+    db_err = env->log_archive (env, &filelist, flags);
+  }
+
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

+  if (filelist == NULL)
+    {
+      db_err = env->close (env, 0);
+      if (db_err)
+        return svn_fs__bdb_dberr (db_err);
+
+      return SVN_NO_ERROR;
+    }
+
+  {
+    *logfiles = apr_array_make (pool, 1, sizeof (const char *));
+
+    for (filename = filelist; *filename != NULL; ++filename)
+      {
+        APR_ARRAY_PUSH (*logfiles, const char *) =  (apr_pstrdup(pool, *filename));
+      }
+
+    /* allocate_env sets malloc and free as the memory management functions,
+       therefore we use free to release memory allocated by DB_ENV */
+    free(filelist);
+  }
+
    db_err = env->close (env, 0);
    if (db_err)
      return svn_fs__bdb_dberr (db_err);

-  *logfiles = filelist;
    return SVN_NO_ERROR;
  }

Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c	(revision 6656)
+++ subversion/libsvn_repos/repos.c	(working copy)
@@ -1261,3 +1261,36 @@

    return SVN_NO_ERROR;
  }
+
+svn_error_t *svn_repos_db_logfiles (apr_array_header_t **logfiles,
+                                    const char *path,
+                                    svn_boolean_t only_unused,
+                                    apr_pool_t *pool)
+{
+  svn_repos_t *repos;
+  int log;
+
+  SVN_ERR (get_repos (&repos, path,
+                      APR_FLOCK_SHARED,
+                      FALSE,     /* Do not open fs. */
+                      pool));
+
+  SVN_ERR (svn_fs_berkeley_logfiles (logfiles,
+                                    svn_repos_db_env (repos, pool),
+                                    only_unused,
+                                    pool));
+
+  if (logfiles == NULL)
+    {
+      return SVN_NO_ERROR;
+    }
+
+  /* Loop, printing log files. */
+  for (log = 0; log < (*logfiles)->nelts; log++)
+    {
+      const char ** log_file = &(APR_ARRAY_IDX (*logfiles, log, const char *));
+      *log_file = svn_path_join(SVN_REPOS__DB_DIR, *log_file, pool);
+    }
+
+  return SVN_NO_ERROR;
+}



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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Michael Wood <mw...@its.uct.ac.za>.
On Tue, Aug 05, 2003 at 03:50:10PM -0500, cmpilato@collab.net wrote:
> Vladimir Berezniker <vm...@tigris.org> writes:
> 
[snip]
> > Is there a particular reason you want a list of only used log files?
> 
> Once again, we're running up against a place where a clean API
> contradicts the main use-case.  To name a function
> svn_repos_logfiles() implies that those log files are actually
> interesting.  It's a weird scenario, if you think about it.  We are
> trying to name function which return a list of things we *want*, but
> those things are the names of files that we *don't want*.  :-)
> 
> You know, I can't bring myself to care about this.  Do what you think
> is right.
> 
> > Any chance end users might think "lslogs" has something to do with log
> > messages? See above above regarding the "[--ignore-unused]" part.
> 
> Yes, good point.  "lsdblogs" ?

lsdblogs is OK, I suppose, but the main use-case for this is to remove
the unnecessary Berkeley DB log files.  How am I supposed to remove
these files if the svnadmin lsdblogs command gives me all of the logs,
including the ones that are still in use?  Oh, that's right, I first run
svnadmin lsdblogs to get all of the logs, then I run svnadmin lsdblogs
--ignore-unused to get the ones that are still in use and compare the
lists to find the ones I can safely delete!

What's the point of that? :)

What would make *much* more sense is for the default to list only the
unused logs and a switch to list all of them.  (Which is what Vladimir
was advocating.)

By the way, is svnadmin always only going to operate on Berkeley DB
based Subversion repositories?

-- 
Michael Wood <mw...@its.uct.ac.za>

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Vladimir Berezniker <vm...@tigris.org> writes:

> > I still say it's kinda silly to have a parameter to toggle this since
> > we can always run over a list of relative paths and convert them to
> > absolute ones.  Nevermind that Berkeley's log_archive() function would
> > be the one doing this work for us anyway -- from an API point of view,
> > it's just noise.
> >
> 
> You are absolutely right, I misunderstood what you said last time. I
> read your comment to mean "returning absolute paths all the time", my
> mistake.

Well, even that would work fine.  Relative path in that case would be:

   REPOS_PATH + "/db/" + svn_path_basename(abs_log_path);

But still, I say just fetch the names.

> How about another function, svn_repos_logfiles() that wraps
> svn_fs_berkeley_logfiles() and returns paths relative to the
> repository root.

Yes.  This is the right thing to do.  Doesn't bother me a bit.

> >>     *
> >>     * @a all_log_files specifies which logs to include in the list:
> >>     * TRUE   - All logs are included
> >>     * FALSE  - Only unused logs are included
> > I'd rather make the default answer contain all the log files, and
> > then have a parameter named 'ignore_unused' to toggle that behavior.
> >
> 
> As I understand Berkeley DB, log_archive can return either list of
> unused logs or a list of used and unused log files.
> 
> Of course a function that does (used and unused log files) - (unused
> log files) = (used log files) can be implemented.
> 
> Is there a particular reason you want a list of only used log files?

Once again, we're running up against a place where a clean API
contradicts the main use-case.  To name a function
svn_repos_logfiles() implies that those log files are actually
interesting.  It's a weird scenario, if you think about it.  We are
trying to name function which return a list of things we *want*, but
those things are the names of files that we *don't want*.  :-)

You know, I can't bring myself to care about this.  Do what you think
is right.

> Any chance end users might think "lslogs" has something to do with log
> messages? See above above regarding the "[--ignore-unused]" part.

Yes, good point.  "lsdblogs" ?

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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by Vladimir Berezniker <vm...@tigris.org>.
cmpilato@collab.net wrote:
> Vladimir Berezniker <vm...@tigris.org> writes:
> 
> 
>>A). Add two new  parameters to  svn_fs_berkeley_archive to look as follows:
>>
>>     /** Set @a *logfiles to array of <tt>const char *</tt> logfile paths
>>      * of Berkeley DB-based Subversion filesystem.
> 
> 
> This function is misnamed.  It should be svn_fs_berkeley_logfiles().
> 
Will be done ...
> 
>>      *
>>      * @a absolute_paths specifies format of the paths:
>>      * TRUE   - Paths are to be absolute
>>      * FALSE  - Paths are to be relative to the Berkeley DB logs directory.
> 
> 
> I still say it's kinda silly to have a parameter to toggle this since
> we can always run over a list of relative paths and convert them to
> absolute ones.  Nevermind that Berkeley's log_archive() function would
> be the one doing this work for us anyway -- from an API point of view,
> it's just noise.
> 

You are absolutely right, I misunderstood what you said last time. I 
read your comment to mean "returning absolute paths all the time", my 
mistake.

How about another function, svn_repos_logfiles() that wraps 
svn_fs_berkeley_logfiles() and returns paths relative to the repository 
root.

E.g. If svn_fs_berkeley_logfiles() returns log.00001, log.00002 then 
svn_repos_logfiles() returns db/log.00001, db/log.00002.

Then svnadmin will wrap around svn_repos_logfiles().

> 
>>     *
>>     * @a all_log_files specifies which logs to include in the list:
>>     * TRUE   - All logs are included
>>     * FALSE  - Only unused logs are included
> 
> 
> I'd rather make the default answer contain all the log files, and then
> have a parameter named 'ignore_unused' to toggle that behavior.
> 

As I understand Berkeley DB, log_archive can return either list of 
unused logs or a list of used and unused log files.

Of course a function that does (used and unused log files) - (unused log 
files) = (used log files) can be implemented.

Is there a particular reason you want a list of only used log files?


> 
>>B) Modify svnadmin's main.c to allow expose the new API to the end user.
> 
> 
> Sure.
> 
> 
>>A). Name of the "archive" command.
>>
>>     As Michael Wood pointed out that "archive" is not the best name
> 
> 
> Right.  How about "svnadmin lslogs [--ignore-unused]" ?

Any chance end users might think "lslogs" has something to do with log 
messages? See above above regarding the "[--ignore-unused]" part.

> 
> 
>>     * If command is renamed does svn_fs_berkeley_archive  get renamed
>>as  well or a new function is created (e.g. svn_fs_berkeley_loglist)
>>and svn_fs_berkeley_archive calls the new function so that any
>>existing code works without a change.?
> 
> 
> Yes.  As I noted above, the function is misnamed.  (And, IMO, so is
> the Berkeley function and utility).
> 
> 
>>B). Need for absolute_paths parameter.
>>
>>     cmpilato@collab.net said: "I don't really see the need to have
>>the abs-vs.-rel path parameter", referring to absolute_paths
>>parameter.
>>http://article.gmane.org/gmane.comp.version-control.subversion.devel/31725
> 
> 
> See above.
> 
> 
>>      * Given that I also want to submit a patch for "svnadmin copy"
>>(hot copy) command that will need log file names only, is that a
>>reason enough to keep that parameter? You can read about why I wish to
>>compare log files at
>>http://article.gmane.org/gmane.comp.version-control.subversion.devel/31740
> 
> 
> Make the function return just the basename (which is how I understand
> BDB's log_archive() function to work), and convert to absolute path
> when you need to.

Thank you for your feedback,
Vladimir Berezniker


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

Re: Enhancing and possibly renaming svnadmin archive command

Posted by cm...@collab.net.
Vladimir Berezniker <vm...@tigris.org> writes:

> A). Add two new  parameters to  svn_fs_berkeley_archive to look as follows:
> 
>      /** Set @a *logfiles to array of <tt>const char *</tt> logfile paths
>       * of Berkeley DB-based Subversion filesystem.

This function is misnamed.  It should be svn_fs_berkeley_logfiles().

>       *
>       * @a absolute_paths specifies format of the paths:
>       * TRUE   - Paths are to be absolute
>       * FALSE  - Paths are to be relative to the Berkeley DB logs directory.

I still say it's kinda silly to have a parameter to toggle this since
we can always run over a list of relative paths and convert them to
absolute ones.  Nevermind that Berkeley's log_archive() function would
be the one doing this work for us anyway -- from an API point of view,
it's just noise.

>      *
>      * @a all_log_files specifies which logs to include in the list:
>      * TRUE   - All logs are included
>      * FALSE  - Only unused logs are included

I'd rather make the default answer contain all the log files, and then
have a parameter named 'ignore_unused' to toggle that behavior.

> B) Modify svnadmin's main.c to allow expose the new API to the end user.

Sure.

> A). Name of the "archive" command.
> 
>      As Michael Wood pointed out that "archive" is not the best name

Right.  How about "svnadmin lslogs [--ignore-unused]" ?

>      * If command is renamed does svn_fs_berkeley_archive  get renamed
> as  well or a new function is created (e.g. svn_fs_berkeley_loglist)
> and svn_fs_berkeley_archive calls the new function so that any
> existing code works without a change.?

Yes.  As I noted above, the function is misnamed.  (And, IMO, so is
the Berkeley function and utility).

> B). Need for absolute_paths parameter.
> 
>      cmpilato@collab.net said: "I don't really see the need to have
> the abs-vs.-rel path parameter", referring to absolute_paths
> parameter.
> http://article.gmane.org/gmane.comp.version-control.subversion.devel/31725

See above.

>       * Given that I also want to submit a patch for "svnadmin copy"
> (hot copy) command that will need log file names only, is that a
> reason enough to keep that parameter? You can read about why I wish to
> compare log files at
> http://article.gmane.org/gmane.comp.version-control.subversion.devel/31740

Make the function return just the basename (which is how I understand
BDB's log_archive() function to work), and convert to absolute path
when you need to.

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