> -----Original Message-----
> From: dglasser@gmail.com [mailto:dglasser@gmail.com] On
> Behalf Of David Glasser
> Sent: Monday, December 03, 2007 8:11 PM
> To: dev@subversion.tigris.org; pburba@tigris.org
> Cc: svn@subversion.tigris.org
> Subject: Re: svn commit: r28213 - in trunk/subversion:
> libsvn_client libsvn_wc tests/cmdline
>
> On Dec 3, 2007 12:24 PM, <pburba@tigris.org> wrote:
> > Author: pburba
> > Date: Mon Dec 3 12:24:18 2007
> > New Revision: 28213
> >
> > Log:
> > Finish issue #3029 by getting strict on what values ps
> svn:mergeinfo permits.
> >
>
>
> >
> > Modified: trunk/subversion/libsvn_wc/props.c
> > URL:
> >
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/props.c?pa
> > threv=28213&r1=28212&r2=28213
> >
> ======================================================================
> > ========
> > --- trunk/subversion/libsvn_wc/props.c (original)
> > +++ trunk/subversion/libsvn_wc/props.c Mon Dec 3 12:24:18 2007
> > @@ -2645,6 +2645,46 @@
> > {
> > new_value =
> svn_stringbuf_create_from_string(&boolean_value, pool);
> > }
> > + else if (strcmp(propname, SVN_PROP_MERGEINFO) == 0)
> > + {
> > + apr_hash_t *mergeinfo;
> > + SVN_ERR(svn_mergeinfo_parse(&mergeinfo,
> propval->data, pool));
> > + /* ### TODO: svn_mergeinfo_parse() conveniently
> performs almost all
> > + the checks we require for valid
> svn:mergeinfo...except one, it
> > + allows path's mapped to empty revision ranges.
> In addressing this
> > + issue,
> http://subversion.tigris.org/issues/show_bug.cgi?id=3029, we
> > + want to disallow this practice. But if we move
> enforcement of this
> > + to svn_mergeinfo_parse() then anyone who has has
> mergeinfo created
> > + by a pre-release version of 1.5 which has paths
> mapped to empty
> > + ranges is in for a world of hurt. Why? Can't
> they simply tweak
> > + their mergeinfo to follow the newly enforced
> rules? Sure, they
> > + could, *if* they could see their
> mergeinfo...which they won't be
> > + able to, since svn pl/pg will call
> svn_mergeinfo_parse() resulting
> > + in an error.
>
> Reeeeally? I could have sworn that
> svn_wc_canonicalize_svn_prop is called only for propsets, on
> the new value only.
Dave,
Let me rephrase...
svn_wc_canonicalize_svn_prop() now calls svn_mergeinfo_parse(), not
because it needs to parse mergeinfo per se, but because
svn_mergeinfo_parse() will QC the prop val and return a helpful error if
it violates mergeinfo grammar or a few other select rules (overlapping
ranges, unordered ranges, and backwards ranges).
The one thing svn_mergeinfo_parse() does *not* check is if paths map to
empty rev ranges (it considers these valid). This check is done
separately in svn_wc_canonicalize_svn_prop().
My comment is essentially asking, "Ok, then, why not put the check for
empty ranges in svn_mergeinfo_parse()?" Answer: Because I didn't want
*svn_mergeinfo_parse()* to start throwing errors when parsing previously
valid mergeinfo. Problem is, my main concern, that
svn_mergeinfo_parse() is called when doing a proplist is, uh, how to put
this, not true. Why, why, why did I think that!?!
Anyway, my comment had very little to do with
svn_wc_canonicalize_svn_prop(). And FWIW, after reading Dan's thoughts
here, http://svn.haxx.se/dev/archive-2007-12/0100.shtml, I was planning
to move the empty range check into svn_mergeinfo_parse() and remove this
comment.
Geez, I hope this all makes some sense(?)
Paul
> (Note that it's not actually called at
> commit time unless it's a propedit-on-URL, so it is possible
> by working around the WC layer to commit invalid props
> anyway...)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 4 03:31:59 2007