Re: [PATCH] Define behavior of svn_wc_get_pristine_content2()

[Neels J Hofmeyr <ne...@elego.de>]

Bert Huijben wrote:
> 
>> -----Original Message-----
>> From: Neels J Hofmeyr [mailto:neels@elego.de]
>> Sent: zondag 28 februari 2010 23:49
>> To: Subversion Development
>> Subject: [PATCH] Define behavior of svn_wc_get_pristine_content2()
>>
>> Hi,
>>
>> Attached patch rewrites the comment for
>> svn_wc_get_pristine_contents2(),
>> which didn't say much about *which* pristine this function returns.
>>
>> I think this new comment describes current behavior (or at least
>> currently
>> intended behavior). We need to properly define this for when we roll
>> out the
>> pristine store.
>>
>> Is anything here not making sense?
>>
>> (pasting the new comment here as well for easier reading:)
>>
>> [[[
>> /** Given a @a path to a wc file, return in @a contents a stream to the
>>  * contents of the pristine copy of the file, as relevant to the
>> currently
>>  * set history of the file. That means:
>>  *
>>  * With no uncommitted changes on the file, or only text/prop
>> modifications,
>>  * or when the file is only locally deleted (not replaced), return the
>> last
>>  * checked-out or updated-/switched-to content of the file.
>>  *
>>  * When the file has been locally copied-/moved-here, return the
>> content of
>>  * the copy/move source (even if the copy-/move-here replaces a locally
>>  * deleted file).
>>  *
>>  * If the file is simply added or replaced (no copy-/move-here
>> involved),
>>  * return @c NULL.
> 
> Set @a contents to @c NULL, or return no error?

The answer is both. Successfully return empty content, which this function
returns in form of a NULL stream_t pointer. (thanks)

> 
>>  *
>>  * If @local_abspath refers to an unversioned or non-existing path,
>> return
>>  * @c SVN_ERR_WC_PATH_NOT_FOUND.  Use @a wc_ctx to access the working
>> copy.
>>  * @a contents may not be @c NULL and must point at a writable
>> svn_stream_t*.
> 
> Writable?
> 
> And *contents = NULL on local add (see above)

yes, *contents = NULL, but contents != NULL :)
I meant to hint at the user having to provide an out parameter stream_t*.
gah, I'll just skip that sentence.

> 
>>  *
>>  * This function is needed so clients can do diffs.
>>  *
>>  * @since New in 1.7. */
>> svn_error_t *
>> svn_wc_get_pristine_contents2(svn_stream_t **contents,
>>                               svn_wc_context_t *wc_ctx,
>>                               const char *local_abspath,
>>                               apr_pool_t *result_pool,
>>                               apr_pool_t *scratch_pool);
>> ]]]
> 
> 	Bert

Did you also check for semantic errors?

~Neels