dlr and glasser have started thinking about ways that renames don't
play nice with 'merge --reintegrate'.
Yesterday, glasser and I had an IRC conversation about it. Below is
the transcript, followed by some thoughts. At the beginning of the
transcript, glasser sets up the scenario; you may wish to skim the
rest and go straight to the meat of this mail, which starts with the
string "Some thoughts:" (didn't take my imagination pill today, sorry).
<glasser> do some work on branch
<glasser> do some work on trunk
<glasser> merge trunk to branch, now mergeinfo on /branch is
"/trunk:10-20"
<glasser> now on the branch, we do a "svn mv foo bar" to rename foo to
bar (or just a cp, whatever0
<kfogel> ok
<glasser> what is the mergeinfo for /branch/bar?
<glasser> (audience participation!)
<kfogel> what happens when we cp something? does it get an auto
empty svn:mergeinfo property, or does it keep the prop value
(and/or inherit from parent)?
<kfogel> Right now I'm asking what _does_ happen.
<kfogel> After that, we'll get to what _should_ happen.
<glasser> Um. I think the answer is "it depends"
<glasser> Let's skip to "should"
* kfogel thinks
<kfogel> I mean, whatever had been merged into foo up till that point
*should* be listed as being merged into bar.
<kfogel> right?
<kfogel> We want that?
<glasser> I don't believe in desires, but that is what I believe the
current design does
<glasser> so that would be "/trunk/foo:10-20", on /branch/bar. right?
<kfogel> um, yes
<kfogel> I feel I am being led, or leading myself, down a path paved
with good intentions and leading straight to...
<kfogel> Well, anyway.
<kfogel> I believe that mergeinfo is correct, to the best of my
knowledge, your honor.
<glasser> Well then!
<glasser> I am --reintegrate trying to figure out if /branch is
uniform or not
<glasser> so I get information from the server about the paths with
mergeinfo
<glasser> they are:
<glasser> /branch, with "/trunk:10-20"
<glasser> /branch/bar, with "/trunk/foo:10-20"
<kfogel> yup
<kfogel> uh-huh
<kfogel> hrm
<glasser> now, if the mergeinfo on /branch/bar was "/trunk/bar:10-20",
then that would in fact be completely redundant with the
mergeinfo on /branch
<glasser> and I would happily say "everything is even"
<glasser> but! in fact! what this mergeinfo tells me is that, while
/branch has up to r20 from /trunk
<glasser> /branch/bar has *nothing* from the corresponding path in
/trunk (/trunk/bar)
<glasser> now certainly that's because bar is new in the branch
<glasser> but the current reintegrate code has no way of knowing that
<glasser> so! how do we allow this perfectly-reasonable branch to be
reintegrated?
<kfogel> however, we could deduce that a file is new in the branch,
by tracing its locations...
<glasser> current trunk code lets it be reintegrated (and the test
suite tests it), but unfortunately there's a bug, the fixing
of which breaks it
<glasser> yeah, that's mostly what I've come to
<glasser> so what do you propose exactly
<glasser> ?
<kfogel> exactly?
<kfogel> Mrmmmph.
<glasser> you look at *all* the location segments of /branch/bar,
seeing if *any* of them are /trunk/bar? if any of them are
/trunk? etc
<kfogel> to hand-wave a *bit* (b/c I don't know the proper interface
names)
<kfogel> you trace back 'bar',
<kfogel> seeing where it comes from:
<kfogel> if it was created de novo on the branch, no problem
<kfogel> if it comes from a source that [[[...]]] comes from
something on trunk, then as long as *that* thing is
up-to-date, we're okay.
<kfogel> Was that too compressed?
<glasser> (This brings us up to the vaguely related issue that
svn_{ra,fs}_get_mergeinfo is happy to fabricate paths that
don't exist for mergeinfo. eg, if /b has mergeinfo
"/t:1-5", and /b/quux is a new file (not copied) with no
svn:mergeinfo property, then
svn_foo_get_mergeinfo("/b/quux") happily returns the
mergeinfo "/t/quux:1-5")
<glasser> hmm, what do you mean by "that thing is up-to-date"? so in
this case you mean /trunk/foo?
<kfogel> holy cow
<kfogel> svn_..._get_mergeinfo() will do that?
<glasser> (Let's not even get into the fact that the mergeinfo
"/trunk:1-100" could be referring to 100 different objects
that happen to have the same name)
<kfogel> (right, /trunk/foo)
* kfogel thinks
<glasser> kfogel: Well, sure! Because you need ("need") to be able to
get the mergeinfo on arbitrary nodes on the tree! Remember
that in our model mergeinfo is the property of each node;
the fact that it is stored as "svn:mergeinfo" and inherited
implicitly is sort of only an implementatino detail. Except
that it isn't!
<kfogel> glasser: yeah, our compression is lossy in particularly
gruesome way
<kfogel> glasser: I think the answer is: we are going to have
mergeinfo bugs.
<kfogel> The question is, how often do they come up, and how serious
are they?
<kfogel> glasser: if you could go back, would you (as I think I
might) want to disallow svn:mergeinfo on anything but the
top of a branch, and then make svn formally aware of tops of
branches?
<epg> people need to be able to pick just the foo.c out of rXXX
that also changed bar.c
<kfogel> yes, if cherry-picking is the way we're going, we might as
well be good at it
<kfogel> glasser: the most likely edgy thing to happen on a branch is
renames.
<kfogel> So if tracing back locations until we get to a source-safe
source (wow, didn't expect the double pun there) is enough
to satisfy reintegrate, I *think* we can Not Worry about the
other edge cases for now, though I admit they are legion.
<glasser> yes, renames is the reasonable thing
<glasser> I don't really understand what you mean by a source-safe
source here
<glasser> I mean I think I know which thing you're referring to but
not what an actual definition would be
<glasser> I mean, presumably you do work on the branch. and then you
rename. and then you do more work
<glasser> and I'm pretty damn sure that if you try to merge from trunk
*after* the rename there's no way you're going to get
mergeinfo onto the new path that represents the last trunk
work
<glasser> i mean, presumably what happens is you get a "skip" in your
merge because the trunk's file doesn't exist, and you have
to manually patch it in or something (right?)
<kfogel> You mean right now, or what "should" happen?
<glasser> I guess perhaps manually patching it in might actually add
to the mergeinfo. I'm not sure
<glasser> um, either, both?
<kfogel> let me try to be more precise
<glasser> I mean, admittedly renames and merges go together poorly
even without merge tracking
<kfogel> when we encounter a file on the branch (merge source) that
doesn't have a partner on trunk (merge dest), we see if the
file on branch comes from a file that comes from trunk.
<kfogel> Crap, I hate having to hand wave this much.
<kfogel> glasser: I know you're available now, but could I experiment
with this a bit and get back to you on-list tomorrow?
Some thoughts:
The problem is that "/branch/bar" has mergeinfo "/trunk/foo:10-20".
If the file's name were still "foo", that would be fine (the mergeinfo
would be redundant and probably even elided out). But as the file's
name is "bar", the reintegrate code can't certify that the branch has
uniform mergeinfo, and therefore cannot proceed.
Possible solution:
When you encounter a file ("/branch/bar") whose mergeinfo disturbs the
desired uniformity, run:
svn_repos_node_location_segments(repos, "/branch/bar"
<HIGHER_REV_OF_MERGE__PROBABLY_HEAD>,
SVN_INVALID_REVNUM,
SVN_INVALID_REVNUM,
receiver_func, receiver_baton,
authz_read_func, authz_read_baton,
pool);
The receiver_func should check the (svn_location_segment_t *)->path on
each invocation. If any of invocations have a path P that
a) corresponds to a path T in the merge target, and
b) T matches the path given in the problematic mergeinfo, and
c) P has no mergeinfo /or/ has mergeinfo that would be compatible
with the uniform mergeinfo requirement
then we can proceed with the reintegration. (Remember, P is really a
rev/path pair, since it comes from an 'svn_location_segment_t'.)
Also, if P is completely new on the branch (that is, it never traces
back to any file that corresponds to a T on trunk), then we should
just ignore its mergeinfo when determining uniformity.
Now, these strategies will not lead to pure, unadulterated goodness.
Incoming merged changes still won't actually *follow* renames, so any
changes to /branch/bar will just 'skip' when reintegrated into /trunk.
It might be worth holding on to all the rename-tracebacks and printing
out (and/or storing) their results in some useful way on the client
side at the end of the merge, just so the user has somewhere to look
to figure out any manual cleanup that has to be done.
(Actually, the change swill 'skip' only if there's no /trunk/bar;
they'll try to merge and likely get rejected if there *is* a
/trunk/bar, but that's not so much different from a 'skip' anyway.)
Thoughts?
I realize there are surely more edge cases here, but renames are the
big one.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-20 21:58:46 CET