On Mon, Aug 9, 2010 at 11:51 AM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
> On 09.08.2010 16:44, Hyrum K. Wright wrote:
>>
>> On Mon, Aug 9, 2010 at 7:46 AM, Julian Foad<julian.foad_at_wandisco.com>
>> wrote:
>>>
>>> On Mon, 2010-08-09, Julian Foad wrote:
>>>>
>>>> On Mon, 2010-08-09, Bert Huijben wrote:
>>>>>
>>>>> Stefan Küng wrote:
>>>>>>
>>>>>> This leads me to another request: would it be possible to
>>>>>> 'un-deprecate'
>>>>>> the svn_wc_get_pristine_copy_path() API?
>>>>
>>>>> One of the ideas of WC-NG (which will most likely not be part of 1.7,
>>>>> but
>>>>> can be added later without really updating our current pristine api)
>>>>> was to
>>>>> allow compressed and/or (partially) absent pristine storage.
>>>>>
>>>>> This specific api function requires that the file is available in the
>>>>> pristine store as uncompressed file or that we store a copy of the file
>>>>> in a
>>>>> tempfolder (see the documentation update from Julian in r983593).
>>>>
>>>> Ah, yes - I'd forgotton that. When the pristine file already exists in
>>>> plain text, it's not a big problem to return its path even though we
>>>> can't promise how long it will live. But if pristine texts are stored
>>>> compressed and so we have to create a temp file in order to support that
>>>> API, when would we delete the temp file?
>>>
>>> A good answer could be: let the caller of
>>> svn_wc_get_pristine_copy_path() specify one of:
>>>
>>> - Give me a new copy of the file that I can keep indefinitely and
>>> delete when I wish.
>>>
>>> - Give me a reference to a shared copy of the file; keep it available
>>> until pool clean-up.
>
> Or just 'give me a reference to a file': if it's already available, just
> return the path to that file. If it's not available yet, get a copy of that
> file (uncompressed or fetched from a repository if it's not available
> locally at all) and return the path to that temp file.
> Maybe with a flag to keep/remove the file on pool cleanup.
>
>>>
>>> - Give me a reference to a shared copy of the file; keep it available
>>> until I call ... some new "unreference it" API?
>>>
>>> Would the second or third of those options work well, Stefan?
>>>
>>> We'll need to consider this within Subversion as well. If we start
>>> supporting compressed bases, we might want to cache non-compressed
>>> copies of some of them for faster access.
>>>
>>> A caller of svn_wc_translated_file2() has some control over its output
>>> file lifetime with a default behaviour of delete-on-pool-cleanup and a
>>> SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP flag to avoid that. However that
>>> only applies when it's making a new copy of the file. The behaviour
>>> when it returns a reference to the shared file is undefined. I've
>>> updated the doc string in r983593 to say so. Thus this function also
>>> needs attention.
>>
>> The whole point of the pristine store is that the location and
>> encoding of pristine contents should be transparent to anybody above
>> libsvn_wc. While making libsvn_wc much easier to maintain and extend,
>> going that route apparently decreases performance for clients such as
>> TortoiseSVN by taking away some of the shortcuts they were exploiting.
>> We need to decide if exposing these shortcuts (and adding the
>> associated code complexity) is worth it.
>>
>> In the potential scenario where a user has opted for no pristines or
>> compressed pristines, how would TortoiseSVN maintain its current
>> functionality without waiting on decompression or network roundtrips?
>
> I think users will understand if they decide to have their BASE compressed
> or not stored locally, that such diffs will take longer.
>
> If I may suggest how the API(s) should look and work:
>
> * rename svn_wc_translated_file2() to svn_wc_translated_filestream() since
> it doesn't return a file but a file stream. File streams can only be used by
> svn clients directly, they can't be passed on to external applications so
> their use is limited.
> * new API svn_wc_translated_file3():
> svn_client_translated_file(const char ** resultfile,
> svn_boolean_t * copyCreated,
> const char * wcfile,
> svn_boolean_t deleteOnPoolCleanup,
> apr_uint32_t flags,
> apr_pool_t* pool)
>
> the flags would be the same as now: SVN_WC_TRANSLATE_FORCE_EOL_REPAIR and
> SVN_WC_TRANSLATE_FORCE_COPY.
> The API would work like this:
> if SVN_WC_TRANSLATE_FORCE_COPY and SVN_WC_TRANSLATE_FORCE_EOL_REPAIR is not
> specified and the BASE file is available locally and uncompressed, just
> return the path to that file.
> Else create a temp copy of the BASE file and return that path.
> deleteOnPoolCleanup of course would get ignored if the file is available
> locally uncompressed and both SVN_WC_TRANSLATE_FORCE_COPY and
> SVN_WC_TRANSLATE_FORCE_EOL_REPAIR are not specified.
>
> The bool copyCreated would get set to true by the API if it created a copy
> and set to false if it just returned the path to the existing BASE file.
>
> That way, I could get the file very fast if it's possible, but still would
> get the file in a slow way if it's not possible.
Whatever we do, I'm -1 on any solution that uses bitflags.
-Hyrum
Received on 2010-08-09 19:12:26 CEST