You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2009/12/09 21:08:11 UTC
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)?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Paul Burba wrote:
> Fixed all the aforementioned problems (e.g. svn mergeinfo and svn
> merge bustage) in r889840. Will nominate for backport later tonight.
You, my friend, are a rawk star.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
On Fri, Dec 11, 2009 at 5:38 PM, Paul Burba <pt...@gmail.com> wrote:
> On Thu, Dec 10, 2009 at 11:38 PM, Paul Burba <pt...@gmail.com> wrote:
>
>> I fixed svn_mergeinfo_parse(), svn_mergeinfo_to_string(),
>> svn_mergeinfo__catalog_to_formatted_string(), and
>> svn_mergeinfo__to_formatted_string(), so that any time we create
>> svn_mergeinfo_t from a string or vice-versa, we tolerate relative
>> paths in the input, but create absolute paths in the output. This
>> solves the original problem Mike saw with his sync merges...
>>
>>>> Properties on '.':
>>>> svn:mergeinfo
>>>> /subversion/trunk:879653-880579
>>>> subversion/trunk:880580-880583
>>>
>>> FWIW, we already have this problem on our 1.6.x branch.
>>
>> ...but in cases like this branch, it actually makes things worse,
>> because when the second "identical" key/val is parsed, it overwrites
>> the first one, so we lose mergeinfo during merges and get partial
>> (i.e. wrong) answers to svn mergeinfo queries. We should be able to
>> simply merge the results for identical keys (I'll look at that
>> tomorrow). Other than that I don't see any other problems, beyond
>> what I already mentioned regarding svn_mergeinfo_diff (which isn't
>> much of a problem).
>>
>> Paul
>
> Fixed all the aforementioned problems (e.g. svn mergeinfo and svn
> merge bustage) in r889840. Will nominate for backport later tonight.
Done. Please review if you have a chance.
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 10, 2009 at 11:38 PM, Paul Burba <pt...@gmail.com> wrote:
> I fixed svn_mergeinfo_parse(), svn_mergeinfo_to_string(),
> svn_mergeinfo__catalog_to_formatted_string(), and
> svn_mergeinfo__to_formatted_string(), so that any time we create
> svn_mergeinfo_t from a string or vice-versa, we tolerate relative
> paths in the input, but create absolute paths in the output. This
> solves the original problem Mike saw with his sync merges...
>
>>> Properties on '.':
>>> svn:mergeinfo
>>> /subversion/trunk:879653-880579
>>> subversion/trunk:880580-880583
>>
>> FWIW, we already have this problem on our 1.6.x branch.
>
> ...but in cases like this branch, it actually makes things worse,
> because when the second "identical" key/val is parsed, it overwrites
> the first one, so we lose mergeinfo during merges and get partial
> (i.e. wrong) answers to svn mergeinfo queries. We should be able to
> simply merge the results for identical keys (I'll look at that
> tomorrow). Other than that I don't see any other problems, beyond
> what I already mentioned regarding svn_mergeinfo_diff (which isn't
> much of a problem).
>
> Paul
Fixed all the aforementioned problems (e.g. svn mergeinfo and svn
merge bustage) in r889840. Will nominate for backport later tonight.
Spent some time looking to see where else relative merge source path
mergeinfo might burn us. Fortunately things don't look that. One
obvious problem area, svn log -g, is unaffected, regardless of whether
your client or server has rr889840. The very short story here is
svn_fs__canonicalize_abspath() saves us from trouble.
The non-merge, non-mergeinfo problem areas are:
1) svnadmin load preserves relative-path mergeinfo.
2) If you are converting svnmerge.py properties to svn:mergeinfo with
svnsync and the svnmerge-integrated props being converted have
relative paths, these will be preserved.
3) svndumpfilter, like svnadmin load, preserves any relative pathed
mergeinfo in the dump stream.
4) Serving your repos via Apache or svnserve: Relative paths in URL
sources for copies/moves are preserved in the destination. Also, if
you are merging to a path without explicit mergeinfo and have to
contact the server for any inherited mergeinfo, if the latter has
relative paths, you get relative inherited paths and probably the
wrong merge.
All of these are fixed by rr889840. Note that 4 is fixed simply by
having a patched client.
Lastly I would again emphasize that you can only get relative source
path mergeinfo in two ways: First, if you use svnadmin load
--parent-dir with a relative path (issue #3547, fixed by Mike).
Second, if you manually use svn propset to create mergeinfo with
relative paths (which is no longer possible with rr889840).
Paul
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 10, 2009 at 12:25 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Dec 10, 2009 at 12:22 PM, Paul Burba <pt...@gmail.com> wrote:
>> 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:
> - snip
>>
>> # 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:
>
> FWIW, this all looks like what I would want it to do. "Fix" the
> property but hide the details in diff.
I fixed svn_mergeinfo_parse(), svn_mergeinfo_to_string(),
svn_mergeinfo__catalog_to_formatted_string(), and
svn_mergeinfo__to_formatted_string(), so that any time we create
svn_mergeinfo_t from a string or vice-versa, we tolerate relative
paths in the input, but create absolute paths in the output. This
solves the original problem Mike saw with his sync merges...
>> Properties on '.':
>> svn:mergeinfo
>> /subversion/trunk:879653-880579
>> subversion/trunk:880580-880583
>
> FWIW, we already have this problem on our 1.6.x branch.
...but in cases like this branch, it actually makes things worse,
because when the second "identical" key/val is parsed, it overwrites
the first one, so we lose mergeinfo during merges and get partial
(i.e. wrong) answers to svn mergeinfo queries. We should be able to
simply merge the results for identical keys (I'll look at that
tomorrow). Other than that I don't see any other problems, beyond
what I already mentioned regarding svn_mergeinfo_diff (which isn't
much of a problem).
Paul
> I am entirely favor of tolerating the missing slash but "silently"
> fixing it when we update the property. Showing that in diff would
> just be noise that I do not think anyone would care about seeing.
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Dec 10, 2009 at 12:22 PM, Paul Burba <pt...@gmail.com> wrote:
> 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:
- snip
>
> # 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:
FWIW, this all looks like what I would want it to do. "Fix" the
property but hide the details in diff.
> Properties on '.':
> svn:mergeinfo
> /subversion/trunk:879653-880579
> subversion/trunk:880580-880583
FWIW, we already have this problem on our 1.6.x branch.
I am entirely favor of tolerating the missing slash but "silently"
fixing it when we update the property. Showing that in diff would
just be noise that I do not think anyone would care about seeing.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: shutting down *@s.t.o
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> I'd like to do it Real Soon Now. We've got a rudimentary page[1] up on
> the new site explaining the various mailing lists and how to subscribe to
> them. I think we can pull the plug whenever somebody has the guts to
> announce a date and follow through with it.
We've had the footer in place for some time now, but frankly I think it's
effectively invisible. Nobody reads those things. I say we (the
committers) send a mail that says:
Subject: WE, THE SUBVERSION DEVS ARE ABANDONING THIS LIST IMMEDIATELY.
Dear Person Interested in the Development of Subversion,
We, the Subversion committers, are abandoning this list (effective
immediately) so we can focus our time and attention on our new dev-list,
dev@subversion.apache.org. We're *not* going to shut this list at this
time. We'll need to port conversations over to the new list from
time to time, and will make liberal use of the archives of this list.
We're simply unsubscribing from the list. Feel free to talk amongst
yourselves, of course -- enjoy the punch and finger foods, and don't
forget to tip the roaming bar maidens as you leave.
And then we actually do unsubscribe and stop encouraging this continued
bifurcation of our collective attention. I would have unsubbed already if I
wasn't so concerned about missing new threads and additions to old ones.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
shutting down *@s.t.o (was Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our future.)
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Dec 10, 2009, at 11:22 AM, Paul Burba wrote:
> 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?
I'd like to do it Real Soon Now. We've got a rudimentary page[1] up on the new site explaining the various mailing lists and how to subscribe to them. I think we can pull the plug whenever somebody has the guts to announce a date and follow through with it.
-Hyrum
[1] http://subversion.apache.org/mailing-lists.html
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
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 <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: C. Michael Pilato [mailto:cmpilato@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.
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
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 <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: C. Michael Pilato [mailto:cmpilato@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
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Dec 9, 2009 at 4:08 PM, C. Michael Pilato <cm...@collab.net> wrote:
> 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)?
Looking into making the code tolerant of either (i.e. always treating
it as repos absolute regardless of the leading slash).
Paul
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Why did this thread jump to the subversion.tigris.org list? Please put it
back where I started it (and don't move any more threads in the future).
Thanks.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2429255
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Dec 10, 2009 at 12:29 PM, Jon Foster <Jo...@cabot.co.uk> wrote:
> Hi,
>
> Paul Burba [ptburba@gmail.com] wrote:
>> [patch snipped]
>
> If you do this, please consider adding a new capability that can be
> tested in the start-commit hook?
>
> I hope I don't ever get bitten by this bug. But if I do, I'll want to
> make absolutely sure that all my users have a SVN client that
> understands the new mergeinfo format.
It is not a new format. It is making the code more tolerant of, and
fix, an invalid format. Which in theory can only be created by
manually mucking with the property value.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
RE: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Jon Foster <Jo...@cabot.co.uk>.
Hi,
Paul Burba [ptburba@gmail.com] wrote:
> [patch snipped]
If you do this, please consider adding a new capability that can be
tested in the start-commit hook?
I hope I don't ever get bitten by this bug. But if I do, I'll want to
make absolutely sure that all my users have a SVN client that
understands the new mergeinfo format.
Kind regards,
Jon
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Re: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 10, 2009 at 4:25 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: C. Michael Pilato [mailto:cmpilato@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.
RE: HEADS UP: Damaged mergeinfo in our history; likely pain in our
future.
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@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.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2429139
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.