Daniel,
Since these are comments you your patch, would you like to discuss
them and propose a follow-up patch?
-Hyrum
On Aug 23, 2009, at 7:54 PM, Greg Stein wrote:
> On Thu, Aug 20, 2009 at 16:44, Hyrum K.
> Wright<hyrum_at_hyrumwright.org> wrote:
>> ...
>> +++ trunk/subversion/libsvn_client/patch.c Thu Aug 20 13:44:45
>> 2009 (r38893)
>> ...
>> @@ -205,7 +206,11 @@ merge_file_changed(svn_wc_adm_access_t *
>> const svn_wc_entry_t *entry;
>> svn_node_kind_t kind;
>>
>> - SVN_ERR(svn_wc_entry(&entry, mine, adm_access, FALSE, subpool));
>> + SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
>> + mine_abspath, svn_node_unknown,
>> + FALSE, FALSE,
>> + subpool, subpool));
>
> This is called merge_file_changed(), so you know the node is a file.
> You should pass svn_node_file here.
>
>> ...
>> @@ -369,7 +377,13 @@ merge_file_added(svn_wc_adm_access_t *ad
>> case svn_node_none:
>> {
>> const svn_wc_entry_t *entry;
>> - SVN_ERR(svn_wc_entry(&entry, mine, adm_access, FALSE,
>> subpool));
>> + svn_error_t *err;
>> +
>> + SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx-
>> >wc_ctx,
>> + mine_abspath, svn_node_none,
>> + FALSE, FALSE,
>> + subpool, subpool));
>
> Now this isn't actually correct. The KIND parameter is to describe the
> *entry* ... not what you found on disk. Thus, this should be
> svn_node_file.
>
> This similar problem exists with other calls to maybe_get_entry() in
> this patch.
>
> Essentially, what will happen is that you say "the entry is a
> directory [cuz that is what I saw on disk]", but the entry is actually
> a *file* ... when you tell svn_wc__get_entry() that it is a directory,
> but it finds an entry which is a file, then it returns UNEXPECTED_KIND
> (naturally). Then svn_wc__get_entry_versioned() will chew that up and
> set *entry to NULL and return an error. svn_wc__maybe_get_entry() will
> then eat *that* error and return a NULL. Net result? A versioned file,
> which is obstructed by a directory, will appear as if it was not
> versioned at all. Problem!
>
>> ...
>
> Cheers,
> -g
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2386544
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2386805
Received on 2009-08-24 19:57:32 CEST