On 12/12/05, Greg Hudson <ghudson@mit.edu> wrote:
> On Mon, 2005-12-12 at 23:03 +0100, Erik Huelsmann wrote:
> > Well, I could create an svn_stream_from_aprfile2 which conforms to the
> > above model. Whether svn_stream_from_aprfile should be immediately
> > deprecated is not clear to me, but we might as well.
On my quest through our sources to replace svn_stream_from_aprfile
with the ...2() version, I came across svn_client_cat2(). I decided to
replace the current implementation which uses temp files with one
using the new translating stream. It's a nice test case for th
desired ownership model.
The patch below works great, but there's a big problem with the ownership model:
it creates a stream to be stacked upon the 'out' stream (a parameter
passed iinto the routine). Clearly, we're given a stream which we
shouldn't close because the caller owns it. The stacked stream -
according to the model - will own the 'out' stream, but this conflicts
with the fact that it's actually a borrowed resource....
Apart from not changing the implementation, I hit a wall pretty fast.
Any ideas about solving this one? I thought of adding an explicitly
ownership-detaching stream which just forwards reads and writes to a
stream which it *doesn't* close upon being closed....
bye,
Erik.
Index: subversion/libsvn_client/cat.c
===================================================================
--- subversion/libsvn_client/cat.c (revision 17766)
+++ subversion/libsvn_client/cat.c (working copy)
@@ -144,7 +144,7 @@
SVN_ERR (svn_io_file_open (&input_file, base,
APR_READ, APR_OS_DEFAULT, pool));
- input = svn_stream_from_aprfile (input_file, pool);
+ input = svn_stream_from_aprfile2 (input_file, pool);
if ( eol || kw )
SVN_ERR (svn_subst_translate_stream3 (input, output, eol, FALSE, kw,
@@ -153,7 +153,6 @@
SVN_ERR (svn_stream_copy (input, output, pool));
SVN_ERR (svn_stream_close (input));
- SVN_ERR (svn_io_file_close (input_file, pool));
return SVN_NO_ERROR;
}
@@ -217,41 +216,19 @@
if (! eol_style && ! keywords)
{
/* It's a file with no special eol style or keywords. */
- SVN_ERR (svn_ra_get_file (ra_session, "", rev, out, NULL, NULL, pool));
- }
- else
- {
- apr_hash_t *kw = NULL;
- svn_subst_eol_style_t style;
- const char *temp_dir;
- const char *tmp_filename;
- svn_stream_t *tmp_stream;
- apr_file_t *tmp_file;
- apr_status_t apr_err;
- apr_off_t off = 0;
- const char *eol = NULL;
+ svn_subst_eol_style_t eol;
+ const char *eol_str;
+ apr_hash_t *kw;
- /* grab a temporary file to write the target to. */
- SVN_ERR (svn_io_temp_dir (&temp_dir, pool));
- SVN_ERR (svn_io_open_unique_file2
- (&tmp_file, &tmp_filename,
- svn_path_join (temp_dir, "tmp", pool), ".tmp",
- svn_io_file_del_on_close, pool));
-
- tmp_stream = svn_stream_from_aprfile (tmp_file, pool);
-
- SVN_ERR (svn_ra_get_file (ra_session, "", rev, tmp_stream,
- NULL, NULL, pool));
-
- /* rewind our stream. */
- apr_err = apr_file_seek (tmp_file, APR_SET, &off);
- if (apr_err)
- return svn_error_wrap_apr (apr_err, _("Can't seek in '%s'"),
- svn_path_local_style (tmp_filename, pool));
-
if (eol_style)
- svn_subst_eol_style_from_value (&style, &eol, eol_style->data);
+ svn_subst_eol_style_from_value (&eol, &eol_str, eol_style->data);
+ else
+ {
+ eol = svn_subst_eol_style_none;
+ eol_str = NULL;
+ }
+
if (keywords)
{
svn_string_t *cmt_rev, *cmt_date, *cmt_author;
@@ -267,21 +244,22 @@
SVN_ERR (svn_time_from_cstring (&when, cmt_date->data, pool));
SVN_ERR (svn_subst_build_keywords2
- (&kw, keywords->data,
+ (&kw, keywords->data,
cmt_rev->data,
url,
when,
cmt_author ? cmt_author->data : NULL,
pool));
}
+ else
+ kw = NULL;
- SVN_ERR (svn_subst_translate_stream3 (tmp_stream, out, eol, FALSE, kw,
- TRUE, pool));
-
- SVN_ERR (svn_stream_close (tmp_stream));
- SVN_ERR (svn_io_file_close (tmp_file, pool));
+ /* Interject a translating stream */
+ out = svn_subst_stream_translated (out, eol_str, FALSE, kw, TRUE, pool);
}
+ SVN_ERR (svn_ra_get_file (ra_session, "", rev, out, NULL, NULL, pool));
+
return SVN_NO_ERROR;
}
Received on Tue Dec 13 23:32:40 2005