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

Re: svn commit: r1144316 - in /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Fri, 08 Jul 2011 20:03:48 +0100

Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:

>> Perhaps. svn_repos__validate_prop is also used by
>> svn_repos_fs_change_txn_props, is it appropriate to validate mergeinfo
>> there?
>>
>
> Why not? No one should be setting svn:mergeinfo as a txnprop (or
> revprop). (and if they do, they shouldn't use the svn:* namespace)
>
>> We should probably split the validate function into two, one for node
>> props and one for revprops.
>>
>
> Given that we don't have any SVN_PROP_* whose semantics differ when it's
> set as a revprop v. when it's set as a nodeprop, I'm not sure what this
> would gain; I'm ±0.

Huh? They differ totally. It makes no sense to check svn:mergeinfo
syntax valid set as a revprop, we should either allow all values or
none.

> Unless, perhaps, you want to add verification that svn:mergeinfo isn't
> set as a revprop and svn:log isn't set as a nodeprop? Feel free to do
> so, but then I suggest you'll also teach svnsync et al to strip those
> (ill-set) properties to avoid breaking any repositories out there that
> do have svn:mergeinfo revprops and svn:log nodeprops.

Old repositories are already problem. The existing svn:mergeinfo
validation is going to prevent people dumping/loading repositories with
invalid svn:mergeinfo on nodes. We don't want to add to the problem by
adding syntax checking where we don't need it unless we *also* add the
stripping code.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Received on 2011-07-08 21:04:30 CEST

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