You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/04/06 20:42:07 UTC

svn commit: r1738021 - in /subversion/trunk: subversion/svnadmin/svnadmin.c subversion/tests/cmdline/svnadmin_tests.py tools/client-side/bash_completion

Author: kotkov
Date: Wed Apr  6 18:42:07 2016
New Revision: 1738021

URL: http://svn.apache.org/viewvc?rev=1738021&view=rev
Log:
Introduce `--file' option for svnadmin dump, dump-revprops, load and
load-revprops subcommands.

This option allows specifying input or output files, instead of using STDIN
or STDOUT.  Using --file (-F) should probably work a bit faster, but, what
is more important, this option allows working with dump files in a shell
that performs additional processing when piping data and thus can silently
corrupt the dumps.  For instance, that's how Microsoft PowerShell currently
works.

* subversion/svnadmin/svnadmin.c
  (options_table): Propagate existing description for -F ...
  (cmd_table): ...here.  Accept -F in the dump, dump-revprops, load
   and load-revprops subcommands and provide corresponding description
   overrides.
  (svnadmin_opt_state): Replace the `filedata' field with `file'.
  (sub_main): Remember just the filename given with -F, instead of reading
   and saving the content of that file to opt_state.
  (subcommand_freeze): Open opt_state->file and parse its content if
   necessary.
  (subcommand_dump, subcommand_dump_revprops): Open opt_state->file
   or STDIN, depending on whether -F was specified.  Overwrite existing
   files to match the behavior of svnadmin dump > file.
  (subcommand_load, subcommand_load_revprops): Open opt_state->file
   or STDOUT, depending on whether -F was specified.

* subversion/tests/cmdline/svnadmin_tests.py
  (dump_to_file, load_from_file): New tests.
  (test_list): Add reference to new tests.

* tools/client-side/bash_completion
  (_svnadmin): Extend completion info for `load' and `dump'.

Modified:
    subversion/trunk/subversion/svnadmin/svnadmin.c
    subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
    subversion/trunk/tools/client-side/bash_completion

Modified: subversion/trunk/subversion/svnadmin/svnadmin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/svnadmin.c?rev=1738021&r1=1738020&r2=1738021&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/svnadmin.c (original)
+++ subversion/trunk/subversion/svnadmin/svnadmin.c Wed Apr  6 18:42:07 2016
@@ -330,7 +330,8 @@ static const svn_opt_subcommand_desc2_t
     "every path present in the repository as of that revision.  (In either\n"
     "case, the second and subsequent revisions, if any, describe only paths\n"
     "changed in those revisions.)\n"),
