On Mon, 2010-08-09, Stefan Küng 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.
Yes, that's what I meant. By saying "a shared copy" I just meant that the caller must not modify or delete (or get an exclusive read lock, etc.) the file, because it *may* be shared. > Maybe with a flag to keep/remove the file on pool cleanup. My point is that this "give me a reference" API *must* specify the lifetime of the reference, otherwise the library implementation doesn't know when it can delete the temp file. The lifetime can be until pool clean-up, or until something else, but it must be specified and it must be finite. Otherwise the temp dir will just grow and grow. > >> - 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. Yes, exactly. > 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. No, svn_wc_translated_file2() already creates a *file* and returns the path to that file. Maybe you're thinking of svn_wc_translated_stream() which also exists and was also deprecated recently. > * 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 More precisely: "if SVN_WC_TRANSLATE_FORCE_COPY is not specified and no translation is required ...". > 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. More precisely: "... ignored unless it creates a temp copy." > 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. Then it's the caller's responsibility to own and delete the file if the output flag says so. That implies that every call must generate a new, independent copy of the file. That is not optimal, but is a possible solution. - Julian
