On Mon, May 03, 2010 at 03:07:49PM +0200, Neels J Hofmeyr wrote:
>
> dannas_at_apache.org wrote:
> > Author: dannas
> > Date: Sat May 1 21:06:36 2010
> > New Revision: 940111
> >
> [...]
>
> > (svn_cl__print_status_xml): Add ctx parameter.
> >
> > * subversion/svn/status.c
> > (print_status): Add ctx parameter. Check status->conflicted and use wc
> > funcs for fetching further information.
> > svn_cl__print_status_xml,
>
> log message error ^
Fixed.
[...]
> > ==============================================================================
> > --- subversion/trunk/subversion/include/svn_wc.h (original)
> > +++ subversion/trunk/subversion/include/svn_wc.h Sat May 1 21:06:36 2010
> > @@ -3645,6 +3645,8 @@ typedef struct svn_wc_status3_t
> > /** The locally present lock creation date.
> > */
> > apr_time_t lock_creation_date;
> > +
> > + svn_boolean_t conflicted;
>
> How about a short comment? ^
> something like "TRUE if tree-, prop- and/or text-conflicted"
Done in a follow-up. Redone in yet another follow-up after Bert pointed
out that we will have obstructed and patch conflicts in the future.
> >
> > /* NOTE! Please update svn_wc_dup_status3() when adding new fields here. */
>
> I think you did not heed this note. ^
I just pasted the line from svn_wc_status2_t and changed the number 2 to
3. Converting between status2_t and status_t was done with a cast and
that worked straight off, except for the ones that needed to be
reallocated with dup_statusX(). I don't know, maybe it's easy to forget
that? I thought that it made no harm and that I had seen similar notes
about updating a dup() func in other structs (though I can't find any
such usages when I looked today).
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_wc/status.c (original)
> > +++ subversion/trunk/subversion/libsvn_wc/status.c Sat May 1 21:06:36 2010
> > @@ -302,6 +302,7 @@ assemble_status(svn_wc_status3_t **statu
> > svn_revnum_t changed_rev;
> > const char *changed_author;
> > apr_time_t changed_date;
> > + svn_boolean_t conflicted;
> > #ifdef HAVE_SYMLINK
> > svn_boolean_t wc_special;
> > #endif /* HAVE_SYMLINK */
> > @@ -393,6 +394,7 @@ assemble_status(svn_wc_status3_t **statu
> > stat->lock_owner = NULL;
> > stat->lock_comment = NULL;
> > stat->lock_creation_date = 0;
> > + stat->conflicted = (tree_conflict != NULL);
>
> I don't understand -- what about prop and text conflicts? Below,
> ->conflicted stand for all three of them.
> (If this is correct, I guess it needs a comment)
Added a comment in r940600. AFAIK, the only scenario where we can get a
conflict on an unversioned node is for an incoming delete to a locally
deleted path. (It will be troublesome to fit that case into the idea of
storing tree conflict info on the node instead of its parent).
Thanks for your review,
Daniel
Received on 2010-05-03 22:27:05 CEST