Resending this to get it back on s.a.o. Bert's reply (which I
originally replied to all to) jumped it to s.t.o. When are we
shutting down the s.t.o mailing lists again?
On Thu, Dec 10, 2009 at 4:25 AM, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
>> -----Original Message-----
>> From: C. Michael Pilato [mailto:cmpilato_at_collab.net]
>> Sent: woensdag 9 december 2009 22:08
>> To: Subversion Development
>> Subject: HEADS UP: Damaged mergeinfo in our history; likely pain in our
>> future.
>>
>> While trying to figure out why my attempt at performing a catch-up merge of
>> the 'issue-3242-dev' branch with ^/subversion/trunk was conflicting like
>> crazy for me today, I discovered something unhappy:
>>
>> $ svn pget svn:mergeinfo .
>> subversion/branches/1.5.x-r30215:870312
>> subversion/branches/bdb-reverse-deltas:872050-872529
>> subversion/branches/diff-callbacks3:870059-870761
>> [...]
>>
>> See the leading slashes on those mergeinfo paths? Oh, that's right, you
>> can't see them *because they aren't there*!
>>
>> A quick glance at the code behind 'svnadmin load --parent-dir' which is
>> supposed to prepend the parent directory onto each of the mergeinfo paths
>> confirms that the code naively does this prepending without any attention to
>> the possibility that the parent directory lacks a leading slash. While
>> that's not an errorful situation in general (parent_dir lacking the leading
>> slash), the slash-less value certainly should have been detected and
>> corrected before being used in a storage format that cares deeply about such
>> things (like our svn:mergeinfo property format does).
>>
>> So. Bug in libsvn_repos loading code. Filed as issue #3547. I'm testing a
>> fix for that now.
>>
>> But the bigger question is, "Where do we go from here?" I suspect that
>> *all* of our repository-stored mergeinfo -- the entire history thereof -- is
>> technically, syntactically invalid at this point. Do we try to repair it?
>> Or do we take this opportunity to make our merge tracking code a little more
>> ... flexible (always storing one format, but accepting either)?
>
> I think we can (and probably should) be a bit more flexible here. I don't see major issues in accepting paths without a '/' here.
>
> I would actually prefer repository root relative paths without '/', but we can't change the design on these values. (The new svn_relpath_ functions should be used on them but can't because they have a leading '/')
>
> I hope we can fix this by just updating the code that parses the property into our internal representation, but I think Paul can answer that easily.
From the "It's Never Quite So Simple" Files...
It's easy enough to tweak svn_mergeinfo_parse() to create
svn_mergeinfo_t with absolute path keys even if the input has relative
paths -- see the attached path. This solves Mike's original problem
with the sync merge to the 'issue-3242-dev' branch. It also passes
our 1.6.x test suite (still running the trunk tests).
This fix does however, produce some odd side effects based on the fact
that mergeinfo that differs only by the leading slash is considered
the same. So the svn_mergeinfo_(diff|merge|remove) APIs behave oddly.
For example, if we checkout the issue 3242 branch @888941 we see the
relative path mergeinfo that the "bad" load into the ASF repos
produced:
issue-3242-dev-wc>svn pg svn:mergeinfo -vR
Properties on '.':
svn:mergeinfo
subversion/branches/1.5.x-r30215:870312
subversion/branches/bdb-reverse-deltas:872050-872529
subversion/branches/diff-callbacks3:870059-870761
subversion/branches/dont-save-plaintext-passwords-by-default:870728-871118
subversion/branches/double-delete:870511-872970
subversion/branches/explore-wc:875486,875493,875497,875507,875511,875514,875559,875580-875581,875584,875587,875611,875627,875647,875667-875668,875711-875712
,875733-875734,875736,875744-875748,875751,875758,875782,875795-875796,875830,875836,875838,875842,875852,875855,875864,875870,875873,875880,875885-875888,87589
0,875897-875898,875905,875907-875909,875935,875943-875944,875946,875979,875982-875983,875985-875986,875990,875997
subversion/branches/file-externals:871779-873302
subversion/branches/fs-rep-sharing:869036-873803
subversion/branches/fsfs-pack:873717-874575
subversion/branches/gnome-keyring:870558-871410
subversion/branches/http-protocol-v2:874395-876041
subversion/branches/in-memory-cache:869829-871452
subversion/branches/issue-2843-dev:871432-874179
subversion/branches/issue-3000:871713,871716-871719,871721-871726,871728,871734
subversion/branches/issue-3067-deleted-subtrees:873375-874084
subversion/branches/issue-3148-dev:875193-875204
subversion/branches/issue-3220-dev:872210-872226
subversion/branches/issue-3334-dirs:875156-875867
subversion/branches/kwallet:870785-871314
subversion/branches/log-g-performance:870941-871032
subversion/branches/merge-skips-obstructions:874525-874615
subversion/branches/ra_serf-digest-authn:875693-876404
subversion/branches/reintegrate-improvements:873853-874164
subversion/branches/subtree-mergeinfo:876734-878766
subversion/branches/svn-mergeinfo-enhancements:870119-870195,870197-870288
subversion/branches/svnpatch-diff:865738-876477
subversion/branches/svnraisetc:874709-875149
subversion/branches/svnserve-logging:869828-870893
subversion/branches/tc-issue-3334:874697-874773
subversion/branches/tc-merge-notify:874017-874062
subversion/branches/tc-resolve:874191-874239
subversion/branches/tc_url_rev:874351-874483
subversion/branches/tree-conflicts:868291-873154
subversion/branches/tree-conflicts-notify:873926-874008
subversion/trunk:879653-880579
# With the patch in place (this is a 1.6.x client) a merge
# covering already merged revisions now works...
issue-3242-dev-wc>svn merge ^^/subversion/trunk . -r0:880583
--- Merging r880580 through r880583 into '.':
U tools\buildbot\master\master.cfg
issue-3242-dev-wc>svn st
M .
M tools\buildbot\master\master.cfg
# ...and while diff appears to show that the mergeinfo
# has changed only by the newly merged revisions...
issue-3242-dev-wc>svn diff --depth empty
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Merged /subversion/trunk:r880580-880583
# ...when we actually look at the mergeinfo we see that
# *all* of it has been quietly "fixed" to absolute paths:
issue-3242-dev-wc>svn pg svn:mergeinfo -vR
Properties on '.':
svn:mergeinfo
/subversion/branches/1.5.x-r30215:870312
/subversion/branches/bdb-reverse-deltas:872050-872529
/subversion/branches/diff-callbacks3:870059-870761
/subversion/branches/dont-save-plaintext-passwords-by-default:870728-871118
/subversion/branches/double-delete:870511-872970
/subversion/branches/explore-wc:875486,875493,875497,875507,875511,875514,875559,875580-875581,875584,875587,875611,875627,875647,875667-875668,875711-87571
2,875733-875734,875736,875744-875748,875751,875758,875782,875795-875796,875830,875836,875838,875842,875852,875855,875864,875870,875873,875880,875885-875888,8758
90,875897-875898,875905,875907-875909,875935,875943-875944,875946,875979,875982-875983,875985-875986,875990,875997
/subversion/branches/file-externals:871779-873302
/subversion/branches/fs-rep-sharing:869036-873803
/subversion/branches/fsfs-pack:873717-874575
/subversion/branches/gnome-keyring:870558-871410
/subversion/branches/http-protocol-v2:874395-876041
/subversion/branches/in-memory-cache:869829-871452
/subversion/branches/issue-2843-dev:871432-874179
/subversion/branches/issue-3000:871713,871716-871719,871721-871726,871728,871734
/subversion/branches/issue-3067-deleted-subtrees:873375-874084
/subversion/branches/issue-3148-dev:875193-875204
/subversion/branches/issue-3220-dev:872210-872226
/subversion/branches/issue-3334-dirs:875156-875867
/subversion/branches/kwallet:870785-871314
/subversion/branches/log-g-performance:870941-871032
/subversion/branches/merge-skips-obstructions:874525-874615
/subversion/branches/ra_serf-digest-authn:875693-876404
/subversion/branches/reintegrate-improvements:873853-874164
/subversion/branches/subtree-mergeinfo:876734-878766
/subversion/branches/svn-mergeinfo-enhancements:870119-870195,870197-870288
/subversion/branches/svnpatch-diff:865738-876477
/subversion/branches/svnraisetc:874709-875149
/subversion/branches/svnserve-logging:869828-870893
/subversion/branches/tc-issue-3334:874697-874773
/subversion/branches/tc-merge-notify:874017-874062
/subversion/branches/tc-resolve:874191-874239
/subversion/branches/tc_url_rev:874351-874483
/subversion/branches/tree-conflicts:868291-873154
/subversion/branches/tree-conflicts-notify:873926-874008
/subversion/trunk:879653-880583
Revving svn_mergeinfo_diff() to be able to consider differences in
abs/rel paths would solve this particular problem. Of course this
assumes we *want* to correct rel path mergeinfo we come across and are
changing anyway. I tend to think we do, otherwise we'll end up with
mergeinfo with mixed rel/abs paths, quite likely with a rel and abs
version of the *same* path, e.g.:
Properties on '.':
svn:mergeinfo
/subversion/trunk:879653-880579
subversion/trunk:880580-880583
Which is horrible from a human-readable perspective alone, not to
mention the fact that we effectively have two instances of the same
key when creating svn_mergeinfo_t from this value (hilarity is sure to
ensue if we allow this).
Anyway, I'm working to be sure we understand all the
implications...more to come, just brain dumping where I am at the
moment.
Paul
Received on 2009-12-10 18:22:49 CET