[RFC] Revision and URL go together

Short version:  The proposal is to replace all APIs that get part of a repository location (a URL or relpath or revnum) with APIs that get the whole repository location (URL+revision, or repo_root+revision+relpath).  Rationale: It's painful working with code that fetches separate parts of a repo location via different APIs that are not clearly documented as referring to the same location.

In Subversion we know that a repository node is located by a URL at a given revision, or by a repos_relpath in a given revision in a given repository.

APIs such as

  svn_client_url_from_path2()
  svn_client__path_relative_to_root

  svn_client__get_revision_number() (some uses; others are OK)

  svn_wc__node_get_base_rev()

  svn_wc__node_get_commit_base_rev()

  svn_wc__node_get_repos_relpath()
  svn_wc__node_get_url()
  svn_wc__db_scan_base_repos() -> (relpath, root_url, uuid)

artificially separate the relpath or URL from the revision in which that path exists.  I have been noticing more and more that this causes the coder to concentrate on the fact that it's dealing with a URL, or a relpath, or a revision number, and to lose sight of what object is being referenced: whether it's the pristine copy of the node, or the working/actual version (whose revision number might be 'uncommitted'), or the base version, for example.

Examples of good APIs are:

  Ones using svn_client__pathrev_t.

  svn_wc__node_get_origin()  -> (root_url, uuid, rev, relpath)
  svn_wc__db_read_info()     -> (root_url, uuid, rev, relpath, ...)
  svn_wc__db_base_get_info() -> (root_url, uuid, rev, relpath, ...)

I propose to upgrade private APIs to input/output a complete repository location where it makes sense, and to deprecate the few public APIs (there's only one in my list above) and provide replacements for them.

The part I'm going to need help with is the semantics: "what object is being referenced?"  In many of the existing calls to such functions, it's not clear what the caller wants.  "Give me a revision number that's something to do with this local abspath, please!"

For example, in two places in libsvn_client, we have code like this:

  svn_wc__node_get_repos_info(wc_abspath) -> root_url, uuid
  svn_wc__node_get_base_rev(  wc_abspath) -> rev
  svn_wc__node_get_url(       wc_abspath) -> url

Problem: those three functions have differing semantics.  The first gets the working (not base) node's repository; the second gets the base node's rev, and the third one appears to get the working/actual node's URL (and "if added, the URL it will have").

This would indicate that the calling code is flawed.  What semantics do we want there really?

PROPOSAL

We'll specify a few functions along the lines of svn_wc__node_get_origin(), each requesting the whole repository location of some node that exists in a repository.  For example, for querying a given WC abspath:

  svn_wc__node_get_origin()
    -> repo location of the "pristine" version: the copy-from
        if it's copied/moved here, "none" if it's added or

       deleted or moved-away, else the base.

  svn_wc__node_get_base_location()
    -> repo location of the "base" version: or "none" if there
        is no base (e.g. added/copied/moved but not a replace)

  svn_wc__node_get_actual_location()
    -> repo location of the "actual" version, with relpath being
        the path it will have if committed, and revnum being
        SVN_INVALID_REVNUM if it's an uncommitted modification.
        This of course is a "pseudo-location" rather than a
        concrete location; this is sometimes useful/needed.

For a concrete starting point, the attached patch implements "svn_wc__node_get_base()", and uses it in the two libsvn_client places mentioned above, although it's preobably semantically wrong.  One test fails, merge_tree_conflict_tests.py 15.  As indicated in the patch diff, the stage I've reached is trying to see if the base actually ever differs from the 'origin' at this point in the code.  I'll push this further, but not today.

- Julian
Received on 2012-04-17 12:14:00 CEST