On 22.05.2015 10:50, Philip Martin wrote:
> Branko Čibej <brane_at_wandisco.com> writes:
>
>> FWIW, I think in this case revving the API without deprecating the old
>> one is justified. Another option would be to invent a different API name
>> for the flushing stream, e.g., svn_stream_from_aprfile_flushed or some
>> such. This way we'd also avoid the dilemma about disown by simply not
>> having that flag in the new function.
> Are there bits missing from r1680819? svn_fs_fs__pack_revprops_shard()
> still uses a stream to write the manifest file and it has not been
> modified to flush. write_non_packed_revprop() also uses a stream that
> is not flushed.
>
> We could create a "flushing stream" that gets chained:
>
> struct baton_flush_t {
> svn_stream_t *stream;
> apr_pool_t *pool;
> };
>
> static svn_error_t *
> close_handler_flush(void *baton)
> {
> struct baton_flush_t *b = baton;
>
> SVN_ERR(svn_io_file_flush_to_disk(svn_stream__aprfile(b->stream), b->pool));
> SVN_ERR(svn_stream_close(b->stream));
> return SVN_NO_ERROR;
> }
>
> svn_error_t *
> svn_stream_flush(svn_stream_t **flush,
> svn_stream_t *stream,
> apr_pool_t *pool)
> {
> struct baton_flush_t *baton;
>
> if (!svn_stream__aprfile(stream))
> return svn_error_create("No file to flush");
>
> baton = apr_palloc(...);
> *flush = svn_stream_create(baton, pool);
> svn_stream_set_close(*flush, close_handler_flush);
> svn_stream_set_read(...);
> ...
>
> return SVN_NO_ERROR;
> }
>
> That would allow the flushing of files we are not going to close which
> probably works but may not be something we want. If we want to avoid
> that then perhaps we use a more specialised svn_stream_flush() that only
> supports streams created by svn_stream_from_aprfile2() by detecting
> close_fn==close_hadler_apr.
>
> svn_error_t *
> svn_stream_flush(svn_stream_t **flush,
> svn_stream_t *stream,
> apr_pool_t *pool)
> {
> struct baton_flush_t *baton;
>
> if (stream->close_fn != close_handler_apr)
> return svn_error_create("No closing file to flush")
>
> ...
> }
>
>
> Another approach would be to have a function to enable flushing directly
> on the stream created by svn_stream_from_aprfile2() without creating a
> new stream. This is probably the smallest/simplest patch. After
> reverting r1680819:
[...]
Big +1! Let's do it this way. We can keep the 1.8.x backport unchanged, too.
-- Brane
Received on 2015-05-22 11:05:43 CEST