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

RE: [RFC] QA of manually set svn:mergeinfo

From: Paul Burba <pburba_at_collab.net>
Date: 2007-11-07 17:31:22 CET

> -----Original Message-----
> From: Mark Phippard [mailto:markphip@gmail.com]
> Sent: Wednesday, November 07, 2007 11:25 AM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: [RFC] QA of manually set svn:mergeinfo
>
> On 11/7/07, Paul Burba <pburba@collab.net> wrote:
> > Users can manually set the svn:mergeinfo property with svn propset.
> >
> > How far should we go to save them from themselves?
> >
> > 1) Today if the mergeinfo isn't validly formatted we let the ps
> > succeed, but attempts to commit it (or merge into it) fail:
> >
> > >svn ps svn:mergeinfo "jrandom" merge_tests-63\A_COPY\D\G
> > property 'svn:mergeinfo' set on 'merge_tests-63\A_COPY\D\G'
> >
> > >svn merge %URL63%/A/D merge_tests-63\A_COPY\D -r3:8
> > ..\..\..\subversion\libsvn_subr\mergeinfo.c:418: (apr_err=200020)
> > svn: Pathname not terminated by ':'
> >
> > >svn ci -m "" merge_tests-63
> > Sending merge_tests-63\A_COPY\D\G
> > ..\..\..\subversion\libsvn_client\commit.c:916: (apr_err=200020)
> > svn: Commit failed (details follow):
> > ..\..\..\subversion\libsvn_subr\mergeinfo.c:418: (apr_err=200020)
> > svn: Pathname not terminated by ':'
> >
> > We do allow the propsets however:
> >
> > 2) Overlapping ranges with the same inheritability
> >
> > >svn ps svn:mergeinfo "/A/D/G:1,5-7,7-9" merge_tests-63\A_COPY\D\G
> > property 'svn:mergeinfo' set on 'merge_tests-63\A_COPY\D\G'
> >
> > 3) Overlapping ranges with differing inhertiability
> >
> > >svn ps svn:mergeinfo "/A/D/G:1,5-7,7-9*" merge_tests-63\A_COPY\D\G
> > property 'svn:mergeinfo' set on 'merge_tests-63\A_COPY\D\G'
> >
> > 4) Unordered ranges
> >
> > >svn ps svn:mergeinfo "/A/D/G:1,7-9,5" merge_tests-63\A_COPY\D\G
> > property 'svn:mergeinfo' set on 'merge_tests-63\A_COPY\D\G'
> >
> > These last three can be merged into (with unpredictable results in
> > some
> > cases) or committed.
> >
> > In the second and fourth cases we could correct the mergeinfo to
> > "/A/D/G:1,5-9" and "/A/D/G:1,5,7-9" respectively, but in the second
> > case any assumption is going to be wrong for somebody ("did
> the user
> > mean r7 to apply only to 'merge_tests-63\A_COPY\D\G' or to all its
> > children as well?").
> >
> > A few options...
> >
> > A) Consider cases 2 - 4 as invalid svn:mergeinfo and fail during a
> > commit or merge the same way as case 1.
> >
> > B) Auto-correct the mergeinfo in cast 2 and 4 immediately following
> > the propset. Handle case 3 the same as case 1 today (no
> error until
> > merge or commit).
> >
> > C) Auto-correct the mergeinfo in cast 2 and 4 immediately following
> > the propset. Fail case 1 and 3 on the propset attempt.
> >
> > D) Fail all four cases on the propset attempt.
> >
> > I really prefer D! In cases 1 and 3 it lets the user know
> as soon as
> > they try an completely bogus propset rather than waiting until a
> > commit or merge. In cases 2 and 4 it prevents a silent
> change to the
> > property they just explicitly set.
> >
> > Thoughts, objections to D?
>
> Can D be done?

Mark,

Yes, much of the code already exists (as the failure during a commit in
case 1 and 3 demonstrate).

> I would be in favor of D over the other
> options. I would not be against auto-correcting in cases
> where we are just helping the user and not potentially
> auto-correcting a typo or mistake. That is why I generally
> favor D.

> If the data they enter is not perfect it is kind of
> impossible to know if we are helping or hurting them by
> "fixing" the problem.

In the case of improperly ordered ranges I think we can safely say there
is only one correct interpretation ("/trunk:6,3" == "/trunk:3,6" no
matter how much you look at it :-).

With overlapping changes with the same inheritability there is only one
correct meaning as well ("/trunk:5-6,6-7" == "/trunk:5-7").

*But* in both cases the fact that a user specified something odd raises
a red flag for me that maybe they meant something else entirely or
simply don't understand what they are doing. I'm quite comfortable
being ultra-strict when someone is using propset to set svn:mergeinfo.

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 7 17:36:28 2007

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