On Tue, Aug 25, 2009 at 12:04:12PM +0100, Stefan Sperling wrote:
> > [[[
> > Follow up to r38893. Changed svn_node_kind_t to use the kind of the
> > entry rather than the kind of the node on disk.
>
> Drive-by remark about the log message.
>
> "Changed svn_node_kind_t" could probably be worded better.
> When read in isolation it sounds like you changed the data type
> itself. But rather, you've changed the meaning of a parameter
> of a function. So let's say so:
>
> Follow up to r38893. Change the KIND parameter of svn_wc__maybe_get_entry()
> to mean the kind of the entry rather than the kind of the node on disk.
Point taken. I shamelessly steal your wordings.
[[[
Follow up to r38893. Change the KIND parameter of svn_wc__maybe_get_entry()
to mean the kind of the entry rather than the kind of the node on disk.
* subversion/libsvn_client/patch.c
(merge_file_changed, merge_file_added): Set kind to svn_node_file.
(merge_dir_added): Set kind to svn_node_dir.
(init_patch_target): Set kind to svn_node_file.
]]]
Another question:
> Why does this patch not change svn_wc__maybe_get_entry() itself?
> Does its behaviour not change? (Sorry if I am being stupid asking this,
> I haven't read the entire discussion).
The thing is that svn_wc__maybe_get_entry() wraps
svn_wc__get_entry_versioned(). And that function catches an
SVN_ERR_NODE_UNEXPECTED_KIND. In the end that error is cleared and a
null entry is returned. So if the expected kind is set to the wrong sort
we may end up with a versioned node appearing as not versioned at all.
I guess that returning the error would make it easier to catch this sort
of misbehavior on the callers behalf but since this is a temporary API
perhaps it's better to just leave it as is?
Mvh
Daniel
>
> > Index: subversion/libsvn_client/patch.c
> > ===================================================================
> > --- subversion/libsvn_client/patch.c (revision 38923)
> > +++ subversion/libsvn_client/patch.c (arbetskopia)
> > @@ -207,7 +207,7 @@
> > svn_node_kind_t kind;
> >
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
> > - mine_abspath, svn_node_unknown,
> > + mine_abspath, svn_node_file,
> > FALSE, FALSE,
> > subpool, subpool));
> >
> > @@ -380,7 +380,7 @@
> > svn_error_t *err;
> >
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
> > - mine_abspath, svn_node_none,
> > + mine_abspath, svn_node_file,
> > FALSE, FALSE,
> > subpool, subpool));
> >
> > @@ -469,7 +469,7 @@
> > const svn_wc_entry_t *entry;
> >
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
> > - mine_abspath, svn_node_dir,
> > + mine_abspath, svn_node_file,
> > FALSE, FALSE,
> > subpool, subpool));
> >
> > @@ -659,7 +659,7 @@
> > case svn_node_none:
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
> > local_abspath,
> > - svn_node_none,
> > + svn_node_dir,
> > FALSE, FALSE,
> > subpool, subpool));
> >
> > @@ -726,7 +726,7 @@
> > {
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, patch_b->ctx->wc_ctx,
> > local_abspath,
> > - svn_node_file,
> > + svn_node_dir,
> > FALSE, FALSE,
> > subpool, subpool));
> >
> > @@ -2060,7 +2060,7 @@
> > /* If the target is versioned, we should be able to get an entry. */
> > SVN_ERR(svn_wc__maybe_get_entry(&entry, ctx->wc_ctx,
> > new_target->abs_path,
> > - svn_node_unknown,
> > + svn_node_file,
> > TRUE, FALSE,
> > result_pool,
> > scratch_pool));
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2387013
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2387028
Received on 2009-08-26 23:08:09 CEST