You open an ra session to one canonical url and then get a session to another one.
At that point you can't trust the relative path apis, nor reparent the session to something using based on the other url.
This breaks checkout, diff, or even simple ‘svn ls’ to these url forms.
The biggest problem is that one of the sources is libsvn_wc, which we can’t just change.. But that is one of the sources. Most other sources already canonicalize using svn_uri_canonicalize().
Also note that the applying the ‘localhost’ rule I mentioned as one of the examples already got a -1 in a recent mail for applying in the generic uri canonicalize code.
Mark Phippard and at least one other user reported this regression on dev_at_s.a.o recently.
The proposed change is reverting a 1.8 behavior change with unwanted side effects. Not relaxing a rule.
Making it strict will take quite some work; which as I mentioned includes a wc.db format bump. But this also includes option parsing changes and applying more canonicalizations than standardized.
Sent from Windows Mail
From: Daniel Shahaf
Sent: Sunday, May 26, 2013 9:25 PM
To: Bert Huijben
Bert Huijben wrote on Sun, May 26, 2013 at 20:51:10 +0200:
> > -----Original Message-----
> > From: danielsh_at_apache.org [mailto:danielsh_at_apache.org]
> > Sent: zondag 26 mei 2013 08:49
> > To: commits_at_subversion.apache.org
> > Subject: svn commit: r1486397 - /subversion/branches/1.8.x/STATUS
> > Author: danielsh
> > Date: Sun May 26 06:48:49 2013
> > New Revision: 1486397
> > URL: http://svn.apache.org/r1486397
> > Log:
> > * STATUS: Ask a question about r1485127.
> > Modified:
> > subversion/branches/1.8.x/STATUS
> > Modified: subversion/branches/1.8.x/STATUS
> > URL:
> > http://svn.apache.org/viewvc/subversion/branches/1.8.x/STATUS?rev=1486
> > 397&r1=1486396&r2=1486397&view=diff
> > ==========================================================
> > ====================
> > --- subversion/branches/1.8.x/STATUS (original)
> > +++ subversion/branches/1.8.x/STATUS Sun May 26 06:48:49 2013
> > @@ -104,6 +104,10 @@ Candidate changes:
> > Restores functionality that was lost in 1.8 compared to 1.7.
> > Votes:
> > +1: rhuijben (for 1.8.1 or soak restart)
> > + +0: danielsh (looks right, but all this does is cause sess->repos_url
> > + to be set to a non-canonical value; I wonder if the right
> > + fix is to add a canonicalize() call in front of a strcmp()
> > + in libsvn_client)
> I wish it were that simple.
> The usage of file://localhost/some/location to a repository on
> /some/location has been hardcoded in ra_local since before 1.0, but is
> not part of our canonicalization rules in svn_uri_canonicalize().
> The 'simple fix' to change the canonicalization rules by including
> this rule would require a working copy format bump, as this prefix is
> currently perfectly valid as a repository location.
What is the libsvn_wc problem that setting
svn_ra_session_t.repos_root_url to a different value fixes? (You
haven't said that yet either in the log message or in this thread; you
only said what URLs are involved in it.)
It is still not clear to me why we are fixing a libsvn_wc issue by
changing an implementation detail of libsvn_ra_local. It sounds like
libsvn_wc could invent its own notion of "equal URL" which ignores drives
letters and considers file:/// and file://localhost/ equivalent, with no
changes to libsvn_ra*.
With the new code, will svn_ra_session_t->repos_root_url always be set
to a canonical value? The code before your change would.
Thanks for the additional background.
> And then for this
> example the api works for us, but is not really part of any RFC.
> That 'localhost' refers to the local machine is a convention on many
> platforms, but not really a standard.
> On Windows we also have file:///some/path urls that refers to
> E:\some\path when E: happens to be the current harddrive. And this has
> similar conversion problems.
> I think that for 1.8 we have to keep the same one to many relation
> between local dirents and repository urls, as in 1.0-1.7 :(
> For 1.9+ we might be able to apply new canonicalization rules via
> a proper upgrade, but doing this now would just delay 1.8, at no real
> gain for our users.
> > * r1485447, r1485449
> > Make 'SQLITE_VERSION=18.104.22.168 ./get-deps.sh' work.
Received on 2013-05-26 22:07:41 CEST