Re: svn commit: r1066276 - /subversion/trunk/subversion/libsvn_subr/io.c
On 2/1/11 3:47 PM, s...@apache.org wrote: Author: stsp Date: Tue Feb 1 23:47:37 2011 New Revision: 1066276 URL: http://svn.apache.org/viewvc?rev=1066276&view=rev Log: Follow-up to 1066249: Unbreak build with thread-less APR. * subversion/libsvn_subr/io.c (FILE_LOCK_RETRY_LOOP): Declare correctly if !APR_HAS_THREADS. Thanks for fixing the mistake! Blair
Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)
Johan, On Tue, Feb 1, 2011 at 11:16 PM, Johan Corveleyn wrote: > On Tue, Feb 1, 2011 at 10:10 PM, Johan Corveleyn > wrote: > > it continues until test nr 58, and then gives the popup. > > > > Hm, I'm confused. I guess I'm going to fire up my debugger and set a > > breakpoint or something to see what happens and why ... > > Ok, I'm starting to understand. If I do this with a Debug build, it > crashes without the popup. If I'm running the test with a Release > build, it gives the annoying popup, blocking the rest of the test > suite. > > Is it supposed to be that way? I guess I should just as well be able > to run the full test suite, unattended, with a Release build, no? > The idea was that people who have Visual Studio installed might be more interested in seeing the code resulting in a crash in their debugger than as a textual stacktrace. This choice is hardcoded in subversion/libsvn_subr/win32_crashrpt.c:svn__unhandled_exception_filter (lines 723-724). I'm not against using SVN_DBG_STACKTRACES_TO_STDERR also as a flag to trigger this, or add (yet) another flag. regards, Lieven
Windows Makefile (was: Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir))
Johan Corveleyn wrote on Tue, Feb 01, 2011 at 13:28:24 +0100: > So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py > (put it in comment, ran "nmake config" and rebuilt everything), then > ran update_tests.py again: same result. It still crashes, and shows > the ugly blocking popup. > I don't use Windows as my day-to-day OS any more, but I suppose it'll be a good idea to adhere to my old promise to commit that Makefile once it has seen some testing :-). Johan, could you do that? Or send it to me and I'll do it. Belatedly, Daniel (as sibling to stsp's tools/dev/unix-build/ I suggest) > Anyone else who recently built trunk on Windows seeing this, when > running update_tests.py? > > -- > Johan
Re: Roadmap : 1.7 Release Status : Test Review : XFails
Paul Burba writes: > Hi All, > > One of the roadmap items yet to be started was a 'test review' to > 'Determine which XFail and WIP tests should remain so, and which need > to be fixed before release.' > > I took a look at this today. We currently have 61 tests set to XFail > (2 of these are WIPs). Here is how they break down: > > A) Tests with an associated issue (35). > > The target milestone of these 35 break down like this: > > (8) Never scheduled, i.e. '---' > (9) Unscheduled [3 of these are marked as RESOLVED] > (1) 1.6.1 > (3) 1.7.0 > (8) 1.7-consider > (1) 1.8.0 > (5) 1.8.0-consider > > B) 26 tests have no issue associated with them (well there *might* be > an issue, but a cursory look in the issue tracker didn't reveal > anything obvious). > > So a whopping 55 tests are *potentially* release blockers (all but the > 1.8.0* milestone issues). > > I put a summary of all 61 Xfailing tests in > ^/subversion/trunk/notes/xfail-status[1]. > > For many of the tests I listed a "point person" as an educated guess > as to who is best equipped to decide the fate of the test for 1.7. In > the case of tests with an associated issue that is assigned to > someone, the point person is the assignee. For issues not assigned to > anyone, I left this blank. In the case of tests with no associated > issues, this is usually the person who committed the test or changed > it to XFail. If I cc'ed you then you are one of the lucky point > people :-P > > Keep in mind this is just a guess, if your name is there you aren't > compelled to do anything...though I suspect we'd all agree it's bad > form to create an XFailing test with no associated issue when we > aren't planning on fixing the underlying problem. > > If you have some time please look through the list. If a test you > wrote needs an issue please write one up. If there is an existing > issue that should be associated with a test add it. If an associated > issue is unscheduled or never scheduled and you're familiar with it, > please take a stab at some issue triage. > > Thanks, > > Paul > > [1] I'm happy to find this a new home if it belongs elsewhere. > Ideally I'd like to tweak svntest\testcase.py:XFail() so that we have > a way to associate an issue and a target milestone with each XFailing > test, so we can easily create this list. I'll take a look at doing > just that tomorrow once I get through the items I've assigned myself. I submitted a test patch for issue # 3013 http://svn.haxx.se/dev/archive-2011-01/0375.shtml Thanks and Regards Noorul
Re: [PATCH] New XFail test case for issue 3013
Noorul Islam K M writes:
> Daniel Shahaf writes:
>
>> Noorul Islam K M wrote on Fri, Jan 28, 2011 at 12:27:46 +0530:
>>
>>> Daniel Shahaf writes:
>>>
>>> > Looks good, but I have a question:
>>> >
>>> > Noorul Islam K M wrote on Wed, Jan 26, 2011 at 13:12:54 +0530:
>>> >>
>>> >> Attached is the python test for issue 3013. This incorporates the steps
>>> >> from the shell script attached in the tracker.
>>> >>
>>> >> Log
>>> >> [[[
>>> >>
>>> >> New XFail test for issue 3013.
>>> >>
>>> >> * subversion/tests/cmdline/update_tests.py
>>> >> (update_after_switching_to_deleted_path, test_list): New XFail test
>>> >>
>>> >> Patch by: Noorul Islam K M
>>> >> ]]]
>>> >>
>>> >> Thanks and Regards
>>> >> Noorul
>>> >>
>>> >
>>> >> Index: subversion/tests/cmdline/update_tests.py
>>> >> ===
>>> >> --- subversion/tests/cmdline/update_tests.py (revision 1063610)
>>> >> +++ subversion/tests/cmdline/update_tests.py (working copy)
>>> >> @@ -5347,6 +5347,34 @@
>>> >>svntest.main.run_svn(None, 'delete', os.path.join('A2', 'mu'))
>>> >>svntest.main.run_svn(None, 'update', os.path.join('A2', 'mu'))
>>> >>
>>> >> +### regression test for issue #3013
>>> >> +def update_after_switching_to_deleted_path(sbox):
>>> >> + "update after switching to deleted path"
>>> >> +
>>> >> + sbox.build()
>>> >> + wc_dir = sbox.wc_dir
>>> >> + repo_url = sbox.repo_url
>>> >> +
>>> >> + # switch to A/B
>>> >> + svntest.actions.run_and_verify_svn2(None, None, [], 0, 'switch',
>>> >> + repo_url + "/A/B", wc_dir)
>>> >> +
>>> >> + # delete A/D
>>> >> + svntest.actions.run_and_verify_svn2(None, None, [], 0, 'rm',
>>> >> + repo_url + "/A/D", '-m',
>>> >> + 'Remove A/D')
>>> >> +
>>> >> + # switch to A/D and this is known to fail
>>> >> + svntest.actions.run_and_verify_svn2(None, None,
>>> >> svntest.verify.AnyOutput,
>>> >> + 1, 'switch', repo_url + "/A/D",
>>> >> wc_dir)
>>> >> +
>>> >> + # switch to A/D@1 and this is known to succeed
>>> >> + svntest.actions.run_and_verify_svn2(None, None, [], 0, 'switch',
>>> >> + repo_url + "/A/D@1", wc_dir)
>>> >> +
>>> >> + # update should succeed
>>> >> + svntest.actions.run_and_verify_svn2(None, None, [], 0, "up", wc_dir)
>>> >> +
>>> >
>>> > Should this 'update' succeed? In my testing, updating the wc root to
>>> > a revision it does not exist in fails.
>>>
>>> "/A/D" @ revision 1 does exist. So update should succeed but it is
>>> failing. This is the issue.
>>>
>>
>> I think the issue is that the 'switch' is failing, nothing about the
>> update.
>>
>> But, in fact: both switch and update work as long as the directory
>> they're targetting is a wc subdir (as opposed to a wc root). Therefore,
>> I'm inclined to mark the issue as FIXED and adjust the test to test that
>> the switch works when the being-switched directory is not the wc root.
>>
>> Thoughts?
>>
>
> Pasting our conversation on IRC here.
>
> danielsh_: The switch is known fail @HEAD and not @1 [12:48]
> A/D is non-existent @HEAD and existent @1 hence first switch is known
>to fail and the second one to succeed
> When the second one succeeds the subsequent update should update WC
>with repo_url@1 [12:49]
> noorul: see mail, I think it's a different issue,
> the switch actually works when you do 'svn sw $URL
> /path/to/wc/rootdir/some/subdir'
> I think I correctly converted
>
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/1157/3013.sh
>into a test [12:52]
> yes
> I think the 'update' shouldn't have been in the .sh script either
> btw [12:53]
> but that's history
> Before svn update if we use 'svn info' we could see that it is
>pointing to repo_url@1 [12:54]
> Shouldn't svn up bring wc in sync with that revision?
> A/D existing @1 the error message is misleading [12:55]
> currently the 'up' fails iff the
> dir-being-updated-past-its-deletion is the wc root
> I think that's sensible,
> to error if the wcroot would have to be deleted,
> but feel free to discuss that on dev@
> re error messages: haven't checked whether they're misleading or
> not
> haven't read them actually, just checked if the up succeeded or
> not and what rev it put me at
> danielsh_: Do you mean to say that "svn up A/D" would succeed instead
>of "svn up ." [13:03]
> noorul: svn rm ^/A/D; svn up A/D/; <-- succeeds [13:04]
> noorul: svn co ^/A foo; cd foo; svn rm ^/A; svn up; <-- fails
> that's current behaviour [13:05]
> danielsh_: With respect to the test case [13:06]
> I mean the test case that I submitted, do you think that svn up
>should succeed?
> noorul: I think the 'svn up' doesn't belong in that test case, I
>
Re: Roadmap : 1.7 Release Status : Test Review : XFails
> LISTING: merge_tests.py > You missed: merge_tests.py: XFail(SkipUnless(merge_change_to_file_with_executable, merge_tests.py-svntest.main.is_posix_os)), which Daniel Becroft (CC'd) recently added, and has submitted a patch that seeks to fix the underlying problem. Issue #3686, r1061020. Daniel: please look at Paul's email (to which I am replying) if you haven't already and consider yourself assigned to this XFail. :-)
Re: Roadmap : 1.7 Release Status : Test Review : XFails
> LISTING: export_tests.py > > 20XFAIL exporting a file refuses to silently overwrite > Issue #: None > Target Milestone: N/A > Thread: N/A > Log: r1037998 > Point Person: danielsh > I'm not sure the test is valid --- doesn't it need a "--force" added to the 'export' invocation? I don't remember what's the context of this patch (the thread or IRC discussion that led to writing it). "What the hell was I doing?" > LISTING: patch_tests.py > > Test # Mode Test Description > -- - > 29XFAIL patch doesn't append newline to properties > Issue #: None > Target Milestone: N/A > Thread: N/A > Log: r1033709 > Point Person: danielsh I added it after IRC discussion, but IIRC dannas/stsp later had other ideas in this area. Not sure what the current plan is.
Re: Roadmap : 1.7 Release Status : Test Review : XFails
Paul Burba writes: > Hi All, > > One of the roadmap items yet to be started was a 'test review' to > 'Determine which XFail and WIP tests should remain so, and which need > to be fixed before release.' > > I took a look at this today. We currently have 61 tests set to XFail > (2 of these are WIPs). Here is how they break down: > > A) Tests with an associated issue (35). > > The target milestone of these 35 break down like this: > > (8) Never scheduled, i.e. '---' > (9) Unscheduled [3 of these are marked as RESOLVED] > (1) 1.6.1 > (3) 1.7.0 > (8) 1.7-consider > (1) 1.8.0 > (5) 1.8.0-consider > > B) 26 tests have no issue associated with them (well there *might* be > an issue, but a cursory look in the issue tracker didn't reveal > anything obvious). > > So a whopping 55 tests are *potentially* release blockers (all but the > 1.8.0* milestone issues). > > I put a summary of all 61 Xfailing tests in > ^/subversion/trunk/notes/xfail-status[1]. > > For many of the tests I listed a "point person" as an educated guess > as to who is best equipped to decide the fate of the test for 1.7. In > the case of tests with an associated issue that is assigned to > someone, the point person is the assignee. For issues not assigned to > anyone, I left this blank. In the case of tests with no associated > issues, this is usually the person who committed the test or changed > it to XFail. If I cc'ed you then you are one of the lucky point > people :-P > > Keep in mind this is just a guess, if your name is there you aren't > compelled to do anything...though I suspect we'd all agree it's bad > form to create an XFailing test with no associated issue when we > aren't planning on fixing the underlying problem. > > If you have some time please look through the list. If a test you > wrote needs an issue please write one up. If there is an existing > issue that should be associated with a test add it. If an associated > issue is unscheduled or never scheduled and you're familiar with it, > please take a stab at some issue triage. > > Thanks, > > Paul > > [1] I'm happy to find this a new home if it belongs elsewhere. > Ideally I'd like to tweak svntest\testcase.py:XFail() so that we have > a way to associate an issue and a target milestone with each XFailing > test, so we can easily create this list. I'll take a look at doing > just that tomorrow once I get through the items I've assigned myself. LISTING: depth_tests.py Test # Mode Test Description -- - 36XFAIL 'info' should treat excluded item as versioned Issue #: None Target Milestone: N/A Thread: N/A Log: r876772 Point Person: danielsh I think issue #3787 is related to this XFail test. Thanks and Regards Noorul
Re: Roadmap : 1.7 Release Status : Test Review : XFails
[ http://svn.apache.org/repos/asf/subversion/trunk/notes/xfail-status ] Paul Burba wrote on Tue, Feb 01, 2011 at 21:53:34 -0500: > Hi All, > > One of the roadmap items yet to be started was a 'test review' to > 'Determine which XFail and WIP tests should remain so, and which need > to be fixed before release.' > > I took a look at this today. And looks like you did a thorough job at that; thanks! > Keep in mind this is just a guess, if your name is there you aren't > compelled to do anything...though I suspect we'd all agree it's bad > form to create an XFailing test with no associated issue when we > aren't planning on fixing the underlying problem. > Sometimes I commit an XFail test that I have no immediate intention to work on myself; for example, just last week I committed two XFail tests which arrived as patch submissions. There are other legitimate reasons for adding XFail tests without immediatley working on a fix, I believe. > If you have some time please look through the list. If a test you > wrote needs an issue please write one up. If there is an existing > issue that should be associated with a test add it. If an associated > issue is unscheduled or never scheduled and you're familiar with it, > please take a stab at some issue triage. > LISTING: depth_tests.py > > Test # Mode Test Description > -- - > 36XFAIL 'info' should treat excluded item as versioned > Issue #: None > Target Milestone: N/A > Thread: N/A > Log: r876772 > Point Person: danielsh Old bug. May not have an issue associated with it. I'll have a look, but it shouldn't block 1.7.0. > LISTING: prop_tests.py > > Test # Mode Test Description > -- - > 12XFAIL set, get, and delete a revprop change > Issue #: 3086 > Target Milestone: 1.8-consider > Thread: N/A > Log: N/A > Point Person: Unassigned > > 26XFAIL test handling invalid svn:* property values > Issue #: None > Target Milestone: N/A > Thread: N/A > Log: r871212 > Point Person: danielsh These two fail for the same reason --- something to do with pre-revprop-change hooks over ra_dav. In any case, feel free to cross #26 off your list; I'm sure it will be fixed as soon as #12 is. We have lots of XFailing tests in our test suite and no easy way of telling which ones are release blockers. This is a first step to figuring out the answer to that question. LISTING: dirent_uri-test.exe Test # Mode Test Description -- - 34XFAIL test svn_dirent_get_absolute with lc drive Issue #: None Target Milestone: N/A Thread: http://svn.haxx.se/dev/archive-2009-10/0682.shtml Log: r880444 Point Person: rhuijben LISTING: fs-test.exe Test # Mode Test Description -- - 18XFAIL (WIP) merging commit [[needs to be written to match new merge() algorithm expectations]] Issue #: N/A Target Milestone: N/A Thread: N/A Log: r875421 Point Person: cmpilato 36XFAIL(WIP) obliterate 1 [[obliterate is in development]] Issue #: 516 Target Milestone: Unscheduled Thread: N/A Log: N/A Point Person: julianfoad LISTING: locks-test.exe Test # Mode Test Description -- - 9XFAIL able to reserve a name (lock non-existent path) Issue #: None Target Milestone: None Thread: N/A Log: r853631 Point Person: cmpilato 10XFAIL directory locks (kinda) Issue #: None Target Milestone: None Thread: N/A Log: r853631 Point Person: cmpilato LISTING: parse-diff-test.exe Test # Mode Test Description -- - 4XFAIL test badly formatted git diff headers Issue #: None Target Milestone: None Thread: N/A Log: r960833 Point Person: dannas LISTING: stream-test.exe Test # Mode Test Description -- - 6XFAIL test stream seeking for translated streams Issue #: 3616 Target Milestone: 1.7.0 Thread: N/A Log: r936844 Point Person: stsp LISTING: authz_tests.py Test # Mode Test Description -- - 4XFAIL (DAV and ra_svn only) test authz for read operations Issue #: 3242 Target Milestone: 1.6.13 (Status Resolved) Thread: N/A Log: r876530 Point Person: cmpilato 13XFAIL (ra_svn only) authz issue #2712 Issue #: 2712 Target Milestone: Unscheduled Thread: N/A Log: N/A Point Person: None (vgeorgescu is assigned the issue, but appears inactive). 14XFAIL (DAV and ra_svn only) switched to directory, no read access on parents Issue #: None Target Milestone: None Thread: N/A Log: r867298 Point Person: lgo LISTING: basic_tests.py Test # Mode Test Description -- - 39XFAIL remotely remove directories from two repositories Issue #: 1199 Target Milestone: Unscheduled Thread: N/A Log: N/A Point Perso
Roadmap : 1.7 Release Status : Test Review : XFails
Hi All, One of the roadmap items yet to be started was a 'test review' to 'Determine which XFail and WIP tests should remain so, and which need to be fixed before release.' I took a look at this today. We currently have 61 tests set to XFail (2 of these are WIPs). Here is how they break down: A) Tests with an associated issue (35). The target milestone of these 35 break down like this: (8) Never scheduled, i.e. '---' (9) Unscheduled [3 of these are marked as RESOLVED] (1) 1.6.1 (3) 1.7.0 (8) 1.7-consider (1) 1.8.0 (5) 1.8.0-consider B) 26 tests have no issue associated with them (well there *might* be an issue, but a cursory look in the issue tracker didn't reveal anything obvious). So a whopping 55 tests are *potentially* release blockers (all but the 1.8.0* milestone issues). I put a summary of all 61 Xfailing tests in ^/subversion/trunk/notes/xfail-status[1]. For many of the tests I listed a "point person" as an educated guess as to who is best equipped to decide the fate of the test for 1.7. In the case of tests with an associated issue that is assigned to someone, the point person is the assignee. For issues not assigned to anyone, I left this blank. In the case of tests with no associated issues, this is usually the person who committed the test or changed it to XFail. If I cc'ed you then you are one of the lucky point people :-P Keep in mind this is just a guess, if your name is there you aren't compelled to do anything...though I suspect we'd all agree it's bad form to create an XFailing test with no associated issue when we aren't planning on fixing the underlying problem. If you have some time please look through the list. If a test you wrote needs an issue please write one up. If there is an existing issue that should be associated with a test add it. If an associated issue is unscheduled or never scheduled and you're familiar with it, please take a stab at some issue triage. Thanks, Paul [1] I'm happy to find this a new home if it belongs elsewhere. Ideally I'd like to tweak svntest\testcase.py:XFail() so that we have a way to associate an issue and a target milestone with each XFailing test, so we can easily create this list. I'll take a look at doing just that tomorrow once I get through the items I've assigned myself.
Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)
On Tue, Feb 1, 2011 at 10:10 PM, Johan Corveleyn wrote:
> On Tue, Feb 1, 2011 at 3:08 PM, Bert Huijben wrote:
>>
>>
>>> -Original Message-
>>> From: Johan Corveleyn [mailto:jcor...@gmail.com]
>>> Sent: dinsdag 1 februari 2011 13:28
>>> To: Daniel Shahaf
>>> Cc: Subversion Development
>>> Subject: Re: Assertion failure during update_tests.py 58 (XFAIL: update
>>> a nonexistent child of a copied dir)
>>>
>>> On Mon, Jan 24, 2011 at 3:21 PM, Daniel Shahaf
>>> wrote:
>>> > Johan Corveleyn wrote on Mon, Jan 24, 2011 at 02:42:11 +0100:
>>> >> Hi,
>>> >>
>>> >> Already for some time now, update_tests.py 58 (XFAIL: update a
>>> >> nonexistent child of a copied dir) crashes on my machine:
>>> >>
>>> >> svn: In file '..\..\..\subversion\libsvn_wc\update_editor.c'
>>> line
>>> >> 4877: assertion failed (repos_root != NULL && repos_uuid != NULL)
>>> >>
>>> >> I understand that this test is XFAIL, that this isn't addressed yet,
>>> >> but is it supposed to fail an assert?
>>> >>
>>> >> On my system (Win XP) this causes an ugly popup to appear (which I
>>> >> need to click away to continue), each time I run the full test suite
>>> >> ("This application has requested the Runtime to terminate it in an
>>> >> unusual way...")
>>> >>
>>> >> Relevant excerpt from tests.log in attachment (this was with
>>> trunk@1062600).
>>> >>
>>> > It certainly isn't supposed to force all test runs to be interactive
>>> :-(
>>> >
>>> > Have you tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py?
>>>
>>> Almost forgot about this one, until I ran into it again yesterday
>>> evening.
>>>
>>> So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
>>> (put it in comment, ran "nmake config" and rebuilt everything), then
>>> ran update_tests.py again: same result. It still crashes, and shows
>>> the ugly blocking popup.
>>>
>>> Anyone else who recently built trunk on Windows seeing this, when
>>> running update_tests.py?
>>
>> The 'SVN_DBG_STACKTRACES_TO_STDERR' environment option that is set in
>> subversion/tests/cmdline/svntest/main.py should stop the popup dialogs while
>> running the tests. (It moves the output to stderr to allow logging them on
>> the Windows buildbots, instead of requiring interactive resolving).
>
> But it is set, and it still crashes with the popup, whether or not I
> remove the define of SVN_USE_WIN32_CRASHHANDLER in gen-win.py.
>
> If I add a print statement just below where
> SVN_DBG_STACKTRACES_TO_STDERR is set in main.py, this is the output I
> get when running update_tests.py:
>
> [[[
> C:\research\svn\client_build\svn_branch_diff-opt>python win-tests.py
> --verbose --cleanup
> --bin=C:\research\svn\client_build\svn_branch_diff-opt\dist\bin
> --release -f fsfs -t update_tests.py
>
> Testing Release configuration on local repository.
> Running tests in update_tests.py [1/1]SVN_DBG_STACKTRACES_TO_STDERR set
> .
> ]]]
>
> it continues until test nr 58, and then gives the popup.
>
> Hm, I'm confused. I guess I'm going to fire up my debugger and set a
> breakpoint or something to see what happens and why ...
Ok, I'm starting to understand. If I do this with a Debug build, it
crashes without the popup. If I'm running the test with a Release
build, it gives the annoying popup, blocking the rest of the test
suite.
Is it supposed to be that way? I guess I should just as well be able
to run the full test suite, unattended, with a Release build, no?
(I always work with a Release build lately, because of the
performance/optimization work (I consider perf-numbers with
Debug-builds more or less irrelevant)).
Cheers,
--
Johan
Re: svn commit: r1066203 - /subversion/trunk/subversion/libsvn_repos/commit.c
On 02/01/2011 04:28 PM, Hyrum K Wright wrote: > It looks like this triggers another abort: > > (gdb) r 8 --allow-segfaults > Starting program: > /Users/Hyrum/dev/svn-trunk3/subversion/tests/libsvn_repos/repos-test 8 > --allow-segfaults > Reading symbols for shared libraries ... done > Assertion failed: (svn_relpath_is_canonical(relpath, result_pool)), > function svn_fspath__join, file subversion/libsvn_subr/dirent_uri.c, > line 2538. I was just fixing this. -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r1066203 - /subversion/trunk/subversion/libsvn_repos/commit.c
It looks like this triggers another abort:
(gdb) r 8 --allow-segfaults
Starting program:
/Users/Hyrum/dev/svn-trunk3/subversion/tests/libsvn_repos/repos-test 8
--allow-segfaults
Reading symbols for shared libraries ... done
Assertion failed: (svn_relpath_is_canonical(relpath, result_pool)),
function svn_fspath__join, file subversion/libsvn_subr/dirent_uri.c,
line 2538.
Program received signal SIGABRT, Aborted.
0x7fff82ab5616 in __kill ()
(gdb) bt
#0 0x7fff82ab5616 in __kill ()
#1 0x7fff82b55cca in abort ()
#2 0x7fff82b42c90 in __assert_rtn ()
#3 0x0001000a1c9e in svn_fspath__join (fspath=0x1000d0438 "/",
relpath=0x1000cd716 "/iota", result_pool=0x10082f428) at
subversion/libsvn_subr/dirent_uri.c:2538
#4 0x0001000119a4 in delete_entry (path=0x1000cd716 "/iota",
revision=-1, parent_baton=0x10077cda8, pool=0x10082f428) at
subversion/libsvn_repos/commit.c:235
#5 0x000174d6 in commit_editor_authz (opts=0x7fff5fbff550,
pool=0x10082d428) at subversion/tests/libsvn_repos/repos-test.c:1429
#6 0x0001f722 in do_test_num (progname=0x7fff5fbff842
"repos-test", test_num=8, msg_only=0, opts=0x7fff5fbff550,
header_msg=0x0, pool=0x10082d428) at
subversion/tests/svn_test_main.c:274
#7 0x000100010248 in main (argc=3, argv=0x7fff5fbff6b8) at
subversion/tests/svn_test_main.c:526
-Hyrum
On Tue, Feb 1, 2011 at 2:57 PM, wrote:
> Author: cmpilato
> Date: Tue Feb 1 20:57:23 2011
> New Revision: 1066203
>
> URL: http://svn.apache.org/viewvc?rev=1066203&view=rev
> Log:
> Update uses of deprecated path functions.
>
> * subversion/libsvn_repos/commit.c
> (delete_entry, add_directory, open_directory, add_file, open_file):
> Use svn_fspath__join() instead of svn_path_join().
> (svn_repos_get_commit_editor5): Canonicalize the base_path as an
> fspath.
>
> Modified:
> subversion/trunk/subversion/libsvn_repos/commit.c
>
> Modified: subversion/trunk/subversion/libsvn_repos/commit.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/commit.c?rev=1066203&r1=1066202&r2=1066203&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_repos/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/commit.c Tue Feb 1 20:57:23 2011
> @@ -38,6 +38,7 @@
> #include "svn_props.h"
> #include "repos.h"
> #include "svn_private_config.h"
> +#include "private/svn_fspath.h"
> #include "private/svn_repos_private.h"
>
>
> @@ -231,7 +232,7 @@ delete_entry(const char *path,
> svn_node_kind_t kind;
> svn_revnum_t cr_rev;
> svn_repos_authz_access_t required = svn_authz_write;
> - const char *full_path = svn_path_join(eb->base_path, path, pool);
> + const char *full_path = svn_fspath__join(eb->base_path, path, pool);
>
> /* Check PATH in our transaction. */
> SVN_ERR(svn_fs_check_path(&kind, eb->txn_root, full_path, pool));
> @@ -292,7 +293,7 @@ add_directory(const char *path,
> {
> struct dir_baton *pb = parent_baton;
> struct edit_baton *eb = pb->edit_baton;
> - const char *full_path = svn_path_join(eb->base_path, path, pool);
> + const char *full_path = svn_fspath__join(eb->base_path, path, pool);
> apr_pool_t *subpool = svn_pool_create(pool);
> svn_boolean_t was_copied = FALSE;
>
> @@ -385,7 +386,7 @@ open_directory(const char *path,
> struct dir_baton *pb = parent_baton;
> struct edit_baton *eb = pb->edit_baton;
> svn_node_kind_t kind;
> - const char *full_path = svn_path_join(eb->base_path, path, pool);
> + const char *full_path = svn_fspath__join(eb->base_path, path, pool);
>
> /* Check PATH in our transaction. If it does not exist,
> return a 'Path not present' error. */
> @@ -438,7 +439,7 @@ add_file(const char *path,
> struct file_baton *new_fb;
> struct dir_baton *pb = parent_baton;
> struct edit_baton *eb = pb->edit_baton;
> - const char *full_path = svn_path_join(eb->base_path, path, pool);
> + const char *full_path = svn_fspath__join(eb->base_path, path, pool);
> apr_pool_t *subpool = svn_pool_create(pool);
>
> /* Sanity check. */
> @@ -533,7 +534,7 @@ open_file(const char *path,
> struct edit_baton *eb = pb->edit_baton;
> svn_revnum_t cr_rev;
> apr_pool_t *subpool = svn_pool_create(pool);
> - const char *full_path = svn_path_join(eb->base_path, path, pool);
> + const char *full_path = svn_fspath__join(eb->base_path, path, pool);
>
> /* Check for read authorization. */
> SVN_ERR(check_authz(eb, full_path, eb->txn_root,
> @@ -873,7 +874,7 @@ svn_repos_get_commit_editor5(const svn_d
> eb->commit_callback_baton = callback_baton;
> eb->authz_callback = authz_callback;
> eb->authz_baton = authz_baton;
> - eb->base_path = apr_pstrdup(subpool, base_path);
> + eb->base_path = svn_fspath__canonicalize(base_path, subpool);
> eb->repos = repos;
> eb->repos_url = repos_url;
> eb->repos_name = svn_dirent_basename(svn_repos_path(repos, subpool),
>
>
>
Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)
On Tue, Feb 1, 2011 at 3:08 PM, Bert Huijben wrote:
>
>
>> -Original Message-
>> From: Johan Corveleyn [mailto:jcor...@gmail.com]
>> Sent: dinsdag 1 februari 2011 13:28
>> To: Daniel Shahaf
>> Cc: Subversion Development
>> Subject: Re: Assertion failure during update_tests.py 58 (XFAIL: update
>> a nonexistent child of a copied dir)
>>
>> On Mon, Jan 24, 2011 at 3:21 PM, Daniel Shahaf
>> wrote:
>> > Johan Corveleyn wrote on Mon, Jan 24, 2011 at 02:42:11 +0100:
>> >> Hi,
>> >>
>> >> Already for some time now, update_tests.py 58 (XFAIL: update a
>> >> nonexistent child of a copied dir) crashes on my machine:
>> >>
>> >> svn: In file '..\..\..\subversion\libsvn_wc\update_editor.c'
>> line
>> >> 4877: assertion failed (repos_root != NULL && repos_uuid != NULL)
>> >>
>> >> I understand that this test is XFAIL, that this isn't addressed yet,
>> >> but is it supposed to fail an assert?
>> >>
>> >> On my system (Win XP) this causes an ugly popup to appear (which I
>> >> need to click away to continue), each time I run the full test suite
>> >> ("This application has requested the Runtime to terminate it in an
>> >> unusual way...")
>> >>
>> >> Relevant excerpt from tests.log in attachment (this was with
>> trunk@1062600).
>> >>
>> > It certainly isn't supposed to force all test runs to be interactive
>> :-(
>> >
>> > Have you tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py?
>>
>> Almost forgot about this one, until I ran into it again yesterday
>> evening.
>>
>> So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
>> (put it in comment, ran "nmake config" and rebuilt everything), then
>> ran update_tests.py again: same result. It still crashes, and shows
>> the ugly blocking popup.
>>
>> Anyone else who recently built trunk on Windows seeing this, when
>> running update_tests.py?
>
> The 'SVN_DBG_STACKTRACES_TO_STDERR' environment option that is set in
> subversion/tests/cmdline/svntest/main.py should stop the popup dialogs while
> running the tests. (It moves the output to stderr to allow logging them on
> the Windows buildbots, instead of requiring interactive resolving).
But it is set, and it still crashes with the popup, whether or not I
remove the define of SVN_USE_WIN32_CRASHHANDLER in gen-win.py.
If I add a print statement just below where
SVN_DBG_STACKTRACES_TO_STDERR is set in main.py, this is the output I
get when running update_tests.py:
[[[
C:\research\svn\client_build\svn_branch_diff-opt>python win-tests.py
--verbose --cleanup
--bin=C:\research\svn\client_build\svn_branch_diff-opt\dist\bin
--release -f fsfs -t update_tests.py
Testing Release configuration on local repository.
Running tests in update_tests.py [1/1]SVN_DBG_STACKTRACES_TO_STDERR set
.
]]]
it continues until test nr 58, and then gives the popup.
Hm, I'm confused. I guess I'm going to fire up my debugger and set a
breakpoint or something to see what happens and why ...
Cheers,
--
Johan
Re: EDEADLK in svn_repos_fs_begin_txn_for_commit2
On 2/1/11 11:53 AM, Blair Zajac wrote: On 1/26/11 5:24 PM, Blair Zajac wrote: On 01/26/2011 11:39 AM, Blair Zajac wrote: On 01/26/2011 11:15 AM, Philip Martin wrote: Blair Zajac writes: I'm now thinking of putting the retry in svn_io_file_lock2() instead of handling a deadlock in libsvn_fs_fs itself. It shouldn't hurt any other use cases and be a general, defensive code. Should retry be conditional on a threaded build? Can this problem even occur in a non-threaded build? Good point. I think with svn's code, no, it wouldn't happen in a non-threaded build because the process does nothing else when it has the lock, it'll only ever have one lock out at a time. I was also going to have the code only retry if EDEADLK is defined, so it wouldn't retry on win32. The #ifdef EDEADLK could also include defined(APR_HAS_THREADS). Fixed in r1063870 and r1063946. Will nominate for backport to 1.6.x. We've now seen the same deadlock on the "db/write-lock" file during a commit, so putting the retry in svn_io_file_lock2() was the ...was the right move. Blair
Re: EDEADLK in svn_repos_fs_begin_txn_for_commit2
On 1/26/11 5:24 PM, Blair Zajac wrote: On 01/26/2011 11:39 AM, Blair Zajac wrote: On 01/26/2011 11:15 AM, Philip Martin wrote: Blair Zajac writes: I'm now thinking of putting the retry in svn_io_file_lock2() instead of handling a deadlock in libsvn_fs_fs itself. It shouldn't hurt any other use cases and be a general, defensive code. Should retry be conditional on a threaded build? Can this problem even occur in a non-threaded build? Good point. I think with svn's code, no, it wouldn't happen in a non-threaded build because the process does nothing else when it has the lock, it'll only ever have one lock out at a time. I was also going to have the code only retry if EDEADLK is defined, so it wouldn't retry on win32. The #ifdef EDEADLK could also include defined(APR_HAS_THREADS). Fixed in r1063870 and r1063946. Will nominate for backport to 1.6.x. We've now seen the same deadlock on the "db/write-lock" file during a commit, so putting the retry in svn_io_file_lock2() was the The only question left if we should retry on an EINTR? It appears that nobody has seen this in practice, because we have have coded for it. Blair
Re: svn commit: r1066143 - /subversion/trunk/subversion/libsvn_subr/target.c
On 02/01/2011 02:13 PM, s...@apache.org wrote: > Author: stsp > Date: Tue Feb 1 19:13:24 2011 > New Revision: 1066143 > > URL: http://svn.apache.org/viewvc?rev=1066143&view=rev > Log: > Follow-up to r1066087: > * subversion/libsvn_subr/target.c > (svn_path_condense_targets): Initialise *pcommon in case the first >target was a URL. Doh! Thanks, Stefan! -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r1066087 - in /subversion/trunk/subversion/libsvn_subr: dirent_uri.c target.c
On Tue, Feb 01, 2011 at 08:14:13PM +0100, Stefan Sperling wrote: > And here, we use it: > > > +*pcommon = svn_uri_get_longest_ancestor(*pcommon, absolute, > > pool); > > See > http://ci.apache.org/builders/svn-slik-w2k3-x64-ra/builds/1628/steps/Test%20fsfs%2Bserf/logs/faillog Actually, that link doesn't provide useful information. Here's how it failed: 0x000203697195 in svn_uri_get_longest_ancestor ( uri1=0x2012d3c48 "/home/stsp/svn/svn-trunk/subversion/tests/libsvn_subr/z/A/file", uri2=0x20313a060 "http://host/A/C/D";, pool=0x20c597028) at subversion/libsvn_subr/dirent_uri.c:1410 1410 assert(svn_uri_is_canonical(uri1, NULL)); > Now we could crash. But in the failing test *pcommon happens to > point to some string from a previous invocation of this function. > We ended up using that value, which happened to be a dirent, > causing the assertion failure. Fixed in r1066143. > > > + else > > +*pcommon = svn_dirent_get_longest_ancestor(*pcommon, absolute, > > + pool); > > +}
Re: svn commit: r1066087 - in /subversion/trunk/subversion/libsvn_subr: dirent_uri.c target.c
On Tue, Feb 01, 2011 at 04:42:10PM -, cmpil...@apache.org wrote:
> Author: cmpilato
> Date: Tue Feb 1 16:42:10 2011
> New Revision: 1066087
>
> URL: http://svn.apache.org/viewvc?rev=1066087&view=rev
> Log:
> Upgrade some uses of deprecated path functions to the new
> dirent/uri/etc. interfaces.
>
> * subversion/libsvn_subr/dirent_uri.c
> (svn_uri_get_longest_ancestor)
> * subversion/libsvn_subr/target.c
> (svn_path_condense_targets, svn_path_remove_redundancies)
> @@ -58,9 +60,12 @@ svn_path_condense_targets(const char **p
> }
Before this change, *pcommon was always initialised:
>/* Get the absolute path of the first target. */
> - SVN_ERR(svn_path_get_absolute(pcommon,
> -APR_ARRAY_IDX(targets, 0, const char *),
> -pool));
> + first_target = APR_ARRAY_IDX(targets, 0, const char *);
> + first_target_is_url = svn_path_is_url(first_target);
> + if (first_target_is_url)
> +first_target = apr_pstrdup(pool, first_target);
Now, we don't initialise it anymore if the first target is a URL!
> + else
> +SVN_ERR(svn_dirent_get_absolute(pcommon, first_target, pool));
>
>/* Early exit when there's only one path to work on. */
>if (targets->nelts == 1)
> @@ -88,9 +93,31 @@ svn_path_condense_targets(const char **p
> {
>const char *rel = APR_ARRAY_IDX(targets, i, const char *);
>const char *absolute;
> - SVN_ERR(svn_path_get_absolute(&absolute, rel, pool));
> + svn_boolean_t is_url = svn_path_is_url(rel);
> +
> + if (is_url)
> +absolute = apr_pstrdup(pool, rel); /* ### TODO: avoid pool dup? */
> + else
> +SVN_ERR(svn_dirent_get_absolute(&absolute, rel, pool));
> +
>APR_ARRAY_PUSH(abs_targets, const char *) = absolute;
> - *pcommon = svn_path_get_longest_ancestor(*pcommon, absolute, pool);
> +
> + /* If we've not already determined that there's no common
> + parent, then continue trying to do so. */
> + if (*pcommon && **pcommon)
> +{
> + /* If the is-url-ness of this target doesn't match that of
> + the first target, our search for a common ancestor can
> + end right here. Otherwise, use the appropriate
> + get-longest-ancestor function per the path type. */
> + if (is_url != first_target_is_url)
> +*pcommon = "";
> + else if (first_target_is_url)
And here, we use it:
> +*pcommon = svn_uri_get_longest_ancestor(*pcommon, absolute,
> pool);
See
http://ci.apache.org/builders/svn-slik-w2k3-x64-ra/builds/1628/steps/Test%20fsfs%2Bserf/logs/faillog
Now we could crash. But in the failing test *pcommon happens to
point to some string from a previous invocation of this function.
We ended up using that value, which happened to be a dirent,
causing the assertion failure. Fixed in r1066143.
> + else
> +*pcommon = svn_dirent_get_longest_ancestor(*pcommon, absolute,
> + pool);
> +}
Re: svn commit: r1064905 - in /subversion/branches/ignore-mergeinfo-log/subversion: libsvn_ra_svn/client.c svnserve/serve.c
Done in r1066137. -Hyrum On Fri, Jan 28, 2011 at 8:17 PM, Daniel Shahaf wrote: > What about libsvn_ra_svn/protocol ? > > hwri...@apache.org wrote on Fri, Jan 28, 2011 at 21:59:14 -: >> Author: hwright >> Date: Fri Jan 28 21:59:14 2011 >> New Revision: 1064905 >> >> URL: http://svn.apache.org/viewvc?rev=1064905&view=rev >> Log: >> On the ignore-mergeinfo-log branch: >> Send the list of ignored prop mods from client to server over the SVN RA >> protocol. >> >> * subversion/libsvn_ra_svn/client.c >> (ra_svn_log): Put the ignored_prop_mods into the server-bound tuple. >> >> * subversion/svnserve/serve.c >> (log_cmd): Parse the incoming ignored_prop_mods. >> >> Modified: >> >> subversion/branches/ignore-mergeinfo-log/subversion/libsvn_ra_svn/client.c >> subversion/branches/ignore-mergeinfo-log/subversion/svnserve/serve.c >
Re: Deltifying directories on the server
On Tue, Feb 1, 2011 at 10:54 AM, Greg Hudson wrote: > On Tue, 2011-02-01 at 10:29 -0500, C. Michael Pilato wrote: >> I can only really speak for the BDB side of things, but... "what he said". > > I'll elaborate a little bit. API issues aside, we're used to putting > artifacts from different versions in different places. More so in FSFS, > where it was baked into the initial architecture, but also in BDB for > the most part. > > The most efficient storage for large directories which frequently change > by small deltas would be some kind of multi-rooted B-tree. To do that > efficiently (that is, without scattering each tiny change into a > separate disk block, requiring lots and lots of opens/seeks/reads), > you'd want to put artifacts from different versions of a directory all > in the same place. You might be able to arrange it so that modifying a > directory is an append-only operation, avoiding the need for a lot of > copying, but you'd still want a place to append to for each directory, > which isn't how FSFS or BDB works. > > So, I'm not sure we can ever have efficient handling of this use case > without designing a completely new back end--which wouldn't be a > terrible idea if someone's up to it. There's been a bit of talk here and there about a new back end, but nothing concrete just yet. Thanks all for the insight. -Hyrum
Re: Deltifying directories on the server
On Tue, 2011-02-01 at 10:29 -0500, C. Michael Pilato wrote: > I can only really speak for the BDB side of things, but... "what he said". I'll elaborate a little bit. API issues aside, we're used to putting artifacts from different versions in different places. More so in FSFS, where it was baked into the initial architecture, but also in BDB for the most part. The most efficient storage for large directories which frequently change by small deltas would be some kind of multi-rooted B-tree. To do that efficiently (that is, without scattering each tiny change into a separate disk block, requiring lots and lots of opens/seeks/reads), you'd want to put artifacts from different versions of a directory all in the same place. You might be able to arrange it so that modifying a directory is an append-only operation, avoiding the need for a lot of copying, but you'd still want a place to append to for each directory, which isn't how FSFS or BDB works. So, I'm not sure we can ever have efficient handling of this use case without designing a completely new back end--which wouldn't be a terrible idea if someone's up to it.
Re: Deltifying directories on the server
On 02/01/2011 04:43 AM, Branko Čibej wrote: > You do know that "diff" and "delta" are two different beasts, and that > the diff optimizations have no effect on deltas? :) > > The problem with directory deltification lies in the length of the delta > chain and the frequency of directory lookup compared to file access. The > sad fact is that our directory storage (/and/ our API) are woefully > unsuited to their tasks. The way they're stored now (in both BDB and > FSFS back-ends) requires the whole directory to be read into memory and > hashed in order to find a single entry, and you have to do this for each > level of directories when resolving a path. It doesn't help that most of > the APIs are strictly path based, e.g., editor drives will do the lookup > any number of times. > > The whole concept of directory storage needs to be changed. The easiest > way would be to store directories as B-trees, however, that doesn't play > nice with versioning. On the other hand, directory structure is well > known and there's no reason to use a generic delta algorithm to store > them. I could probably come up with a better storage schema for > directories in a couple weeks, but I don't have the time to implement > such a thing. > > -- Brane I can only really speak for the BDB side of things, but... "what he said". -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
RE: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)
> -Original Message-
> From: Johan Corveleyn [mailto:jcor...@gmail.com]
> Sent: dinsdag 1 februari 2011 13:28
> To: Daniel Shahaf
> Cc: Subversion Development
> Subject: Re: Assertion failure during update_tests.py 58 (XFAIL: update
> a nonexistent child of a copied dir)
>
> On Mon, Jan 24, 2011 at 3:21 PM, Daniel Shahaf
> wrote:
> > Johan Corveleyn wrote on Mon, Jan 24, 2011 at 02:42:11 +0100:
> >> Hi,
> >>
> >> Already for some time now, update_tests.py 58 (XFAIL: update a
> >> nonexistent child of a copied dir) crashes on my machine:
> >>
> >> svn: In file '..\..\..\subversion\libsvn_wc\update_editor.c'
> line
> >> 4877: assertion failed (repos_root != NULL && repos_uuid != NULL)
> >>
> >> I understand that this test is XFAIL, that this isn't addressed yet,
> >> but is it supposed to fail an assert?
> >>
> >> On my system (Win XP) this causes an ugly popup to appear (which I
> >> need to click away to continue), each time I run the full test suite
> >> ("This application has requested the Runtime to terminate it in an
> >> unusual way...")
> >>
> >> Relevant excerpt from tests.log in attachment (this was with
> trunk@1062600).
> >>
> > It certainly isn't supposed to force all test runs to be interactive
> :-(
> >
> > Have you tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py?
>
> Almost forgot about this one, until I ran into it again yesterday
> evening.
>
> So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
> (put it in comment, ran "nmake config" and rebuilt everything), then
> ran update_tests.py again: same result. It still crashes, and shows
> the ugly blocking popup.
>
> Anyone else who recently built trunk on Windows seeing this, when
> running update_tests.py?
The 'SVN_DBG_STACKTRACES_TO_STDERR' environment option that is set in
subversion/tests/cmdline/svntest/main.py should stop the popup dialogs while
running the tests. (It moves the output to stderr to allow logging them on
the Windows buildbots, instead of requiring interactive resolving).
Bert
Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)
On Mon, Jan 24, 2011 at 3:21 PM, Daniel Shahaf wrote:
> Johan Corveleyn wrote on Mon, Jan 24, 2011 at 02:42:11 +0100:
>> Hi,
>>
>> Already for some time now, update_tests.py 58 (XFAIL: update a
>> nonexistent child of a copied dir) crashes on my machine:
>>
>> svn: In file '..\..\..\subversion\libsvn_wc\update_editor.c' line
>> 4877: assertion failed (repos_root != NULL && repos_uuid != NULL)
>>
>> I understand that this test is XFAIL, that this isn't addressed yet,
>> but is it supposed to fail an assert?
>>
>> On my system (Win XP) this causes an ugly popup to appear (which I
>> need to click away to continue), each time I run the full test suite
>> ("This application has requested the Runtime to terminate it in an
>> unusual way...")
>>
>> Relevant excerpt from tests.log in attachment (this was with trunk@1062600).
>>
> It certainly isn't supposed to force all test runs to be interactive :-(
>
> Have you tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py?
Almost forgot about this one, until I ran into it again yesterday evening.
So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
(put it in comment, ran "nmake config" and rebuilt everything), then
ran update_tests.py again: same result. It still crashes, and shows
the ugly blocking popup.
Anyone else who recently built trunk on Windows seeing this, when
running update_tests.py?
--
Johan
RE: [PATCH] New XFail tests for issue 3609
> -Original Message-
> From: Noorul Islam K M [mailto:noo...@collab.net]
> Sent: dinsdag 1 februari 2011 12:41
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: [PATCH] New XFail tests for issue 3609
>
> "Bert Huijben" writes:
>
> >> -Original Message-
> >> From: Noorul Islam K M [mailto:noo...@collab.net]
> >> Sent: dinsdag 1 februari 2011 5:58
> >> To: dev@subversion.apache.org
> >> Subject: Re: [PATCH] New XFail tests for issue 3609
> >>
> >> Noorul Islam K M writes:
> >>
> >> > Log
> >> >
> >> > [[[
> >> >
> >> > New XFail tests for issue 3609.
> >> >
> >> > * subversion/tests/cmdline/mergeinfo_tests.py
> >> > (mergeinfo_url_special_characters, test_list),
> >> > subversion/tests/cmdline/prop_tests.py
> >> > (props_url_special_characters, test_list),
> >> > subversion/tests/cmdline/merge_tests.py
> >> > (merge_url_special_characters, test_list),
> >> > subversion/tests/cmdline/log_tests.py
> >> > (log_url_special_characters, test_list),
> >> > subversion/tests/cmdline/copy_tests.py
> >> > (copy_url_special_characters, test_list),
> >> > subversion/tests/cmdline/blame_tests.py
> >> > (blame_url_special_characters, test_list):
> >> > New XFail tests.
> >> >
> >> > Patch by: Noorul Islam K M
> >> > ]]]
> >> >
> >> > Index: subversion/tests/cmdline/mergeinfo_tests.py
> >> >
> ===
> >> > --- subversion/tests/cmdline/mergeinfo_tests.py (revision 1062140)
> >> > +++ subversion/tests/cmdline/mergeinfo_tests.py (working copy)
> >> > @@ -479,6 +479,18 @@
> >> > adjust_error_for_server_version(''),
> >> > ['4', '5'], A_path, A_COPY_path + '@PREV', '--show-revs',
> >> 'eligible')
> >> >
> >> > +def mergeinfo_url_special_characters(sbox):
> >> > + """special characters in svn mergeinfo URL"""
> >> > +
> >> > + sbox.build()
> >> > + wc_dir = sbox.wc_dir
> >> > + special_url = sbox.repo_url + '/%2E'
> >
> > With an url like http://server/svn/repos
> > and %2E = '.'
> >
> > This would create an url as 'http://server/svn/repos/%2E'
> >
> > Following our canonicalization rules this would be a non-canonical
> > equivalent to http://server/svn/repos (via http://server/svn/repos/.)
> >
> > I just added a few tests for this specific scenarios in
> > subversion/tests/libsvn_subr/dirent_uri_tests.c, which showed that
> > svn_uri_canonicalize() doesn't handle this case correctly.
> > (svn_uri_canonicalize() returned "http://server/svn/repos/.";, which
> is not
> > canonical by itself)
> >
> > I committed a fix in r 1066006, so you might need a different special
> > character to trigger your issue. (Or maybe this just fixed the
> issue?)
>
> Do you mean this test cases will pass with you fix? If so, is it not
> okay to add these test cases? May be not as XFail?
I haven't tested your tests.
The comments in your tests say you are testing 'special characters', while
the test really failed in the canonicalization of '.' which is handled
unlike any other 'special' character.
The tests in dirent_uri_tests.c explicitly test this behavior now, so I
don't think you need this number of commandline tests for the '.'
canonicalization problems any more. (The test suite is slow enough already
and these few tests don't really improve the coverage now).
But the tests might still be valid for other special characters like 'é',
'+' and '%'.
Bert
Re: [PATCH] New XFail tests for issue 3609
"Bert Huijben" writes:
>> -Original Message-
>> From: Noorul Islam K M [mailto:noo...@collab.net]
>> Sent: dinsdag 1 februari 2011 5:58
>> To: dev@subversion.apache.org
>> Subject: Re: [PATCH] New XFail tests for issue 3609
>>
>> Noorul Islam K M writes:
>>
>> > Log
>> >
>> > [[[
>> >
>> > New XFail tests for issue 3609.
>> >
>> > * subversion/tests/cmdline/mergeinfo_tests.py
>> > (mergeinfo_url_special_characters, test_list),
>> > subversion/tests/cmdline/prop_tests.py
>> > (props_url_special_characters, test_list),
>> > subversion/tests/cmdline/merge_tests.py
>> > (merge_url_special_characters, test_list),
>> > subversion/tests/cmdline/log_tests.py
>> > (log_url_special_characters, test_list),
>> > subversion/tests/cmdline/copy_tests.py
>> > (copy_url_special_characters, test_list),
>> > subversion/tests/cmdline/blame_tests.py
>> > (blame_url_special_characters, test_list):
>> > New XFail tests.
>> >
>> > Patch by: Noorul Islam K M
>> > ]]]
>> >
>> > Index: subversion/tests/cmdline/mergeinfo_tests.py
>> > ===
>> > --- subversion/tests/cmdline/mergeinfo_tests.py(revision 1062140)
>> > +++ subversion/tests/cmdline/mergeinfo_tests.py(working copy)
>> > @@ -479,6 +479,18 @@
>> > adjust_error_for_server_version(''),
>> > ['4', '5'], A_path, A_COPY_path + '@PREV', '--show-revs',
>> 'eligible')
>> >
>> > +def mergeinfo_url_special_characters(sbox):
>> > + """special characters in svn mergeinfo URL"""
>> > +
>> > + sbox.build()
>> > + wc_dir = sbox.wc_dir
>> > + special_url = sbox.repo_url + '/%2E'
>
> With an url like http://server/svn/repos
> and %2E = '.'
>
> This would create an url as 'http://server/svn/repos/%2E'
>
> Following our canonicalization rules this would be a non-canonical
> equivalent to http://server/svn/repos (via http://server/svn/repos/.)
>
> I just added a few tests for this specific scenarios in
> subversion/tests/libsvn_subr/dirent_uri_tests.c, which showed that
> svn_uri_canonicalize() doesn't handle this case correctly.
> (svn_uri_canonicalize() returned "http://server/svn/repos/.";, which is not
> canonical by itself)
>
> I committed a fix in r 1066006, so you might need a different special
> character to trigger your issue. (Or maybe this just fixed the issue?)
Do you mean this test cases will pass with you fix? If so, is it not
okay to add these test cases? May be not as XFail?
Thanks and Regards
Noorul
RE: [PATCH] New XFail tests for issue 3609
> -Original Message-
> From: Noorul Islam K M [mailto:noo...@collab.net]
> Sent: dinsdag 1 februari 2011 5:58
> To: dev@subversion.apache.org
> Subject: Re: [PATCH] New XFail tests for issue 3609
>
> Noorul Islam K M writes:
>
> > Log
> >
> > [[[
> >
> > New XFail tests for issue 3609.
> >
> > * subversion/tests/cmdline/mergeinfo_tests.py
> > (mergeinfo_url_special_characters, test_list),
> > subversion/tests/cmdline/prop_tests.py
> > (props_url_special_characters, test_list),
> > subversion/tests/cmdline/merge_tests.py
> > (merge_url_special_characters, test_list),
> > subversion/tests/cmdline/log_tests.py
> > (log_url_special_characters, test_list),
> > subversion/tests/cmdline/copy_tests.py
> > (copy_url_special_characters, test_list),
> > subversion/tests/cmdline/blame_tests.py
> > (blame_url_special_characters, test_list):
> > New XFail tests.
> >
> > Patch by: Noorul Islam K M
> > ]]]
> >
> > Index: subversion/tests/cmdline/mergeinfo_tests.py
> > ===
> > --- subversion/tests/cmdline/mergeinfo_tests.py (revision 1062140)
> > +++ subversion/tests/cmdline/mergeinfo_tests.py (working copy)
> > @@ -479,6 +479,18 @@
> > adjust_error_for_server_version(''),
> > ['4', '5'], A_path, A_COPY_path + '@PREV', '--show-revs',
> 'eligible')
> >
> > +def mergeinfo_url_special_characters(sbox):
> > + """special characters in svn mergeinfo URL"""
> > +
> > + sbox.build()
> > + wc_dir = sbox.wc_dir
> > + special_url = sbox.repo_url + '/%2E'
With an url like http://server/svn/repos
and %2E = '.'
This would create an url as 'http://server/svn/repos/%2E'
Following our canonicalization rules this would be a non-canonical
equivalent to http://server/svn/repos (via http://server/svn/repos/.)
I just added a few tests for this specific scenarios in
subversion/tests/libsvn_subr/dirent_uri_tests.c, which showed that
svn_uri_canonicalize() doesn't handle this case correctly.
(svn_uri_canonicalize() returned "http://server/svn/repos/.";, which is not
canonical by itself)
I committed a fix in r 1066006, so you might need a different special
character to trigger your issue. (Or maybe this just fixed the issue?)
Bert
Re: Deltifying directories on the server
You do know that "diff" and "delta" are two different beasts, and that the diff optimizations have no effect on deltas? :) The problem with directory deltification lies in the length of the delta chain and the frequency of directory lookup compared to file access. The sad fact is that our directory storage (/and/ our API) are woefully unsuited to their tasks. The way they're stored now (in both BDB and FSFS back-ends) requires the whole directory to be read into memory and hashed in order to find a single entry, and you have to do this for each level of directories when resolving a path. It doesn't help that most of the APIs are strictly path based, e.g., editor drives will do the lookup any number of times. The whole concept of directory storage needs to be changed. The easiest way would be to store directories as B-trees, however, that doesn't play nice with versioning. On the other hand, directory structure is well known and there's no reason to use a generic delta algorithm to store them. I could probably come up with a better storage schema for directories in a couple weeks, but I don't have the time to implement such a thing. -- Brane On 01.02.2011 05:29, Hyrum K Wright wrote: > Philip and I had an interesting conversation with some users this > evening, and I'm just archiving my brain dump here. > > These users have a large repository with a large number of branches in > the /branches directory (~35k). We described the well-known > phenomenon in which directories aren't deltified on commit, and thus > cause the repository to have very large revisions, even when the > actual content changes are fairly small. This is due to bubble up and > having to re-write the entire directory list of the /branches > directory. > > Philip recalled a time several years ago when he enabled directory > deltification, but the performance was awful, and we've never released > it. In our discussion, we mentioned that directory deltification may > be better performing now, especially in light of the imminent merge of > the diff-bytes-optimizations branch. In the case of a bubble-up > directory modification, the prefix and suffix matching would simplify > the problem space, leaving a very small diff. > > The only trouble with the above theory is if directory entry lists are > stored in a hash, and are serialized in an unordered manner, thus > negating any benefits prefix-scanning would provide (and potentially > causing the horrific delta performance in the first place). > > Anyway, that was the kernel of our discussion. I haven't dug around > in the code to determine how much of it is true or not, but if anybody > wants something to do, this might be interesting. > > -Hyrum
