And here's the patch attachment. - Julian
----- Original Message ----- > From: Julian Foad <julianf...@btopenworld.com> > To: SubversionDevelopment <dev@subversion.apache.org> > Cc: > Sent: Tuesday, 17 April 2012, 11:13 > Subject: [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 >
wc_node_get_base.patch
Description: Binary data
