RE: svn commit: r1651980 - in /subversion/trunk/subversion: libsvn_wc/wc_db.c tests/libsvn_wc/op-depth-test.c

> -----Original Message-----
> From: Branko Čibej [mailto:brane_at_wandisco.com]
> Sent: donderdag 15 januari 2015 10:35
> To: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1651980 - in /subversion/trunk/subversion:
> libsvn_wc/wc_db.c tests/libsvn_wc/op-depth-test.c
>
> On 15.01.2015 09:51, rhuijben_at_apache.org wrote:
> > Author: rhuijben
> > Date: Thu Jan 15 08:51:47 2015
> > New Revision: 1651980
> >
> > URL: http://svn.apache.org/r1651980
> > Log:
> > Resolve the issue identified in r1651963, by properly calculating the depth
> > of the location where the moved to information should be stored after
> another
> > move.
> >
> > * subversion/libsvn_wc/wc_db.c
> > (delete_update_movedto): Make this assertion maintainer only, like other
> > similar checks in wc_db.c.
> > (delete_node): Fix depth calculation.
>
> If the depth calculation is now correct and the assertion doesn't get
> triggered, the SVN_DEBUG is doubly useless because it masks bugs. If the
> SVN_DEBUG had been there before, we'd never have got the original bug
> report in the first place. Either leave the assertion in or rip it out.

This is a common pattern in the move handling, which really isn't stable yet (especially during updates). We like the information about these missed matches during testing, but the move lookup code verifies whether there is a match ing src and destination before handling a node as moved.

Eventually somebody would have reported that he moved a file, but didn't see it as a move in status (or see it handled as move during update). Yes, that delays the report, but as long as we don't have server side move handling nothing really breaks when move information gets lost.

The 'assert(SOMETHING);' statements in the code are assumed to be left out in release builds, while SVN_ERR_ASSERT() statements are assumed to be left in.
(That is why we didn't get reports from TortoiseSVN users, nor mails from Stefan Kung on abort()s in release code)

SVN_ERR_ASSERT() can either abort() or return an error depending on what the api user wants to happen.

The problem here is that this specific user got an assert() triggering, so I switched it to the same code as at least 3 or 4 other functions have for similar cases.

        Bert
Received on 2015-01-15 11:09:36 CET