On 28.05.2015 13:07, Julian Foad wrote:
> Philip Martin wrote:
>> I still prefer the stream patch I posted earlier, and it can be extended
>> to support compressed streams. Something like:
>>
>> svn_error_t *
>> svn_stream_flush_to_disk_on_close(svn_stream_t *stream)
>> {
>> if (stream->close_fn == close_handler_apr)
>> {
>> svn_stream_set_close(stream, close_handler_flush);
>> }
>> else if (stream->close_fn == close_handler_gz)
>> {
>> struct zbaton *zbaton = stream->baton;
>> SVN_ERR(svn_stream_flush_to_disk_on_close(zbaton->substream));
>> }
>> else [...]
>> }
>>
>> That only allows flushing the stream on close but I do not see any need
>> at present to support flushing at arbitrary positions.
> The point about the generic stream API is you should always be able to
> define a new stream class that wraps a stream (examples: a 'tee'
> stream wraps one stream while copying to another; a checksumming
> stream, etc.).
>
> And you should always be able to use the wrapping stream in place of
> the original stream.
>
> The 'svn_stream_flush_to_disk_on_close()' that you suggest breaks that.
>
> The implementation you suggest in your email an hour ago needs direct
> access to the implementation methods of all the stream classes that it
> may possibly encounter (close_handler_gz, for example).
>
> And functionality supported by streams should be provided as a virtual
> method, overridden in each stream class.
I still think that we should do this on the level of stream
implementations, not on the level of the abstract stream API.
In the case we're talking about here, that would mean revising the
svn_stream_from_aprfile function and adding a flush-on-close flag, or
adding a similar function that has the flush-on-close semantics. That
would leave the generic stream API completely unchanged and only affect
the properties of the stream itself, at the point where it's created.
Nicely encapsulated, and far better than replacing existing stream-based
code with messy file-based code.
Whether or not we need a generic svn_stream_flush method is a separate
question on a completely different level, and quite irrelevant to the
problem of fixing this flushing issue for 1.9.x.
-- Brane
