Re: [RFC] Revision and URL go together

And here's the patch attachment. - Julian ----- Original Message ----- > From: Julian Foad <julianfoad_at_btopenworld.com> > To: SubversionDevelopment <dev_at_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 >