[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

scan_deletion - why?

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Tue, 16 Nov 2010 17:51:05 +0000

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

This is an archived mail posted to the Subversion Dev mailing list.