On Mon, Aug 9, 2010 at 11:51 AM, Stefan Küng <tortoise...@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.f...@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