You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2013/05/26 20:51:10 UTC
RE: svn commit: r1486397 - /subversion/branches/1.8.x/STATUS
> -----Original Message-----
> From: danielsh@apache.org [mailto:danielsh@apache.org]
> Sent: zondag 26 mei 2013 08:49
> To: commits@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. 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.
Bert
>
> * r1485447, r1485449
> Make 'SQLITE_VERSION=3.7.15.1 ./get-deps.sh' work.
>
Re: svn commit: r1486397 - /subversion/branches/1.8.x/STATUS
Posted by Daniel Shahaf <da...@elego.de>.
Bert Huijben wrote on Sun, May 26, 2013 at 19:57:42 +0000:
> 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.
>
In other words, your problem is that you can't reliably figure out what
the "repos root URL" portion of the URL you passed to svn_ra_open() is,
right?
If so I'd be +1: ensuring the session URL is a prefix of the input URL
is indeed the easiest way to fix this, and it doesn't constrain us going
forward.
>
> 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@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.
>
I don't see why that is the case, but never mind. Understanding the
problem in terms of svn_ra_open4()'s contract is good enough for me at
the moment.
Thanks,
Daniel
>
> Bert
>
>
>
> Sent from Windows Mail
>
>
>
> From: Daniel Shahaf
> Sent: Sunday, May 26, 2013 9:25 PM
> To: Bert Huijben
> Cc: dev@subversion.apache.org
>
> Bert Huijben wrote on Sun, May 26, 2013 at 20:51:10 +0200:
> >
> >
> > > -----Original Message-----
> > > From: danielsh@apache.org [mailto:danielsh@apache.org]
> > > Sent: zondag 26 mei 2013 08:49
> > > To: commits@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.
>
> Daniel
>
> > 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.
> >
> > Bert
> >
> >
> >
> >
> > >
> > > * r1485447, r1485449
> > > Make 'SQLITE_VERSION=3.7.15.1 ./get-deps.sh' work.
> > >
> >
> >
Re: svn commit: r1486397 - /subversion/branches/1.8.x/STATUS
Posted by Bert Huijben <be...@qqmail.nl>.
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@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.
Bert
Sent from Windows Mail
From: Daniel Shahaf
Sent: Sunday, May 26, 2013 9:25 PM
To: Bert Huijben
Cc: dev@subversion.apache.org
Bert Huijben wrote on Sun, May 26, 2013 at 20:51:10 +0200:
>
>
> > -----Original Message-----
> > From: danielsh@apache.org [mailto:danielsh@apache.org]
> > Sent: zondag 26 mei 2013 08:49
> > To: commits@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.
Daniel
> 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.
>
> Bert
>
>
>
>
> >
> > * r1485447, r1485449
> > Make 'SQLITE_VERSION=3.7.15.1 ./get-deps.sh' work.
> >
>
>
Re: svn commit: r1486397 - /subversion/branches/1.8.x/STATUS
Posted by Daniel Shahaf <da...@elego.de>.
Bert Huijben wrote on Sun, May 26, 2013 at 20:51:10 +0200:
>
>
> > -----Original Message-----
> > From: danielsh@apache.org [mailto:danielsh@apache.org]
> > Sent: zondag 26 mei 2013 08:49
> > To: commits@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.
Daniel
> 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.
>
> Bert
>
>
>
>
> >
> > * r1485447, r1485449
> > Make 'SQLITE_VERSION=3.7.15.1 ./get-deps.sh' work.
> >
>
>