You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2012/03/02 06:33:22 UTC
svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Author: hwright
Date: Fri Mar 2 05:33:22 2012
New Revision: 1296056
URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
Log:
In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom paths
correctly.
Current number of test failures over ra_svn: 357
* subversion/libsvn_client/util.c
(fetch_props_func, fetch_kind_func, fetch_base_func): Detect and appropriately
munge copyfrom urls as paths.
Modified:
subversion/trunk/subversion/libsvn_client/util.c
Modified: subversion/trunk/subversion/libsvn_client/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/util.c?rev=1296056&r1=1296055&r2=1296056&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/util.c (original)
+++ subversion/trunk/subversion/libsvn_client/util.c Fri Mar 2 05:33:22 2012
@@ -256,8 +256,25 @@ fetch_props_func(apr_hash_t **props,
apr_pool_t *scratch_pool)
{
struct shim_callbacks_baton *scb = baton;
- const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
- scratch_pool);
+ const char *local_abspath;
+
+ if (svn_path_is_url(path))
+ {
+ /* This is a copyfrom URL */
+ const char *wcroot_abspath;
+ const char *wcroot_url;
+ const char *relpath;
+
+ SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
+ scb->anchor_abspath,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
+ scratch_pool, scratch_pool));
+ relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
+ local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
+ }
+ else
+ local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
SVN_ERR(svn_wc_get_pristine_props(props, scb->wc_ctx, local_abspath,
result_pool, scratch_pool));
@@ -274,8 +291,25 @@ fetch_kind_func(svn_kind_t *kind,
{
struct shim_callbacks_baton *scb = baton;
svn_node_kind_t node_kind;
- const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
- scratch_pool);
+ const char *local_abspath;
+
+ if (svn_path_is_url(path))
+ {
+ /* This is a copyfrom URL */
+ const char *wcroot_abspath;
+ const char *wcroot_url;
+ const char *relpath;
+
+ SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
+ scb->anchor_abspath,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
+ scratch_pool, scratch_pool));
+ relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
+ local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
+ }
+ else
+ local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
SVN_ERR(svn_wc_read_kind(&node_kind, scb->wc_ctx, local_abspath, FALSE,
scratch_pool));
@@ -293,12 +327,29 @@ fetch_base_func(const char **filename,
apr_pool_t *scratch_pool)
{
struct shim_callbacks_baton *scb = baton;
- const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
- scratch_pool);
+ const char *local_abspath;
svn_stream_t *pristine_stream;
svn_stream_t *temp_stream;
svn_error_t *err;
+ if (svn_path_is_url(path))
+ {
+ /* This is a copyfrom URL */
+ const char *wcroot_abspath;
+ const char *wcroot_url;
+ const char *relpath;
+
+ SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
+ scb->anchor_abspath,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
+ scratch_pool, scratch_pool));
+ relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
+ local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
+ }
+ else
+ local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
+
err = svn_wc_get_pristine_contents2(&pristine_stream, scb->wc_ctx,
local_abspath, scratch_pool,
scratch_pool);
Re: svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Fri, Mar 2, 2012 at 6:20 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 03/02/2012 01:03 AM, Greg Stein wrote:
>> On Fri, Mar 2, 2012 at 01:01, Greg Stein <gs...@gmail.com> wrote:
>>> On Fri, Mar 2, 2012 at 00:47, Greg Stein <gs...@gmail.com> wrote:
>>> Looking more into this, the shims should accept an "anchor_url" and if
>>> they receive a URL in the copyfrom_path, then it should turn that URL
>>> into a relpath from that anchor. And *then* invoke the callback.
>>>
>>> IOW, I think the original bug is that a URL is passed into
>>> ev1->add_file(copyfrom_path). The shim should strip that down to a
>>> relpath before invoking the callback.
>>
>> And replying to myself one more time... :-)
>>
>> Yes, I know that the delta editor spec allows a URL to be passed as
>> that parameter. But I don't think we should propagate it into the
>> callbacks. Messy.
I would welcome a cleaner handling of this, but as structured we just
don't have enough information to generate a proper relpath from the
URL that Ev1 senders give us.
> Just to add, you know, a second voice to this already lengthy conversation
> :-): that Ev1 add_file/dir allows URLs is definitely confusing at times.
> The need for it is fairly obvious, though: sometimes you have to copy from
> a location that's outside the edit tree, requiring either an absolute path
> reference (such as a URL) or a relative path with '..' components. URLs
> were the obvious first choice.
>
> If Ev2 can handle such cases without using URLs, +1 to barring their
> introduction into the supported parameter set.
Agreed, though I think the changes required to support such a paradigm
will be much more intrusive than it may appear at first blush (and
somewhat beyond the scope of what I'm trying to do *right now*). If
somebody wants to jump in and tackle them, I'd be much obliged.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 03/02/2012 01:03 AM, Greg Stein wrote:
> On Fri, Mar 2, 2012 at 01:01, Greg Stein <gs...@gmail.com> wrote:
>> On Fri, Mar 2, 2012 at 00:47, Greg Stein <gs...@gmail.com> wrote:
>> Looking more into this, the shims should accept an "anchor_url" and if
>> they receive a URL in the copyfrom_path, then it should turn that URL
>> into a relpath from that anchor. And *then* invoke the callback.
>>
>> IOW, I think the original bug is that a URL is passed into
>> ev1->add_file(copyfrom_path). The shim should strip that down to a
>> relpath before invoking the callback.
>
> And replying to myself one more time... :-)
>
> Yes, I know that the delta editor spec allows a URL to be passed as
> that parameter. But I don't think we should propagate it into the
> callbacks. Messy.
Just to add, you know, a second voice to this already lengthy conversation
:-): that Ev1 add_file/dir allows URLs is definitely confusing at times.
The need for it is fairly obvious, though: sometimes you have to copy from
a location that's outside the edit tree, requiring either an absolute path
reference (such as a URL) or a relative path with '..' components. URLs
were the obvious first choice.
If Ev2 can handle such cases without using URLs, +1 to barring their
introduction into the supported parameter set.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Mar 2, 2012 at 01:01, Greg Stein <gs...@gmail.com> wrote:
> On Fri, Mar 2, 2012 at 00:47, Greg Stein <gs...@gmail.com> wrote:
>>
>> On Mar 2, 2012 12:33 AM, <hw...@apache.org> wrote:
>>>
>>> Author: hwright
>>> Date: Fri Mar 2 05:33:22 2012
>>> New Revision: 1296056
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
>>> Log:
>>> In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom
>>> paths
>>> correctly.
>>>
>>> Current number of test failures over ra_svn: 357
>>>
>>> * subversion/libsvn_client/util.c
>>> (fetch_props_func, fetch_kind_func, fetch_base_func): Detect and
>>> appropriately
>>> munge copyfrom urls as paths.
>>
>> Woah... wait a second. How did a URL get into the callback? I really think
>> these callbacks should have a single semantic: relpath. But why should we
>> continue the client monstrosity of "maybe path; maybe URL".
>>
>> ???
>>
>> This change seems to paper over a URL leaking into the callbacks. That
>> doesn't seem right.
>
> Looking more into this, the shims should accept an "anchor_url" and if
> they receive a URL in the copyfrom_path, then it should turn that URL
> into a relpath from that anchor. And *then* invoke the callback.
>
> IOW, I think the original bug is that a URL is passed into
> ev1->add_file(copyfrom_path). The shim should strip that down to a
> relpath before invoking the callback.
And replying to myself one more time... :-)
Yes, I know that the delta editor spec allows a URL to be passed as
that parameter. But I don't think we should propagate it into the
callbacks. Messy.
Cheers,
-g
Re: svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Mar 2, 2012 at 00:47, Greg Stein <gs...@gmail.com> wrote:
>
> On Mar 2, 2012 12:33 AM, <hw...@apache.org> wrote:
>>
>> Author: hwright
>> Date: Fri Mar 2 05:33:22 2012
>> New Revision: 1296056
>>
>> URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
>> Log:
>> In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom
>> paths
>> correctly.
>>
>> Current number of test failures over ra_svn: 357
>>
>> * subversion/libsvn_client/util.c
>> (fetch_props_func, fetch_kind_func, fetch_base_func): Detect and
>> appropriately
>> munge copyfrom urls as paths.
>
> Woah... wait a second. How did a URL get into the callback? I really think
> these callbacks should have a single semantic: relpath. But why should we
> continue the client monstrosity of "maybe path; maybe URL".
>
> ???
>
> This change seems to paper over a URL leaking into the callbacks. That
> doesn't seem right.
Looking more into this, the shims should accept an "anchor_url" and if
they receive a URL in the copyfrom_path, then it should turn that URL
into a relpath from that anchor. And *then* invoke the callback.
IOW, I think the original bug is that a URL is passed into
ev1->add_file(copyfrom_path). The shim should strip that down to a
relpath before invoking the callback.
Cheers,
-g
Re: svn commit: r1296056 - /subversion/trunk/subversion/libsvn_client/util.c
Posted by Greg Stein <gs...@gmail.com>.
On Mar 2, 2012 12:33 AM, <hw...@apache.org> wrote:
>
> Author: hwright
> Date: Fri Mar 2 05:33:22 2012
> New Revision: 1296056
>
> URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
> Log:
> In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom
paths
> correctly.
>
> Current number of test failures over ra_svn: 357
>
> * subversion/libsvn_client/util.c
> (fetch_props_func, fetch_kind_func, fetch_base_func): Detect and
appropriately
> munge copyfrom urls as paths.
Woah... wait a second. How did a URL get into the callback? I really think
these callbacks should have a single semantic: relpath. But why should we
continue the client monstrosity of "maybe path; maybe URL".
???
This change seems to paper over a URL leaking into the callbacks. That
doesn't seem right.
Cheers,
-g
Re: svn commit: r1296056 -
/subversion/trunk/subversion/libsvn_client/util.c
Posted by Daniel Shahaf <da...@elego.de>.
Eewww: I pressed 'page down' and the picture didn't change. (Duplicate
block of code.) Break out a helper function?
hwright@apache.org wrote on Fri, Mar 02, 2012 at 05:33:22 -0000:
> Author: hwright
> Date: Fri Mar 2 05:33:22 2012
> New Revision: 1296056
>
> URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
> Log:
> In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom paths
> correctly.
>
> Current number of test failures over ra_svn: 357
>
> * subversion/libsvn_client/util.c
> (fetch_props_func, fetch_kind_func, fetch_base_func): Detect and appropriately
> munge copyfrom urls as paths.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/util.c
>
> Modified: subversion/trunk/subversion/libsvn_client/util.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/util.c?rev=1296056&r1=1296055&r2=1296056&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/util.c (original)
> +++ subversion/trunk/subversion/libsvn_client/util.c Fri Mar 2 05:33:22 2012
> @@ -256,8 +256,25 @@ fetch_props_func(apr_hash_t **props,
> apr_pool_t *scratch_pool)
> {
> struct shim_callbacks_baton *scb = baton;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> +
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
>
> SVN_ERR(svn_wc_get_pristine_props(props, scb->wc_ctx, local_abspath,
> result_pool, scratch_pool));
> @@ -274,8 +291,25 @@ fetch_kind_func(svn_kind_t *kind,
> {
> struct shim_callbacks_baton *scb = baton;
> svn_node_kind_t node_kind;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> +
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
>
> SVN_ERR(svn_wc_read_kind(&node_kind, scb->wc_ctx, local_abspath, FALSE,
> scratch_pool));
> @@ -293,12 +327,29 @@ fetch_base_func(const char **filename,
> apr_pool_t *scratch_pool)
> {
> struct shim_callbacks_baton *scb = baton;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> svn_stream_t *pristine_stream;
> svn_stream_t *temp_stream;
> svn_error_t *err;
>
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
> +
> err = svn_wc_get_pristine_contents2(&pristine_stream, scb->wc_ctx,
> local_abspath, scratch_pool,
> scratch_pool);
>
>
Re: svn commit: r1296056 -
/subversion/trunk/subversion/libsvn_client/util.c
Posted by Daniel Shahaf <da...@elego.de>.
Eewww: I pressed 'page down' and the picture didn't change. (Duplicate
block of code.) Break out a helper function?
hwright@apache.org wrote on Fri, Mar 02, 2012 at 05:33:22 -0000:
> Author: hwright
> Date: Fri Mar 2 05:33:22 2012
> New Revision: 1296056
>
> URL: http://svn.apache.org/viewvc?rev=1296056&view=rev
> Log:
> In the client-side ra Ev2 shim callbacks, make sure we handle copyfrom paths
> correctly.
>
> Current number of test failures over ra_svn: 357
>
> * subversion/libsvn_client/util.c
> (fetch_props_func, fetch_kind_func, fetch_base_func): Detect and appropriately
> munge copyfrom urls as paths.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/util.c
>
> Modified: subversion/trunk/subversion/libsvn_client/util.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/util.c?rev=1296056&r1=1296055&r2=1296056&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/util.c (original)
> +++ subversion/trunk/subversion/libsvn_client/util.c Fri Mar 2 05:33:22 2012
> @@ -256,8 +256,25 @@ fetch_props_func(apr_hash_t **props,
> apr_pool_t *scratch_pool)
> {
> struct shim_callbacks_baton *scb = baton;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> +
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
>
> SVN_ERR(svn_wc_get_pristine_props(props, scb->wc_ctx, local_abspath,
> result_pool, scratch_pool));
> @@ -274,8 +291,25 @@ fetch_kind_func(svn_kind_t *kind,
> {
> struct shim_callbacks_baton *scb = baton;
> svn_node_kind_t node_kind;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> +
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
>
> SVN_ERR(svn_wc_read_kind(&node_kind, scb->wc_ctx, local_abspath, FALSE,
> scratch_pool));
> @@ -293,12 +327,29 @@ fetch_base_func(const char **filename,
> apr_pool_t *scratch_pool)
> {
> struct shim_callbacks_baton *scb = baton;
> - const char *local_abspath = svn_dirent_join(scb->anchor_abspath, path,
> - scratch_pool);
> + const char *local_abspath;
> svn_stream_t *pristine_stream;
> svn_stream_t *temp_stream;
> svn_error_t *err;
>
> + if (svn_path_is_url(path))
> + {
> + /* This is a copyfrom URL */
> + const char *wcroot_abspath;
> + const char *wcroot_url;
> + const char *relpath;
> +
> + SVN_ERR(svn_wc__get_wc_root(&wcroot_abspath, scb->wc_ctx,
> + scb->anchor_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_url(&wcroot_url, scb->wc_ctx, wcroot_abspath,
> + scratch_pool, scratch_pool));
> + relpath = svn_uri_skip_ancestor(wcroot_url, path, scratch_pool);
> + local_abspath = svn_dirent_join(wcroot_abspath, relpath, scratch_pool);
> + }
> + else
> + local_abspath = svn_dirent_join(scb->anchor_abspath, path, scratch_pool);
> +
> err = svn_wc_get_pristine_contents2(&pristine_stream, scb->wc_ctx,
> local_abspath, scratch_pool,
> scratch_pool);
>
>