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

Re: [PATCH] support printing changed properties in svnlook

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 29 Dec 2010 05:20:21 +0200

[ The second hunk below has a discussion regarding the representation
in the API of 'svn ps k v iota; svn ci iota; svn cp iota iota2;
svn ps k2 v2 iota2; svn ci iota2', which I'd be happy to have further
input on. ]

Erik Johansson wrote on Wed, Dec 22, 2010 at 23:08:30 +0100:
> On Fri, Dec 10, 2010 at 15:08, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> > [ I'm somewhat confused about this issue. ]
> >
> > Erik Johansson wrote on Thu, Dec 09, 2010 at 21:41:38 +0100:
> >> On Wed, Dec 8, 2010 at 23:22, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> >> > Erik Johansson wrote on Wed, Dec 08, 2010 at 17:17:47 +0100:
> >> >> +  /** How this property entered the node tree: 'A'dd, 'D'elete, 'R'eplace */
> >> >> +  char action;
> >
> > I should have caught this earlier, but "replaced" doesn't make sense
> > for properties.  (A node can be replaced by another node at the same
> > path, but for a fixed node properties can be added/modified/removed but
> > not replaced.)
>
> I'll change it to 'A'dd, 'D'elete or 'U'pdate then?
>

'M'odify, not 'U'pdate, I think. At least that's what the UI level
does, but if there's precedent for 'U'pdated then follow it.

> >> >
> >> > This is copied from svn_repos_node_t->action.  There was recently
> >> > a question about that field:
> >> > http://mid.gmane.org/3ABD28AA-A2FC-4D7D-A502-479D37995DB9@orcaware.com
> >> >
> >> > So, that asks whether 'C'hanged is a valid answer to the question that
> >> > ->action is meant to answer.  I'll also ask how this interacts with node
> >> > changes: for example; if r5 replaces-with-history a node that has
> >> > 'fooprop' set with another node that also has 'fooprop' set, what would
> >> > the value of 'action' be?
> >>
> >> What about this:
> >> When a node is deleted all the properties it had are listed in
> >> mod_prop with action D.
> >>
> >> When a node is added-with-history all the properties the source had
> >> are listed in mod_prop with action A and a new flag copyfrom = TRUE.
> >>
> > There is already a copyfrom indication in svn_repos_node_t->copyfrom_rev,
> > I don't think we need another one.
>
> I was thinking that the copyfrom flag would be useful in the case
> where you copy a node with a propery and add another property before
> committing. Here you would have two property with action A and one
> would have copyfrom = TRUE.
>

In other words, you'd like to make a distinction between "This node was
added-with-history, and this property came along" and "<ditto>, but the
property is new on this copy target and didn't exist in the copy
source".

I see, and I agree it's a good distinction to make.

I'm not sure how/whether our other API's (the editor, possibly the
reporter, others?) represent this.

> >> A replace-with-history will result in two repos_nodes, each having a
> >> mod_prop list. If the same property exists in both it means it has
> >> been replaced.
> >>
> > Two nodes doesn't sound like a good idea; one node marked as 'replaced'
> > should suffice.
>
> In the current api a replaced node is represented by two nodes. Should
> we change this?

Doesn't that mean that API callers who want to distinguish a replace
from a remove or an add have to walk through all the child entries (for
a given dir) before they can process any adds or removals?

I'm thinking that one entry is better, because callers can split it if they
wish; that's easier than two entries and letting callers figure out for
themselves how to recombine them.

> Should svnlook stay back-wards compatible and print a
> replaced node twice, once as Deleted and once as Added?
>

I could argue both ways: it should be back-compatible with itself, but
it should also be compatible with the cmdline client where the latter
prints just one "R" notification.

> > In case of a replacement, do we want the API to provide the replaced
> > node's properties? (at least their names)
>

I suppose we can agree to answer this as "Yes", then? :)

> It would be pretty simple if there are two separate nodes for a
> replaced node. The deleted node will have all the properties it had
> marked as Deleted. The new node will have its properties marked as
> appropiate.
>

That's a straightforward representation, yes. I suppose we could find
a straightforward in the "single node" case, though --- eg by adding
fields only used in the case of 'R'eplacements.

> // Erik
>
> --
> Erik Johansson
> Home Page: http://ejohansson.se/
> PGP Key: http://ejohansson.se/erik.asc
Received on 2010-12-29 04:23:29 CET

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.