Philip Martin <philip.mar...@wandisco.com> writes: > It's also hard to fix 1.8, how do we pass the information into the > client library without changing the API? Perhaps we could recognise a > special part of the command name or a special external parameter, so > > --diff-cmd svn:interactive:myscript
Another user raised the issue http://subversion.tigris.org/issues/show_bug.cgi?id=4382 Using '--diff-cmd colordiff' to get coloured output no longer works. Here's a solution that requires the user to mark the command as requiring direct access. Log and patch: Allow the user to bypass the temporary spool file when invoking an external diff command. This allows commands that expect to see a terminal to work. The user adds the prefix 'svn:direct:' to the command and Subversion passes the stream's file rather than creating a spool file. So --diff-cmd foo runs foo with a spool file and --diff-cmd svn:direct:foo runs foo with the stream's file. * subversion/include/private/svn_io_private.h (svn_stream__aprfile, svn_io__file_for_command_stream): New. * subversion/libsvn_subr/io.c (svn_io__file_for_command_stream): New. * subversion/libsvn_subr/stream.c (struct svn_stream_t): Add file member. (svn_stream_create, svn_stream_from_aprfile2): Set file member. (svn_stream__aprfile): New. * subversion/libsvn_client/diff.c (diff_content_changed): Use svn_io__file_for_command_stream. * subversion/svnlook/svnlook.c (print_diff_tree): Use svn_io__file_for_command_stream. Index: subversion/include/private/svn_io_private.h =================================================================== --- subversion/include/private/svn_io_private.h (revision 1495378) +++ subversion/include/private/svn_io_private.h (working copy) @@ -90,7 +90,29 @@ svn_stream__set_is_buffered(svn_stream_t *stream, svn_boolean_t svn_stream__is_buffered(svn_stream_t *stream); +/** Return the underlying file, if any, associated with the stream, or + * NULL if not available. Accessing the file bypasses the stream and + * should only be done when such bypass is acceptable. + */ +apr_file_t * +svn_stream__aprfile(svn_stream_t *stream); +/** Examine ORIG_COMMAND to determine whether it requests direct + * access to the file, if so then return STREAM's file in *FILE and set + * *FILENAME to NULL or return an error if the file is not available. + * Otherwise return a temporary file in *FILE and the filename in + * *FILENAME. Return the true command in * *TRUE_COMMAND. + */ +svn_error_t * +svn_io__file_for_command_stream(apr_file_t **file, + const char **filename, + const char **true_command, + svn_stream_t *stream, + const char *orig_command, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + + #ifdef __cplusplus } #endif /* __cplusplus */ Index: subversion/libsvn_client/diff.c =================================================================== --- subversion/libsvn_client/diff.c (revision 1495378) +++ subversion/libsvn_client/diff.c (working copy) @@ -51,6 +51,7 @@ #include "private/svn_wc_private.h" #include "private/svn_diff_private.h" #include "private/svn_subr_private.h" +#include "private/svn_io_private.h" #include "svn_private_config.h" @@ -793,6 +794,7 @@ diff_content_changed(svn_boolean_t *wrote_header, const char *outfilename; const char *errfilename; svn_stream_t *stream; + const char *diff_cmd; /* Print out the diff header. */ SVN_ERR(svn_stream_printf_from_utf8(outstream, @@ -807,11 +809,13 @@ diff_content_changed(svn_boolean_t *wrote_header, * ### a non-git compatible diff application.*/ /* We deal in streams, but svn_io_run_diff2() deals in file handles, - unfortunately, so we need to make these temporary files, and then + so we need to make these temporary files, and then copy the contents to our stream. */ - SVN_ERR(svn_io_open_unique_file3(&outfile, &outfilename, NULL, - svn_io_file_del_on_pool_cleanup, - scratch_pool, scratch_pool)); + SVN_ERR(svn_io__file_for_command_stream(&outfile, &outfilename, + &diff_cmd, outstream, + diff_cmd_baton->diff_cmd, + scratch_pool, scratch_pool)); + SVN_ERR(svn_io_open_unique_file3(&errfile, &errfilename, NULL, svn_io_file_del_on_pool_cleanup, scratch_pool, scratch_pool)); @@ -822,17 +826,21 @@ diff_content_changed(svn_boolean_t *wrote_header, label1, label2, tmpfile1, tmpfile2, &exitcode, outfile, errfile, - diff_cmd_baton->diff_cmd, scratch_pool)); + diff_cmd, scratch_pool)); - SVN_ERR(svn_io_file_close(outfile, scratch_pool)); + if (outfilename) + SVN_ERR(svn_io_file_close(outfile, scratch_pool)); SVN_ERR(svn_io_file_close(errfile, scratch_pool)); /* Now, open and copy our files to our output streams. */ - SVN_ERR(svn_stream_open_readonly(&stream, outfilename, - scratch_pool, scratch_pool)); - SVN_ERR(svn_stream_copy3(stream, svn_stream_disown(outstream, - scratch_pool), - NULL, NULL, scratch_pool)); + if (outfilename) + { + SVN_ERR(svn_stream_open_readonly(&stream, outfilename, + scratch_pool, scratch_pool)); + SVN_ERR(svn_stream_copy3(stream, svn_stream_disown(outstream, + scratch_pool), + NULL, NULL, scratch_pool)); + } SVN_ERR(svn_stream_open_readonly(&stream, errfilename, scratch_pool, scratch_pool)); SVN_ERR(svn_stream_copy3(stream, svn_stream_disown(errstream, Index: subversion/libsvn_subr/io.c =================================================================== --- subversion/libsvn_subr/io.c (revision 1495378) +++ subversion/libsvn_subr/io.c (working copy) @@ -2854,7 +2854,44 @@ svn_io_run_cmd(const char *path, return svn_io_wait_for_cmd(&cmd_proc, cmd, exitcode, exitwhy, pool); } +svn_error_t * +svn_io__file_for_command_stream(apr_file_t **file, + const char **filename, + const char **true_command, + svn_stream_t *stream, + const char *orig_command, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + const char direct_magic[] = "svn:direct:"; + svn_boolean_t direct_required = !strncmp(direct_magic, orig_command, + sizeof(direct_magic) - 1); + + if (direct_required) + { + /* The user doesn't want a temporary file, probably because + they want some sort of terminal. We assume they don't + care that any output from the command may not be ordered + with the stream output. */ + *file = svn_stream__aprfile(stream); + if (!*file) + return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL, + "stream does not support direct use"); + *filename = NULL; + *true_command = apr_pstrdup(result_pool, + orig_command + sizeof(direct_magic) - 1); + } + else + { + SVN_ERR(svn_io_open_unique_file3(file, filename, NULL, + svn_io_file_del_on_pool_cleanup, + scratch_pool, scratch_pool)); + *true_command = apr_pstrdup(result_pool, orig_command); + } + return SVN_NO_ERROR; +} + svn_error_t * svn_io_run_diff2(const char *dir, const char *const *user_args, Index: subversion/libsvn_subr/stream.c =================================================================== --- subversion/libsvn_subr/stream.c (revision 1495378) +++ subversion/libsvn_subr/stream.c (working copy) @@ -56,6 +56,7 @@ struct svn_stream_t { svn_stream_mark_fn_t mark_fn; svn_stream_seek_fn_t seek_fn; svn_stream__is_buffered_fn_t is_buffered_fn; + apr_file_t *file; /* Maybe NULL */ }; @@ -81,6 +82,7 @@ svn_stream_create(void *baton, apr_pool_t *pool) stream->mark_fn = NULL; stream->seek_fn = NULL; stream->is_buffered_fn = NULL; + stream->file = NULL; return stream; } @@ -913,6 +915,7 @@ svn_stream_from_aprfile2(apr_file_t *file, svn_stream_set_mark(stream, mark_handler_apr); svn_stream_set_seek(stream, seek_handler_apr); svn_stream__set_is_buffered(stream, is_buffered_handler_apr); + stream->file = file; if (! disown) svn_stream_set_close(stream, close_handler_apr); @@ -920,6 +923,12 @@ svn_stream_from_aprfile2(apr_file_t *file, return stream; } +apr_file_t * +svn_stream__aprfile(svn_stream_t *stream) +{ + return stream->file; +} + /* Compressed stream support */ Index: subversion/svnlook/svnlook.c =================================================================== --- subversion/svnlook/svnlook.c (revision 1495378) +++ subversion/svnlook/svnlook.c (working copy) @@ -57,6 +57,7 @@ #include "private/svn_diff_private.h" #include "private/svn_cmdline_private.h" #include "private/svn_fspath.h" +#include "private/svn_io_private.h" #include "svn_private_config.h" @@ -957,6 +958,7 @@ print_diff_tree(svn_stream_t *out_stream, int exitcode; const char *orig_label; const char *new_label; + const char *diff_cmd; diff_cmd_argv = NULL; diff_cmd_argc = c->diff_options->nelts; @@ -983,10 +985,11 @@ print_diff_tree(svn_stream_t *out_stream, SVN_ERR(generate_label(&new_label, root, path, pool)); /* We deal in streams, but svn_io_run_diff2() deals in file - handles, unfortunately, so we need to make these temporary + handles, so we need to make these temporary files, and then copy the contents to our stream. */ - SVN_ERR(svn_io_open_unique_file3(&outfile, &outfilename, NULL, - svn_io_file_del_on_pool_cleanup, pool, pool)); + SVN_ERR(svn_io__file_for_command_stream(&outfile, &outfilename, + &diff_cmd, out_stream, + c->diff_cmd, pool, pool)); SVN_ERR(svn_io_open_unique_file3(&errfile, &errfilename, NULL, svn_io_file_del_on_pool_cleanup, pool, pool)); @@ -996,18 +999,22 @@ print_diff_tree(svn_stream_t *out_stream, orig_label, new_label, orig_path, new_path, &exitcode, outfile, errfile, - c->diff_cmd, pool)); + diff_cmd, pool)); - SVN_ERR(svn_io_file_close(outfile, pool)); + if (outfilename) + SVN_ERR(svn_io_file_close(outfile, pool)); SVN_ERR(svn_io_file_close(errfile, pool)); /* Now, open and copy our files to our output streams. */ + if (outfilename) + { + SVN_ERR(svn_stream_open_readonly(&stream, outfilename, + pool, pool)); + SVN_ERR(svn_stream_copy3(stream, + svn_stream_disown(out_stream, pool), + NULL, NULL, pool)); + } SVN_ERR(svn_stream_for_stderr(&err_stream, pool)); - SVN_ERR(svn_stream_open_readonly(&stream, outfilename, - pool, pool)); - SVN_ERR(svn_stream_copy3(stream, - svn_stream_disown(out_stream, pool), - NULL, NULL, pool)); SVN_ERR(svn_stream_open_readonly(&stream, errfilename, pool, pool)); SVN_ERR(svn_stream_copy3(stream, -- Philip Martin | Subversion Committer WANdisco | Non-Stop Data www.wandisco.com
