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

Re: r32158: svn_repos_mergeinfo_changed() - meaning vs. representation

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 21 Jul 2008 18:14:11 +0100

On Mon, 2008-07-21 at 10:25 -0400, C. Michael Pilato wrote:
> Julian Foad wrote:
> > A question about mergeinfo representation versus mergeinfo canonical
> > meaning after taking account of inheritance etc. (Bear in mind that I'm
> > not well aquainted with this merging stuff.)
> >
> > The new function svn_repos_mergeinfo_changed() looks like a useful
> > helper for things that want to know in what way mergeinfo changed in a
> > particular revision.
> >
> >> /* Set @a *deleted_mergeinfo_catalog and @a *added_mergeinfo_catalog
> >> * to mergeinfo catalogs describing how mergeinfo values on paths
> >> * (which are the keys of those catalogs) were changed via the commit
> >> * of @a rev in @a repos. Allocate returned data from @a pool.
> >> *
> >> * Every path whose mergeinfo was created or modified in @a rev in
> >> * such a way that it differs from its value in @a rev - 1 is
> >> * represented in both returned catalog hashes, and only those paths
> >> * are represented therein.
> >> *
> >> * @since New in 1.6.
> >> */
> >> svn_error_t *
> >> svn_repos_mergeinfo_changed(svn_mergeinfo_catalog_t *deleted_mergeinfo_catalog,
> >> svn_mergeinfo_catalog_t *added_mergeinfo_catalog,
> >> svn_repos_t *repos,
> >> svn_revnum_t rev,
> >> apr_pool_t *pool);
> >
> >>From the fact that it reports its results in "svn_mergeinfo_catalog_t"
> > which has separate representations for "no mergeinfo" and "empty
> > mergeinfo", it looks like it reports changes to the mergeinfo
> > representation, so, for example, if somebody "cleans up" their mergeinfo
> > properties by performing some elision of mergeinfo that would be
> > inherited, or by removing inoperative revision ranges, that would be
> > reported as a change.
> >
> > Have I understood it correctly?
>
> That's correct. Just as svn_fs_get_mergeinfo()
>
> > There's nothing wrong with that as such, but I'm wondering:
> >
> > * Do we have the complementary support function(s) for determining
> > whether the canonical meaning of the mergeinfo has changed?
>
> No.
>
> > * Where are we drawing the architectural line between APIs that deal
> > with the representation and APIs that deal with the canonical meaning?
> >
> > * Do we need to make that distinction clearer in the doc strings,
> > perhaps using terminology like "mergeinfo properties" versus "canonical
> > mergeinfo"?
>
> Per this line of questioning, I'm wondering if you'd have been happier if
> I'd named the function svn_repos_fs_props_changed() and had it accept any
> property name and return a hash of path->propvals. :-)

Mmm... Separating the basic data-gathering algorithm from any
mergeinfo-specific processing is architecturally good. I hadn't thought
of that here but I've just been thinking the analogous thing about some
libsvn_wc APIs, so +1.

> > Also, does this new function need "authz_read" functionality like the
> > v1.5 function svn_repos_fs_get_mergeinfo() has, or need to document its
> > behaviour in this regard?
>
> Ah, yes, it probably does if it's going to be public.
>
> Many of these concerns appeared the minute I made the function public from
> its previous internal-helper state. The easiest fix for them all is to
> de-publicize it again and revisit when/if third-party software grows a
> tangible need for the functionality. Shall I go that route?

Seems like it might be the most effective thing to do here.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-21 19:09:55 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.