Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> writes:
> That is indeed a good idea, so I tried it in various ways:
>
> Index: subversion/libsvn_subr/stream.c
> ===================================================================
> --- subversion/libsvn_subr/stream.c (revision 1701330)
> +++ subversion/libsvn_subr/stream.c (working copy)
> @@ -1826,10 +1826,20 @@ svn_stream_for_stdin(svn_stream_t **in, apr_pool_t
> apr_status_t apr_err;
>
> apr_err = apr_file_open_stdin(&stdin_file, pool);
> +
> +/* Alternatively to the above: tell APR to create a buffered file
> + apr_err = apr_file_open_flags_stdin(&stdin_file, APR_BUFFERED, pool);
> +*/
> if (apr_err)
> return svn_error_wrap_apr(apr_err, "Can't open stdin");
The original commit begins using svn_stream_wrap_buffered_read() during
svnadmin load-revprops and svnfsfs load-index. This patch, however, does
something entirely different and adds buffering to *every* stdin.
I am not sure why would we want to do this, but there certainly is a reason
against a change like this — buffering can't be used when something interactive
happens using stdin, e.g., with svnserve.
So, why can't we enable buffering for these subcommands (perhaps including
svnadmin load), leave everything else as is and avoid the necessity of having
and maintaining the custom buffered stream adapter?
We could introduce svn_stream_for_stdin2(..., svn_boolean_t buffered, ...),
and then the required change for svnadmin load-revprops would look as below
(changes for other subcommands would look almost exactly the same):
[[[
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 1701648)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -1820,12 +1820,20 @@ svn_stream_wrap_buffered_read(svn_stream_t *inner,
svn_error_t *
-svn_stream_for_stdin(svn_stream_t **in, apr_pool_t *pool)
+svn_stream_for_stdin2(svn_stream_t **in,
+ svn_boolean_t buffered,
+ apr_pool_t *pool)
{
apr_file_t *stdin_file;
apr_status_t apr_err;
+ apr_int32_t flags;
- apr_err = apr_file_open_stdin(&stdin_file, pool);
+ if (buffered)
+ flags = APR_BUFFERED;
+ else
+ flags = 0;
+
+ apr_err = apr_file_open_flags_stdin(&stdin_file, flags, pool);
if (apr_err)
return svn_error_wrap_apr(apr_err, "Can't open stdin");
Index: subversion/svnadmin/svnadmin.c
===================================================================
--- subversion/svnadmin/svnadmin.c (revision 1701648)
+++ subversion/svnadmin/svnadmin.c (working copy)
@@ -1547,8 +1547,7 @@ subcommand_load_revprops(apr_getopt_t *os, void *b
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
/* Read the stream from STDIN. Users can redirect a file. */
- SVN_ERR(svn_stream_for_stdin(&stdin_stream, pool));
- stdin_stream = svn_stream_wrap_buffered_read(stdin_stream, pool);
+ SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
/* Progress feedback goes to STDOUT, unless they asked to suppress it. */
if (! opt_state->quiet)
]]]
Regards,
Evgeny Kotkov
Received on 2015-09-07 18:39:23 CEST