Hi!
On Sun, Aug 23, 2009 at 05:54:49PM +0000, 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.
This was bad but not a catastrophe. Just takes some more processing to
find out the right kind.
> >...
> > @@ -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!
This IS a catastrophe! I had totally missed that the svn_node_kind_t
type could both describe an entry and and what actually is on the disk.
The error returned from get_entry_versioned() is determined like this:
* subversion/libsvn_wc/entries.c (svn_wc__get_entry_versioned)
if (err && (err->apr_err == SVN_ERR_WC_MISSING
|| err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND
|| err->apr_err == SVN_ERR_NODE_UNEXPECTED_KIND))
{
svn_error_clear(err);
*entry = NULL;
}
....
if (! *entry)
return svn_error_createf(SVN_ERR_ENTRY_NOT_FOUND, NULL,
_("'%s' is not under version control"),
svn_dirent_local_style(local_abspath,
scratch_pool));
I created the svn_wc__maybe_get_entry() under the assumption that the
unexpected errors in the test suite when a NULL entry was expected
required a changed return semantics.
Can I make a new patch and just change the KIND parameter to a proper
value?
Mvh
Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2386822
Received on 2009-08-27 00:06:56 CEST