[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r38893 - in trunk/subversion: include/private libsvn_client libsvn_wc

From: Daniel Näslund <daniel_at_longitudo.com>
Date: Mon, 24 Aug 2009 20:58:19 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.