On Fri, Nov 19, 2010 at 11:56 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Thu, Nov 18, 2010 at 8:12 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>> On Thu, Nov 18, 2010 at 7:11 PM, <stsp_at_apache.org> wrote:
>>> Author: stsp
>>> Date: Fri Nov 19 00:11:15 2010
>>> New Revision: 1036686
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1036686&view=rev
>>> Log:
>>> * STATUS: Downgrade my vote for r1036429.
>>>
>>> Modified:
>>> subversion/branches/1.6.x/STATUS
>>>
>>> Modified: subversion/branches/1.6.x/STATUS
>>> URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1036686&r1=1036685&r2=1036686&view=diff
>>> ==============================================================================
>>> --- subversion/branches/1.6.x/STATUS (original)
>>> +++ subversion/branches/1.6.x/STATUS Fri Nov 19 00:11:15 2010
>>> @@ -111,7 +111,9 @@ Candidate changes:
>>> Justification:
>>> Avoids an assert in the server.
>>> Votes:
>>> - +1: philip, stsp
>>> + +1: philip
>>> + +0: stsp (it fixes the test, but pburba asked on IRC how this change
>>> + will affect replaced directories and I'm not sure about that)
>>
>> Hi Philip,
>>
>> My question was what happens if, in svnsync test 29, if in r3,
>> ^/trunk/H/B is not a replacement resulting from a copy, but is a
>> replacement without history from svn mkdir. I momentarily thought
>> that we'd end up in replay.c:add_subdir and would hit this block:
>>
>> if (! change->copyfrom_known)
>> {
>> SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev,
>> &change->copyfrom_path,
>> target_root, new_path, pool));
>> change->copyfrom_known = TRUE;
>> }
>>
>> and call svn_fs_copied_from() for a path-rev that wasn't actually
>> copied. But when looking closer into this that seems impossible,
>> because we'd never be in add_subdir() in the first place...I
>> *think*...this part of the code is quite unfamiliar to me :-\
>
> Scratch that, there *is* a problem:
>
> First apply this patch to break out of the svnsync tests before we
> remove the svn:sync* revprops:
>
> [[[
> Index: subversion/tests/cmdline/svnsync_tests.py
> ===================================================================
> --- subversion/tests/cmdline/svnsync_tests.py (revision 1036857)
> +++ subversion/tests/cmdline/svnsync_tests.py (working copy)
> @@ -159,6 +159,7 @@
> run_sync(dest_sbox.repo_url)
> run_copy_revprops(dest_sbox.repo_url)
>
> + exit(0)
> # Remove some SVNSync-specific housekeeping properties from the
> # mirror repository in preparation for the comparison dump.
> for prop_name in ("svn:sync-from-url", "svn:sync-from-uuid",
> ]]]
>
> ### Then run svnsync_tests.py 29 with BDB:
>
> C:\SVN\src-branch-1.6.x>run.python.test.DEBUG.bat svnsync 29 --fs-type bdb -v
> C:\SVN\src-branch-1.6.x>set TESTNAME=svnsync
> C:\SVN\src-branch-1.6.x>set CONFIG=Debug
> C:\SVN\src-branch-1.6.x>if not exist Debug\subversion\tests\cmdline
> mkdir Debug\subversion\tests\cmdline
> C:\SVN\src-branch-1.6.x>pushd Debug\subversion\tests\cmdline
> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline>python
> C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py 29
> --fs-type bdb -v
> CMD: svnadmin.exe create "svn-test-work\local_tmp\repos"
> --bdb-txn-nosync "--fs-type=bdb" <TIME = 0.127000>
> CMD: svn.exe import -m "Log message for revision 1."
> "svn-test-work\local_tmp\greekfiles"
> "file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-
> test-work/local_tmp/repos" --config-dir
> "C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
> --password rayjandom --no-auth-c
> ache --username jrandom <TIME = 0.077000>
> Adding svn-test-work\local_tmp\greekfiles\A
> Adding svn-test-work\local_tmp\greekfiles\A\B
> Adding svn-test-work\local_tmp\greekfiles\A\B\lambda
> Adding svn-test-work\local_tmp\greekfiles\A\B\E
> Adding svn-test-work\local_tmp\greekfiles\A\B\E\alpha
> Adding svn-test-work\local_tmp\greekfiles\A\B\E\beta
> Adding svn-test-work\local_tmp\greekfiles\A\B\F
> Adding svn-test-work\local_tmp\greekfiles\A\mu
> Adding svn-test-work\local_tmp\greekfiles\A\C
> Adding svn-test-work\local_tmp\greekfiles\A\D
> Adding svn-test-work\local_tmp\greekfiles\A\D\gamma
> Adding svn-test-work\local_tmp\greekfiles\A\D\G
> Adding svn-test-work\local_tmp\greekfiles\A\D\G\pi
> Adding svn-test-work\local_tmp\greekfiles\A\D\G\rho
> Adding svn-test-work\local_tmp\greekfiles\A\D\G\tau
> Adding svn-test-work\local_tmp\greekfiles\A\D\H
> Adding svn-test-work\local_tmp\greekfiles\A\D\H\chi
> Adding svn-test-work\local_tmp\greekfiles\A\D\H\omega
> Adding svn-test-work\local_tmp\greekfiles\A\D\H\psi
> Adding svn-test-work\local_tmp\greekfiles\iota
>
> Committed revision 1.
> CMD: svnadmin.exe create "svn-test-work\repositories\svnsync_tests-29"
> --bdb-txn-nosync "--fs-type=bdb" <TIME = 0.150000>
> CMD: svnadmin.exe load --force-uuid --quiet
> "svn-test-work\repositories\svnsync_tests-29" <TIME = 0.090000>
> CMD: svnadmin.exe create
> "svn-test-work\repositories\svnsync_tests-29-1" --bdb-txn-nosync
> "--fs-type=bdb" <TIME = 0.150000>
> CMD: svnlook.exe uuid "svn-test-work\repositories\svnsync_tests-29"
> <TIME = 0.026000>
> b105e986-03ce-ba4b-b01a-b7a10dd0b7f5
> CMD: svnadmin.exe setuuid
> "svn-test-work\repositories\svnsync_tests-29-1"
> b105e986-03ce-ba4b-b01a-b7a10dd0b7f5 <TIME = 0.027000>
> CMD: svnsync.exe initialize
> "file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1"
> "file:///C%3A/SVN/s
> rc-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29/trunk/H"
> --username jrandom --password rayjandom --config-dir "C:\SVN
> \src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
> <TIME = 0.339000>
> Copied properties for revision 0.
> CMD: svnsync.exe synchronize
> "file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1"
> --username jrandom
> --password rayjandom --config-dir
> "C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
> <TIME = 0.929000>
> Committed revision 1.
> Copied properties for revision 1.
> Committed revision 2.
> Copied properties for revision 2.
> Transmitting file data ........
> Committed revision 3.
> Copied properties for revision 3.
> CMD: svnsync.exe copy-revprops
> "file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1"
> --username jrand
> om --password rayjandom --config-dir
> "C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
> <TIME = 0.690000>
> Copied properties for revision 0.
> Copied properties for revision 1.
> Copied properties for revision 2.
> Copied properties for revision 3.
> EXCEPTION: SystemExit(0), skipping cleanup
> PASS: svnsync_tests.py 29: descending into replaced dir looks in src
>
> ### Notice that r3 was successfully synced above
>
> ### To spare you looking at the test, r3 is a copy with a replacement
> with history within it:
>
> C:\SVN\src-branch-1.6.x>svn log -v -r3 %URL%
> ------------------------------------------------------------------------
> r3 | Daniel | 2010-07-08 17:25:07 -0400 (Thu, 08 Jul 2010) | 1 line
> Changed paths:
> A /trunk/H (from /trunk/A:2)
> R /trunk/H/B (from /trunk/X:2)
>
> cp
> ------------------------------------------------------------------------
>
> ### Now use svnmucc to create r4, a change similar to r3, but this
> time the replacement
> ### is without history:
>
> C:\SVN\src-branch-1.6.x>set
> URL=file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29
>
> C:\SVN\src-branch-1.6.x>svnmucc.exe -mm cp head %URL%/trunk/A
> %URL%/trunk/H/H1 rm %URL%/trunk/H/H1/B mkdir %URL%/trunk/H/H1/B
> r4 committed by pburba at 2010-11-19T16:49:47.096098Z
>
> ### And boom, the svnsync sync is broken:
>
> C:\SVN\src-branch-1.6.x>svnsync synchronize %URL%-1
> Transmitting file data ...\..\..\subversion\libsvn_fs_base\tree.c:739:
> (apr_err=160013)
> svnsync: File not found: revision 4, path '/trunk/H/H1/B/lambda'
>
> Paul
I reopened issue #3641 and tweaked the test for that issue to
demonstrate this problem, see
http://svn.apache.org/viewvc?view=revision&revision=1036978.
So what to do about the r1036429 backport nomination? With r1036429
in place, the sync of a replace without history within a copy results,
as described in my previous post, in a "svnsync: File not found:
revision 4, path '/trunk/H/H1/B/lambda'" error. However, without
r1036429 the assert in replay.c is triggered, which strikes me as less
desirable. Now what about r962378, which added the assert and is
already backported? If we remove revert that change, then issue #3641
is alive and well on 1.6.x, but there is no assert, both of the
'replaced-within-copied' variants give the 'File not found' error.
So unless somebody has a fix coming soon for issue #3641, I think we
should either backport r1036429 or veto it *and* revert r962378. The
third option, leaving r962378 but not backporting r1036429 seems the
worst of all worlds, since we are introducing an assert.
Thoughts?
Paul
Paul
Received on 2010-11-19 20:51:34 CET