You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Roman Donchenko <DX...@yandex.ru> on 2009/10/11 20:18:45 UTC
svn_client_status and path relativity
Hello,
While debugging the last Python testsuite failure, I've come upon an issue
in the info implementation.
First of all, as I've already mentioned in IRC, the receiver argument to
svn_client_info3 is of type svn_info_receiver2_t in the function
declaration, but svn_info_receiver_t in the definition. As both types are
identical, it doesn't cause any problems, but I'm listing it here for the
record.
Later on, however, in the aforementioned function we have:
SVN_ERR(receiver(receiver_baton, base_name, info, pool));
Why are we passing it the base name here? The parameter is called
abspath_or_url, presumably it expects to be passed an absolute path or a
URL. 8=]
Moreover, I'm not sure what should be passed there. If we pass the URL in
the repository, then info_receiver_relpath_wrapper will be unable to
transform it into a relative path for svn_client_info2 users. And if we
use the absolute path in the WC, we still won't achieve full
compatibility: previously, "svn info mywc/blah.txt -rHEAD" would return
"blah.txt" as the path, now it would return "mywc/blah.txt". And I'm not
sure which one is a better answer. The latter appeals to me more, but it's
backwards incompatible.
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406436
RE: svn_client_status and path relativity
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: news [mailto:news@ger.gmane.org] On Behalf Of Roman Donchenko
> Sent: maandag 12 oktober 2009 1:10
> To: dev@subversion.tigris.org
> Subject: Re: svn_client_status and path relativity
>
> Bert Huijben <rh...@sharpsvn.net> писал в своём письме Mon, 12 Oct
> 2009
> 02:36:21 +0400:
>
> >> I'm talking about the path that's being passed to the receiver, not
> the
> >> one that info itself uses.
> >>
> >> E.g.:
> >>
> >> > svn info -rBASE src-trunk\COMMITTERS
> >> Path: COMMITTERS <-- shouldn't this be src-trunk\COMMITTERS?
> >> ....
> >>
> >> > bin-trunk\svn info -rBASE testwc\trunk\README.txt
> >> Path: testwc\trunk\README.txt\README.txt <-- the same bug as in the
> >> bindings testsuite, caused by passing base_name to the receiver
> >> ....
> >
> > -r BASE is also translated in the base revision of the path and then
> > send to the server with the url of the working copy path. Only if you
> > pass an unspecified revision (API) or no revision (CLI) you get the
> > wc-path version.
>
> BASE or HEAD is not the point here; point is that info currently
> returns a
> clearly malformed path (when the repo is queried).
Ok.. I think I found your issue.
In my steps I forgot a check that is crucial in this specific case where we allow paths and urls. Before appending the relative prefix, you must check that the path really starts with the absolute prefix.
(No time to check right now, but I think this is what you saw).
I will try to apply a fix to libsvn_client/deprecated.c later today...
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406549
Re: svn_client_status and path relativity
Posted by Roman Donchenko <DX...@yandex.ru>.
Bert Huijben <rh...@sharpsvn.net> писал в своём письме Mon, 12 Oct 2009
02:36:21 +0400:
>> I'm talking about the path that's being passed to the receiver, not the
>> one that info itself uses.
>>
>> E.g.:
>>
>> > svn info -rBASE src-trunk\COMMITTERS
>> Path: COMMITTERS <-- shouldn't this be src-trunk\COMMITTERS?
>> ....
>>
>> > bin-trunk\svn info -rBASE testwc\trunk\README.txt
>> Path: testwc\trunk\README.txt\README.txt <-- the same bug as in the
>> bindings testsuite, caused by passing base_name to the receiver
>> ....
>
> -r BASE is also translated in the base revision of the path and then
> send to the server with the url of the working copy path. Only if you
> pass an unspecified revision (API) or no revision (CLI) you get the
> wc-path version.
BASE or HEAD is not the point here; point is that info currently returns a
clearly malformed path (when the repo is queried).
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406461
RE: svn_client_status and path relativity
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Роман Донченко [mailto:DXDragon@yandex.ru]
> Sent: maandag 12 oktober 2009 0:24
> To: Bert Huijben; dev@subversion.tigris.org
> Subject: Re: svn_client_status and path relativity
>
> Bert Huijben <be...@qqmail.nl> писал в своём письме Mon, 12 Oct 2009
> 01:49:13 +0400:
>
> >> -----Original Message-----
> >> From: news [mailto:news@ger.gmane.org] On Behalf Of Roman Donchenko
> >> Sent: zondag 11 oktober 2009 22:19
> >> To: dev@subversion.tigris.org
> >> Subject: svn_client_status and path relativity
> >>
> >> Hello,
> >>
> >> While debugging the last Python testsuite failure, I've come upon an
> >> issue
> >> in the info implementation.
> >>
> >> First of all, as I've already mentioned in IRC, the receiver
> argument
> >> to
> >> svn_client_info3 is of type svn_info_receiver2_t in the function
> >> declaration, but svn_info_receiver_t in the definition. As both
> types
> >> are
> >> identical, it doesn't cause any problems, but I'm listing it here
> for
> >> the
> >> record.
> >>
> >> Later on, however, in the aforementioned function we have:
> >>
> >> SVN_ERR(receiver(receiver_baton, base_name, info, pool));
> >>
> >> Why are we passing it the base name here? The parameter is called
> >> abspath_or_url, presumably it expects to be passed an absolute path
> or
> >> a
> >> URL. 8=]
> >>
> >> Moreover, I'm not sure what should be passed there. If we pass the
> URL
> >> in
> >> the repository, then info_receiver_relpath_wrapper will be unable to
> >> transform it into a relative path for svn_client_info2 users. And if
> we
> >> use the absolute path in the WC, we still won't achieve full
> >> compatibility: previously, "svn info mywc/blah.txt -rHEAD" would
> return
> >> "blah.txt" as the path, now it would return "mywc/blah.txt". And I'm
> >
> > Second reply..
> >
> > svn info mywc/blah.txt -rHEAD uses the url of mywc/blah.txt at
> revision
> > HEAD
> > and never the working copy path, in any format. That hasn't changed.
>
> I'm talking about the path that's being passed to the receiver, not the
> one that info itself uses.
>
> E.g.:
>
> > svn info -rBASE src-trunk\COMMITTERS
> Path: COMMITTERS <-- shouldn't this be src-trunk\COMMITTERS?
> ....
>
> > bin-trunk\svn info -rBASE testwc\trunk\README.txt
> Path: testwc\trunk\README.txt\README.txt <-- the same bug as in the
> bindings testsuite, caused by passing base_name to the receiver
> ....
-r BASE is also translated in the base revision of the path and then send to the server with the url of the working copy path. Only if you pass an unspecified revision (API) or no revision (CLI) you get the wc-path version.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406453
Re: svn_client_status and path relativity
Posted by Roman Donchenko <DX...@yandex.ru>.
Bert Huijben <be...@qqmail.nl> писал в своём письме Mon, 12 Oct 2009
01:49:13 +0400:
>> -----Original Message-----
>> From: news [mailto:news@ger.gmane.org] On Behalf Of Roman Donchenko
>> Sent: zondag 11 oktober 2009 22:19
>> To: dev@subversion.tigris.org
>> Subject: svn_client_status and path relativity
>>
>> Hello,
>>
>> While debugging the last Python testsuite failure, I've come upon an
>> issue
>> in the info implementation.
>>
>> First of all, as I've already mentioned in IRC, the receiver argument
>> to
>> svn_client_info3 is of type svn_info_receiver2_t in the function
>> declaration, but svn_info_receiver_t in the definition. As both types
>> are
>> identical, it doesn't cause any problems, but I'm listing it here for
>> the
>> record.
>>
>> Later on, however, in the aforementioned function we have:
>>
>> SVN_ERR(receiver(receiver_baton, base_name, info, pool));
>>
>> Why are we passing it the base name here? The parameter is called
>> abspath_or_url, presumably it expects to be passed an absolute path or
>> a
>> URL. 8=]
>>
>> Moreover, I'm not sure what should be passed there. If we pass the URL
>> in
>> the repository, then info_receiver_relpath_wrapper will be unable to
>> transform it into a relative path for svn_client_info2 users. And if we
>> use the absolute path in the WC, we still won't achieve full
>> compatibility: previously, "svn info mywc/blah.txt -rHEAD" would return
>> "blah.txt" as the path, now it would return "mywc/blah.txt". And I'm
>
> Second reply..
>
> svn info mywc/blah.txt -rHEAD uses the url of mywc/blah.txt at revision
> HEAD
> and never the working copy path, in any format. That hasn't changed.
I'm talking about the path that's being passed to the receiver, not the
one that info itself uses.
E.g.:
> svn info -rBASE src-trunk\COMMITTERS
Path: COMMITTERS <-- shouldn't this be src-trunk\COMMITTERS?
....
> bin-trunk\svn info -rBASE testwc\trunk\README.txt
Path: testwc\trunk\README.txt\README.txt <-- the same bug as in the
bindings testsuite, caused by passing base_name to the receiver
....
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406452
RE: svn_client_status and path relativity
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: news [mailto:news@ger.gmane.org] On Behalf Of Roman Donchenko
> Sent: zondag 11 oktober 2009 22:19
> To: dev@subversion.tigris.org
> Subject: svn_client_status and path relativity
>
> Hello,
>
> While debugging the last Python testsuite failure, I've come upon an
> issue
> in the info implementation.
>
> First of all, as I've already mentioned in IRC, the receiver argument
> to
> svn_client_info3 is of type svn_info_receiver2_t in the function
> declaration, but svn_info_receiver_t in the definition. As both types
> are
> identical, it doesn't cause any problems, but I'm listing it here for
> the
> record.
>
> Later on, however, in the aforementioned function we have:
>
> SVN_ERR(receiver(receiver_baton, base_name, info, pool));
>
> Why are we passing it the base name here? The parameter is called
> abspath_or_url, presumably it expects to be passed an absolute path or
> a
> URL. 8=]
>
> Moreover, I'm not sure what should be passed there. If we pass the URL
> in
> the repository, then info_receiver_relpath_wrapper will be unable to
> transform it into a relative path for svn_client_info2 users. And if we
> use the absolute path in the WC, we still won't achieve full
> compatibility: previously, "svn info mywc/blah.txt -rHEAD" would return
> "blah.txt" as the path, now it would return "mywc/blah.txt". And I'm
Second reply..
svn info mywc/blah.txt -rHEAD uses the url of mywc/blah.txt at revision HEAD
and never the working copy path, in any format. That hasn't changed.
The result only changed for the case where no -r or peg revision is
specified.. which specifies the working copy version.
The new version will return the absolute path, and the old version the
relative path. (And the old result is still provided for the old function
name via the wrapper in deprecated.c)
Bert
> not
> sure which one is a better answer. The latter appeals to me more, but
> it's
> backwards incompatible.
>
> Roman.
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageI
> d=2406436
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406447
RE: svn_client_status and path relativity
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@xbc.nu]
> Sent: maandag 12 oktober 2009 2:21
> To: Bert Huijben
> Cc: 'Roman Donchenko'; dev@subversion.tigris.org
> Subject: Re: svn_client_status and path relativity
>
> Bert Huijben wrote:
> > (One major issue is that there are an
> > infinite numbers to describe a path in relative form. Luckily there
> is just
> > one absolute path)
> >
>
> Do we make certain of that? What about symlinks in the path?
That is a completely different layer of abstraction.
There is an infinite number of relative paths that describe an absolute path. (That is what I was trying to describe here, because that is what we changed).
There is also an infinite number of paths that can describe a different target. (symlinks, hardlinks, mounts, ..., ...). But this is handled at the io and working copy level. (In some cases we want to version the symlinks, but in some cases we want to ignore them)
We have a major issue with relative path forms in the old access baton based api.. You can have multiple access batons with a different path describing and caching the same path in different ways. Those issues are mostly resolved by always using absolute path internally.
(E.g. it allows using hashtables on paths).
Handling other things becomes a lot easier if we remove a few other problems that have hardcoded workarounds all through our internal api.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406548
Re: svn_client_status and path relativity
Posted by Branko Cibej <br...@xbc.nu>.
Bert Huijben wrote:
> (One major issue is that there are an
> infinite numbers to describe a path in relative form. Luckily there is just
> one absolute path)
>
Do we make certain of that? What about symlinks in the path?
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406481
RE: svn_client_status and path relativity
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: news [mailto:news@ger.gmane.org] On Behalf Of Roman Donchenko
> Sent: zondag 11 oktober 2009 22:19
> To: dev@subversion.tigris.org
> Subject: svn_client_status and path relativity
>
> Hello,
>
> While debugging the last Python testsuite failure, I've come upon an
> issue
> in the info implementation.
>
> First of all, as I've already mentioned in IRC, the receiver argument
> to
> svn_client_info3 is of type svn_info_receiver2_t in the function
> declaration, but svn_info_receiver_t in the definition. As both types
> are
> identical, it doesn't cause any problems, but I'm listing it here for
> the
> record.
>
> Later on, however, in the aforementioned function we have:
>
> SVN_ERR(receiver(receiver_baton, base_name, info, pool));
>
> Why are we passing it the base name here? The parameter is called
> abspath_or_url, presumably it expects to be passed an absolute path or
> a
> URL. 8=]
>
> Moreover, I'm not sure what should be passed there. If we pass the URL
> in
> the repository, then info_receiver_relpath_wrapper will be unable to
> transform it into a relative path for svn_client_info2 users. And if we
> use the absolute path in the WC, we still won't achieve full
> compatibility: previously, "svn info mywc/blah.txt -rHEAD" would return
> "blah.txt" as the path, now it would return "mywc/blah.txt". And I'm
> not
> sure which one is a better answer. The latter appeals to me more, but
> it's
> backwards incompatible.
The idea is that all new functions will only accept absolute paths for local
dirents. This allows cleaning up a lot of cod in our backend libraries
(read: libsvn_wc) on path handling. (One major issue is that there are an
infinite numbers to describe a path in relative form. Luckily there is just
one absolute path)
If a new function accepts a relative path for its operation and calls
callbacks you can easily transform the expected paths back to their relative
form. (See libsvn_client/deprecated.c and libsvn_wc/deprecated.c for dozens
of examples).
* Take the absolute path of the anchor/target argument...
* Pass this absolute value to the new function, with a wrapped callback.
* Store both the relative path and the absolute path in the baton.
In the callback:
* strip the absolute path from the start of the passed absolute path.
(svn_dirent_skip_ancestor() does this for you).
* Join the original relpath with the rest of the path. This brings you are
back in the relative format callers expect.
If you look at svn_client_info2() in subversion/libsvn_client/deprecated.c
(and its helper function info_receiver_relpath_wrapper()) you can see how
svn_client_info2() does this for its old users.
(It actually has a small optimization for callers that already pass absolute
paths)
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406443