On Wed, Feb 20, 2008 at 12:58 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> 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'.)
OK. But is this likely to actually occur in the common case? I mean,
great, so we trace back /branch/bar to /trunk/foo and then, um, I
guess do yet another svn_ra_get_mergeinfo on that. But there's no
mergeinfo *on* /trunk/foo *from* /trunk/foo, which is exactly the
mergeinfo we're looking for. So does this actually work?
> 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.
That does seem reasonable enough. Note now that "completely new on
branch" (not copied, just made there) files will currently claim to
have mergeinfo (inherited from the branch root, say), which is weird
and probably leads to other problems later, but at least isn't
problematic here.
--dave
> 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
>
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
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-27 00:53:50 CET