subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <...@daniel.shahaf.name>
Subject Re: svn commit: r1144316 - in /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c
Date Fri, 08 Jul 2011 19:32:10 GMT
My point wasn't the validation of svn:mergeinfo revprops, it was whether
the validation of svn:mergeinfo nodeprops should happen in
svn_repos__validate_prop().

I'm not against telling that function whether it's validating a nodeprop
or a revprop, and applying different logic in either case.

Philip Martin wrote on Fri, Jul 08, 2011 at 20:03:48 +0100:
> Daniel Shahaf <d.s@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

Mime
View raw message