You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2010/11/01 09:30:00 UTC

Re: Issue 3727 - svn export into the current directory

Noorul Islam K M <no...@collab.net> writes:

> @@ -79,8 +84,17 @@
>      {
>        to = APR_ARRAY_IDX(targets, 1, const char *);
>  
> +      /* Get the RA connection. */
> +      SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
> +                                               &url, truefrom, NULL,
> +                                               &peg_revision,
> +                                               &(opt_state->start_revision),
> +                                               ctx, pool));
> +
> +      SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
> +
>        /* If given the cwd, pretend we weren't given anything. */
> -      if (strcmp("", to) == 0)
> +      if ((strcmp("", to) == 0) && (kind == svn_node_file))
>          to = svn_path_uri_decode(svn_uri_basename(truefrom, pool), pool);
>        else
>          /* svn_cl__eat_peg_revisions() but only on one target */

svn_ra_check_path is expensive, as it involves a round trip to the
server, and output is not always used.  Can we avoid the call when the
kind is not needed?

-- 
Philip

Re: Issue 3727 - svn export into the current directory

Posted by Philip Martin <ph...@wandisco.com>.
Noorul Islam K M <no...@collab.net> writes:

> [[[
> Fix issue 3727: Fix regression caused by r880559
>
> * subversion/tests/cmdline/export_tests.py
>   (export_to_explicit_cwd): Remove XFail.
>
> * subversion/svn/export-cmd.c
>   (svn_cl__export): Move logic into subversion/libsvn_client/export.c
>
> * subversion/libsvn_client/export.c
>   (svn_client_export5): If the explicit TO path is '.', treat it as if it weren't
>     given only if the source is a file.
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]

I tweaked it to use svn_path_is_empty, to pass a NULL pool where the
basename does not need to be allocated, and to update the
svn_client_export5 docstring.  r1029751.  Thanks!

-- 
Philip

Re: Issue 3727 - svn export into the current directory

Posted by Noorul Islam K M <no...@collab.net>.
Philip Martin <ph...@wandisco.com> writes:

> Noorul Islam K M <no...@collab.net> writes:
>
>> Philip Martin <ph...@wandisco.com> writes:
>>
>>> Noorul Islam K M <no...@collab.net> writes:
>>>
>>>> @@ -79,8 +84,17 @@
>>>>      {
>>>>        to = APR_ARRAY_IDX(targets, 1, const char *);
>>>>  
>>>> +      /* Get the RA connection. */
>>>> +      SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
>>>> +                                               &url, truefrom, NULL,
>>>> +                                               &peg_revision,
>>>> +                                               &(opt_state->start_revision),
>>>> +                                               ctx, pool));
>>>> +
>>>> +      SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
>>>> +
>>>>        /* If given the cwd, pretend we weren't given anything. */
>>>> -      if (strcmp("", to) == 0)
>>>> +      if ((strcmp("", to) == 0) && (kind == svn_node_file))
>>>>          to = svn_path_uri_decode(svn_uri_basename(truefrom, pool), pool);
>>>>        else
>>>>          /* svn_cl__eat_peg_revisions() but only on one target */
>>>
>>> svn_ra_check_path is expensive, as it involves a round trip to the
>>> server, and output is not always used.  Can we avoid the call when the
>>> kind is not needed?
>>
>> But I think here we need to check kind to branch. Do you mean to say
>> that we don't need kind in all cases?
>
> If the call strcpm("", to) returns non-zero then kind is never used.
>
> Including client.h is wrong as well.

Here is a modified patch. I moved the logic into export.c where we
already use ra_session.

[[[
Fix issue 3727: Fix regression caused by r880559

* subversion/tests/cmdline/export_tests.py
  (export_to_explicit_cwd): Remove XFail.

* subversion/svn/export-cmd.c
  (svn_cl__export): Move logic into subversion/libsvn_client/export.c

* subversion/libsvn_client/export.c
  (svn_client_export5): If the explicit TO path is '.', treat it as if it weren't
    given only if the source is a file.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]


Thanks and Regards
Noorul


Re: Issue 3727 - svn export into the current directory

Posted by Philip Martin <ph...@wandisco.com>.
Noorul Islam K M <no...@collab.net> writes:

> Philip Martin <ph...@wandisco.com> writes:
>
>> Noorul Islam K M <no...@collab.net> writes:
>>
>>> @@ -79,8 +84,17 @@
>>>      {
>>>        to = APR_ARRAY_IDX(targets, 1, const char *);
>>>  
>>> +      /* Get the RA connection. */
>>> +      SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
>>> +                                               &url, truefrom, NULL,
>>> +                                               &peg_revision,
>>> +                                               &(opt_state->start_revision),
>>> +                                               ctx, pool));
>>> +
>>> +      SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
>>> +
>>>        /* If given the cwd, pretend we weren't given anything. */
>>> -      if (strcmp("", to) == 0)
>>> +      if ((strcmp("", to) == 0) && (kind == svn_node_file))
>>>          to = svn_path_uri_decode(svn_uri_basename(truefrom, pool), pool);
>>>        else
>>>          /* svn_cl__eat_peg_revisions() but only on one target */
>>
>> svn_ra_check_path is expensive, as it involves a round trip to the
>> server, and output is not always used.  Can we avoid the call when the
>> kind is not needed?
>
> But I think here we need to check kind to branch. Do you mean to say
> that we don't need kind in all cases?

If the call strcpm("", to) returns non-zero then kind is never used.

Including client.h is wrong as well.

-- 
Philip

Re: Issue 3727 - svn export into the current directory

Posted by Noorul Islam K M <no...@collab.net>.
Philip Martin <ph...@wandisco.com> writes:

> Noorul Islam K M <no...@collab.net> writes:
>
>> @@ -79,8 +84,17 @@
>>      {
>>        to = APR_ARRAY_IDX(targets, 1, const char *);
>>  
>> +      /* Get the RA connection. */
>> +      SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
>> +                                               &url, truefrom, NULL,
>> +                                               &peg_revision,
>> +                                               &(opt_state->start_revision),
>> +                                               ctx, pool));
>> +
>> +      SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
>> +
>>        /* If given the cwd, pretend we weren't given anything. */
>> -      if (strcmp("", to) == 0)
>> +      if ((strcmp("", to) == 0) && (kind == svn_node_file))
>>          to = svn_path_uri_decode(svn_uri_basename(truefrom, pool), pool);
>>        else
>>          /* svn_cl__eat_peg_revisions() but only on one target */
>
> svn_ra_check_path is expensive, as it involves a round trip to the
> server, and output is not always used.  Can we avoid the call when the
> kind is not needed?

But I think here we need to check kind to branch. Do you mean to say
that we don't need kind in all cases?

Thanks and Regards
Noorul