I'm auditing the uses of scan_deletion() and its semi-public wrapper
svn_wc__db_scan_deletion(): what do its callers really want? My goals:
to be able to understand the callers, and then adapt them to op-depth
semantics and also simplify them. It seems to me there's far too much
obfuscation going on in these callers at present, with lots of
near-but-not-quite-duplication.
Here are all the uses and what I can understand about them:
1. adm_crawler.c:find_base_rev()
-> work_del_abspath
This code path is unused in a test suite run.
2. entries.c:get_base_info_for_deleted() (first occurrence)
-> work_del_abspath
Use of result:
read_info(dirname(work_del_abspath))+scan_add -> repo+relpath
Ultimate need:
Copyfrom repo+relpath of the deleted node.
(The repo+relpath that this path belonged to before it was
deleted; if this deletion is inside a move/copy, then the repo
location within the copy source.)
3. entries.c:get_base_info_for_deleted() (second occurrence)
-> base_replaced, work_del_abspath
Use of result:
base_replaced,
read_info(dirname(work_del_abspath))+scan_add -> status
Ultimate need:
Status indications for constructing an entry_t.
4. node.c:svn_wc__node_get_working_rev_info()
-> base_del_abspath, work_del_abspath
This function is only used for back-compat in status reporting.
5. node.c:svn_wc__node_get_commit_base_rev()
-> work_del_abspath
Use of result:
read_info(dirname(work_del_abspath))+scan_add -> original_rev
assert(status is some kind of 'added')
Ultimate need:
The "commit base rev" of the deleted node, which probably
really means the repository revision of the parent dir of the
deletion root. See doc string - "should go away".
6. node.c:svn_wc__internal_node_get_schedule()
-> work_del_abspath
Use of result:
work_del_abspath == NULL?
Ultimate need:
To know whether this deletion is inside a copy/move.
7. wc_db.c:get_info_for_copy()
-> work_del_relpath
Use of result:
scan_add(dirname(work_del_relpath)) -> original_repos_*, op_root
Ultimate aim:
Copyfrom repos+relpath+rev of the deleted node.
### Relpath of this node, or of root of the copy?
8. wc_db.c:svn_wc__db_global_relocate()
-> work_del_relpath
Use of result:
dirname(work_del_relpath)
Ultimate aim:
Repos+relpath and local path of the parent of the root of the
deletion.
9. db-test.c:test_scan_deletion()
Several calls, each checking all the output parameters.
So what to do with this? Work out what semantics the callers really
want, and provide them through one or more APIs. In each case, return
the wanted data in a single call instead of providing a path. Then
we'll be in a position where we can optimize the queries inside each
such function if we need to.
1. Seems unused, so ignore for now. I'm not confident that it can't be
triggered. I hope this whole private function can be removed and some
(semi-public) API used instead.
2, 5, 7, 8. These are all similar and want to know the repos location of
"where the deleted node would have been". Need to check carefully what
relpath these want: the relpath of the given path, or of the deletion
root, or of the parent of the deletion root. Also the revnum - should
it be the revnum that the node was at before it was deleted, or the
revnum that its parent is at? They can differ in a mixed-rev base or in
a copy of a mixed-rev base.
3. Wants status of parent of deletion root.
4. The function for back-compatibility in "status" reporting. The old,
deprecated version of the public status API should retrieve the
back-compat fields natively, rather than having the client make a
separate call. The newer, revved status API can be non-compatible and
avoid that overhead.
6. "Is this deletion inside a copy/move?" This usage is not a problem
and can continue to use scan_deletion or one of the functions that may
replace it.
9. Tests: amend as appropriate.
The "moved_to_abspath" output is never used. Ditch it?
The "base_replaced" output is only used in case 3. Ditch it from the
main API and use something else in this case?
The "base_del_abspath" output is only used in case 4. Ditch it from the
main API and use something else in this case?
- Julian
Received on 2010-11-16 18:51:49 CET