-  {'r', svnadmin__incremental, svnadmin__deltas, 'q', 'M'} },
+  {'r', svnadmin__incremental, svnadmin__deltas, 'q', 'M', 'F'},
+  {{'F', N_("write to file ARG instead of stdout")}} },
 
   {"dump-revprops", subcommand_dump_revprops, {0}, N_
    ("usage: svnadmin dump-revprops REPOS_PATH [-r LOWER[:UPPER]]\n\n"
@@ -339,7 +340,8 @@ static const svn_opt_subcommand_desc2_t
     "LOWER rev through UPPER rev.  If no revisions are given, dump the\n"
     "properties for all revisions.  If only LOWER is given, dump the\n"
     "properties for that one revision.\n"),
-  {'r', 'q'} },
+  {'r', 'q', 'F'},
+  {{'F', N_("write to file ARG instead of stdout")}} },
 
   {"freeze", subcommand_freeze, {0}, N_
    ("usage: 1. svnadmin freeze REPOS_PATH PROGRAM [ARG...]\n"
@@ -349,7 +351,8 @@ static const svn_opt_subcommand_desc2_t
     "2. Like 1 except all repositories listed in FILE are locked. The file\n"
     "   format is repository paths separated by newlines.  Repositories are\n"
     "   locked in the same order as they are listed in the file.\n"),
-   {'F'} },
+   {'F'},
+   {{'F', N_("read repository paths from file ARG")}} },
 
   {"help", subcommand_help, {"?", "h"}, N_
    ("usage: svnadmin help [SUBCOMMAND...]\n\n"
@@ -392,7 +395,8 @@ static const svn_opt_subcommand_desc2_t
     svnadmin__ignore_dates,
     svnadmin__use_pre_commit_hook, svnadmin__use_post_commit_hook,
     svnadmin__parent_dir, svnadmin__bypass_prop_validation, 'M',
-    svnadmin__no_flush_to_disk} },
+    svnadmin__no_flush_to_disk, 'F'},
+   {{'F', N_("read from file ARG instead of stdin")}} },
 
   {"load-revprops", subcommand_load_revprops, {0}, N_
    ("usage: svnadmin load-revprops REPOS_PATH\n\n"
@@ -402,7 +406,8 @@ static const svn_opt_subcommand_desc2_t
     "If --revision is specified, limit the loaded revisions to only those\n"
     "in the dump stream whose revision numbers match the specified range.\n"),
    {'q', 'r', svnadmin__force_uuid, svnadmin__bypass_prop_validation,
-    svnadmin__no_flush_to_disk} },
+    svnadmin__no_flush_to_disk, 'F'},
+   {{'F', N_("read from file ARG instead of stdin")}} },
 
   {"lock", subcommand_lock, {0}, N_
    ("usage: svnadmin lock REPOS_PATH PATH USERNAME COMMENT-FILE [TOKEN]\n\n"
@@ -544,7 +549,7 @@ struct svnadmin_opt_state
                                                        --force-uuid */
   apr_uint64_t memory_cache_size;                   /* --memory-cache-size M */
   const char *parent_dir;                           /* --parent-dir */
-  svn_stringbuf_t *filedata;                        /* --file */
+  const char *file;                                 /* --file */
 
   const char *config_dir;    /* Overriding Configuration Directory */
 };
@@ -1234,7 +1239,7 @@ subcommand_dump(apr_getopt_t *os, void *
 {
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
-  svn_stream_t *stdout_stream;
+  svn_stream_t *out_stream;
   svn_revnum_t lower, upper;
   svn_stream_t *feedback_stream = NULL;
 
@@ -1244,13 +1249,25 @@ subcommand_dump(apr_getopt_t *os, void *
   SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(get_dump_range(&lower, &upper, repos, opt_state, pool));
 
-  SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
+  /* Open the file or STDOUT, depending on whether -F was specified. */
+  if (opt_state->file)
+    {
+      apr_file_t *file;
+
+      /* Overwrite existing files, same as with > redirection. */
+      SVN_ERR(svn_io_file_open(&file, opt_state->file,
+                               APR_WRITE | APR_CREATE | APR_TRUNCATE
+                               | APR_BUFFERED, APR_OS_DEFAULT, pool));
+      out_stream = svn_stream_from_aprfile2(file, FALSE, pool);
+    }
+  else
+    SVN_ERR(svn_stream_for_stdout(&out_stream, pool));
 
   /* Progress feedback goes to STDERR, unless they asked to suppress it. */
   if (! opt_state->quiet)
     feedback_stream = recode_stream_create(stderr, pool);
 
-  SVN_ERR(svn_repos_dump_fs4(repos, stdout_stream, lower, upper,
+  SVN_ERR(svn_repos_dump_fs4(repos, out_stream, lower, upper,
                              opt_state->incremental, opt_state->use_deltas,
                              TRUE, TRUE,
                              !opt_state->quiet ? repos_notify_handler : NULL,
@@ -1265,7 +1282,7 @@ subcommand_dump_revprops(apr_getopt_t *o
 {
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
-  svn_stream_t *stdout_stream;
+  svn_stream_t *out_stream;
   svn_revnum_t lower, upper;
   svn_stream_t *feedback_stream = NULL;
 
@@ -1275,13 +1292,25 @@ subcommand_dump_revprops(apr_getopt_t *o
   SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(get_dump_range(&lower, &upper, repos, opt_state, pool));
 
-  SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
+  /* Open the file or STDOUT, depending on whether -F was specified. */
+  if (opt_state->file)
+    {
+      apr_file_t *file;
+
+      /* Overwrite existing files, same as with > redirection. */
+      SVN_ERR(svn_io_file_open(&file, opt_state->file,
+                               APR_WRITE | APR_CREATE | APR_TRUNCATE
+                               | APR_BUFFERED, APR_OS_DEFAULT, pool));
+      out_stream = svn_stream_from_aprfile2(file, FALSE, pool);
+    }
+  else
+    SVN_ERR(svn_stream_for_stdout(&out_stream, pool));
 
   /* Progress feedback goes to STDERR, unless they asked to suppress it. */
   if (! opt_state->quiet)
     feedback_stream = recode_stream_create(stderr, pool);
 
-  SVN_ERR(svn_repos_dump_fs4(repos, stdout_stream, lower, upper,
+  SVN_ERR(svn_repos_dump_fs4(repos, out_stream, lower, upper,
                              FALSE, FALSE, TRUE, FALSE,
                              !opt_state->quiet ? repos_notify_handler : NULL,
                              feedback_stream, check_cancel, NULL, pool));
@@ -1336,7 +1365,7 @@ subcommand_freeze(apr_getopt_t *os, void
     return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, 0,
                             _("No program provided"));
 
-  if (!opt_state->filedata)
+  if (!opt_state->file)
     {
       /* One repository on the command line. */
       paths = apr_array_make(pool, 1, sizeof(const char *));
@@ -1344,9 +1373,11 @@ subcommand_freeze(apr_getopt_t *os, void
     }
   else
     {
+      svn_stringbuf_t *buf;
       const char *utf8;
-      /* All repositories in filedata. */
-      SVN_ERR(svn_utf_cstring_to_utf8(&utf8, opt_state->filedata->data, pool));
+      /* Read repository paths from the -F file. */
+      SVN_ERR(svn_stringbuf_from_file2(&buf, opt_state->file, pool));
+      SVN_ERR(svn_utf_cstring_to_utf8(&utf8, buf->data, pool));
       paths = svn_cstring_split(utf8, "\r\n", FALSE, pool);
     }
 
@@ -1465,7 +1496,7 @@ subcommand_load(apr_getopt_t *os, void *
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
   svn_revnum_t lower, upper;
-  svn_stream_t *stdin_stream;
+  svn_stream_t *in_stream;
   svn_stream_t *feedback_stream = NULL;
 
   /* Expect no more arguments. */
@@ -1477,14 +1508,18 @@ subcommand_load(apr_getopt_t *os, void *
 
   SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
-  /* Read the stream from STDIN.  Users can redirect a file. */
-  SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
+  /* Open the file or STDIN, depending on whether -F was specified. */
+  if (opt_state->file)
+    SVN_ERR(svn_stream_open_readonly(&in_stream, opt_state->file,
+                                     pool, pool));
+  else
+    SVN_ERR(svn_stream_for_stdin2(&in_stream, TRUE, pool));
 
   /* Progress feedback goes to STDOUT, unless they asked to suppress it. */
   if (! opt_state->quiet)
     feedback_stream = recode_stream_create(stdout, pool);
 
-  err = svn_repos_load_fs5(repos, stdin_stream, lower, upper,
+  err = svn_repos_load_fs5(repos, in_stream, lower, upper,
                            opt_state->uuid_action, opt_state->parent_dir,
                            opt_state->use_pre_commit_hook,
                            opt_state->use_post_commit_hook,
@@ -1508,7 +1543,7 @@ subcommand_load_revprops(apr_getopt_t *o
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
   svn_revnum_t lower, upper;
-  svn_stream_t *stdin_stream;
+  svn_stream_t *in_stream;
 
   svn_stream_t *feedback_stream = NULL;
 
@@ -1521,14 +1556,18 @@ subcommand_load_revprops(apr_getopt_t *o
 
   SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
-  /* Read the stream from STDIN.  Users can redirect a file. */
-  SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
+  /* Open the file or STDIN, depending on whether -F was specified. */
+  if (opt_state->file)
+    SVN_ERR(svn_stream_open_readonly(&in_stream, opt_state->file,
+                                     pool, pool));
+  else
+    SVN_ERR(svn_stream_for_stdin2(&in_stream, TRUE, pool));
 
   /* Progress feedback goes to STDOUT, unless they asked to suppress it. */
   if (! opt_state->quiet)
     feedback_stream = recode_stream_create(stdout, pool);
 
-  err = svn_repos_load_fs_revprops(repos, stdin_stream, lower, upper,
+  err = svn_repos_load_fs_revprops(repos, in_stream, lower, upper,
                                    !opt_state->bypass_prop_validation,
                                    opt_state->ignore_dates,
                                    opt_state->quiet ? NULL
@@ -2682,9 +2721,7 @@ sub_main(int *exit_code, int argc, const
             = 0x100000 * apr_strtoi64(opt_arg, NULL, 0);
         break;
       case 'F':
-        SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
-        SVN_ERR(svn_stringbuf_from_file2(&(opt_state.filedata),
-                                             utf8_opt_arg, pool));
+        SVN_ERR(svn_utf_cstring_to_utf8(&(opt_state.file), opt_arg, pool));
         dash_F_arg = TRUE;
         break;
       case svnadmin__version:

Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1738021&r1=1738020&r2=1738021&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Wed Apr  6 18:42:07 2016
@@ -3384,6 +3384,51 @@ def load_no_flush_to_disk(sbox):
   load_and_verify_dumpstream(sbox, [], [], expected, True, dump,
                              '--no-flush-to-disk', '--ignore-uuid')
 
+def dump_to_file(sbox):
+  "svnadmin dump --file ARG"
+
+  sbox.build(create_wc=False, empty=False)
+  expected_dump = svntest.actions.run_and_verify_dump(sbox.repo_dir)
+
+  file = sbox.get_tempname()
+  svntest.actions.run_and_verify_svnadmin2([],
+                                           ["* Dumped revision 0.\n",
+                                            "* Dumped revision 1.\n"],
+                                           0, 'dump', '--file', file,
+                                           sbox.repo_dir)
+  actual_dump = open(file, 'rb').readlines()
+  svntest.verify.compare_dump_files(None, None, expected_dump, actual_dump)
+
+  # Test that svnadmin dump --file overwrites existing files.
+  file = sbox.get_tempname()
+  svntest.main.file_write(file, '')
+  svntest.actions.run_and_verify_svnadmin2([],
+                                           ["* Dumped revision 0.\n",
+                                            "* Dumped revision 1.\n"],
+                                           0, 'dump', '--file', file,
+                                           sbox.repo_dir)
+  actual_dump = open(file, 'rb').readlines()
+  svntest.verify.compare_dump_files(None, None, expected_dump, actual_dump)
+
+def load_from_file(sbox):
+  "svnadmin load --file ARG"
+
+  sbox.build(empty=True)
+
+  file = sbox.get_tempname()
+  open(file, 'wb').writelines(clean_dumpfile())
+  svntest.actions.run_and_verify_svnadmin2(None, [],
+                                           0, 'load', '--file', file,
+                                           '--ignore-uuid', sbox.repo_dir)
+  expected_tree = \
+    svntest.wc.State('', {
+      'A' : svntest.wc.StateItem(contents="text\n",
+                                 props={'svn:keywords': 'Id'})
+      })
+  svntest.actions.run_and_verify_svn(svntest.verify.AnyOutput, [],
+                                     'update', sbox.wc_dir)
+  svntest.actions.verify_disk(sbox.wc_dir, expected_tree, check_props=True)
+
 ########################################################################
 # Run the tests
 
@@ -3446,7 +3491,9 @@ test_list = [ None,
               dump_revprops,
               dump_no_op_change,
               dump_no_op_prop_change,
-              load_no_flush_to_disk
+              load_no_flush_to_disk,
+              dump_to_file,
+              load_from_file
              ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/tools/client-side/bash_completion
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/client-side/bash_completion?rev=1738021&r1=1738020&r2=1738021&view=diff
==============================================================================
--- subversion/trunk/tools/client-side/bash_completion (original)
+++ subversion/trunk/tools/client-side/bash_completion Wed Apr  6 18:42:07 2016
@@ -1136,7 +1136,7 @@ _svnadmin ()
 		;;
 	dump)
 		cmdOpts="-r --revision --incremental -q --quiet --deltas \
-		         -M --memory-cache-size"
+		         -M --memory-cache-size -F --file"
 		;;
 	freeze)
 		cmdOpts="-F --file"
@@ -1151,7 +1151,7 @@ _svnadmin ()
 		cmdOpts="--ignore-uuid --force-uuid --parent-dir -q --quiet \
 		         --use-pre-commit-hook --use-post-commit-hook \
 		         --bypass-prop-validation -M --memory-cache-size \
-		         --no-flush-to-disk"
+		         --no-flush-to-disk -F --file"
 		;;
 	lstxns)
         	cmdOpts="-r --revision"



Re: svn commit: r1738021 - in /subversion/trunk: subversion/svnadmin/svnadmin.c subversion/tests/cmdline/svnadmin_tests.py tools/client-side/bash_completion

Posted by Branko Čibej <br...@apache.org>.
On 07.04.2016 12:31, Evgeny Kotkov wrote:
> Branko Čibej <br...@apache.org> writes:
>
>> PowerShell fiddles with redirected stdout? That's ... scary!
>>
>> Out of interest, do you know what it does with the data in the stream
>> and why?
> It assumes that commands output text, and automatically splits the output
> based on line endings:
>
>   $out = svnadmin dump C:\Repositories\test
>   $out.GetType()  # Object[]
>   $out.Length  # 14
>   $out[0].GetType()  # String
>   $out[0]  # SVN-fs-dump-format-version: 2
>
> Redirection is implemented as pipelining the array of strings to the Out-File
> command.  This command reconstitutes the text and translates it based on
> the current encoding, and that produces unreadable dumps.
>
> In case you're interested, there's an in depth explanation available in [1].
>
> I am not too sure about the "why" part, but maybe assuming everything as
> text has its benefits for the majority of commands and cmdlets.  Or perhaps,
> this is just a side effect of making > work in a predictable way for both
> PowerShell commands and executables.

Thanks for the explanation. It's kind of painful ... Although adding
--file makes perfect sense in any case.

-- Brane

Re: svn commit: r1738021 - in /subversion/trunk: subversion/svnadmin/svnadmin.c subversion/tests/cmdline/svnadmin_tests.py tools/client-side/bash_completion

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Branko Čibej <br...@apache.org> writes:

> PowerShell fiddles with redirected stdout? That's ... scary!
>
> Out of interest, do you know what it does with the data in the stream
> and why?

It assumes that commands output text, and automatically splits the output
based on line endings:

  $out = svnadmin dump C:\Repositories\test
  $out.GetType()  # Object[]
  $out.Length  # 14
  $out[0].GetType()  # String
  $out[0]  # SVN-fs-dump-format-version: 2

Redirection is implemented as pipelining the array of strings to the Out-File
command.  This command reconstitutes the text and translates it based on
the current encoding, and that produces unreadable dumps.

In case you're interested, there's an in depth explanation available in [1].

I am not too sure about the "why" part, but maybe assuming everything as
text has its benefits for the majority of commands and cmdlets.  Or perhaps,
this is just a side effect of making > work in a predictable way for both
PowerShell commands and executables.

[1] http://brianreiter.org/2010/01/29/powershells-object-pipeline-corrupts-piped-binary-data


Regards,
Evgeny Kotkov

Re: svn commit: r1738021 - in /subversion/trunk: subversion/svnadmin/svnadmin.c subversion/tests/cmdline/svnadmin_tests.py tools/client-side/bash_completion

Posted by Branko Čibej <br...@apache.org>.
On 06.04.2016 20:42, kotkov@apache.org wrote:
> Author: kotkov
> Date: Wed Apr  6 18:42:07 2016
> New Revision: 1738021
>
> URL: http://svn.apache.org/viewvc?rev=1738021&view=rev
> Log:
> Introduce `--file' option for svnadmin dump, dump-revprops, load and
> load-revprops subcommands.
>
> This option allows specifying input or output files, instead of using STDIN
> or STDOUT.  Using --file (-F) should probably work a bit faster, but, what
> is more important, this option allows working with dump files in a shell
> that performs additional processing when piping data and thus can silently
> corrupt the dumps.  For instance, that's how Microsoft PowerShell currently
> works.


PowerShell fiddles with redirected stdout? That's ... scary!

Out of interest, do you know what it does with the data in the stream
and why?

-- Brane