On Mon, Apr 19, 2010 at 12:14, Daniel Näslund <dan...@longitudo.com> wrote: > Hi! > > I have a patch that passes make check. But a test suite can't test > everything (although the status code is probably one of the best covered > due to it's part in almost every cmdline-test). Posting only the changes > in libsvn_wc/status.c > > What I'm wondering about: > > * I'm using SVN_INVALID_REVNUM() for determining if read_info() gives us > a valid revision. If it does I assume that we don't have to check for > base_shadowed, addition or deletion. If the path used as IN parameter > to read_info() is deleted, we will get a valid revision. Correct?
If you pass &revision, then it will always be assigned (unless read_info gets an error). If you have any kind of WORKING node (meaning an add/delete/move/copy), then it will be assigned SVN_INVALID_REVNUM. If there are no changes (you're looking at just a BASE node), then you will *usually* have a valid revision. If the BASE node is incomplete, excluded, not-present, or absent, then you can't truly rely on its revision. (and we "should" probably set it to SVN_INVALID_REVNUM for those four cases; Philip?) > * svn_wc__db_scan_deletion() checks work_del_abspath. That works for all > the cases in the test suite. I assumed that a plain delete would set > the work_del_abspath to the root but this part of the doc comment of > scan_deletion() says something else. Am I misinterpreting this (e.g. > it's a part of the earlier paragraphs and has more operations done) or > can it really be that a plain delete does not set the work_del_abspath? > > [[[ > If B/W/D does not exist in the WORKING tree (we're only talking about a > deletion of nodes of the BASE tree), then deleting B/W/D would have > marked the subtree for deletion. BASE_DEL_ABSPATH will refer to B/W/D, > BASE_REPLACED will be FALSE, MOVED_TO_ABSPATH will be NULL, and > WORK_DEL_ABSPATH will be NULL. > ]]] A plain delete should not set WORK_DEL_ABSPATH (I'd call that a bug). Only a delete of a WORKING node should set that (e.g copy/move followed by a deletion of a child). > * For all cases where read_info() says we have an addition (and by that > read_info() means copied-here and moved-here too) I've set the > revision to -1. Correct? Until committed, no revision exists is my > point! read_info() will return SVN_INVALID_REVNUM already, for those nodes. Cheers, -g