You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@subversion.apache.org by Shlomi Fish <sh...@vipe.stud.technion.ac.il> on 2003/07/07 18:23:20 UTC

[PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Here's a patch for this issue. I hope I got it right.

Regards,

	Shlomi Fish


----------------------------------------------------------------------
Shlomi Fish        shlomif@vipe.technion.ac.il
Home Page:         http://t2.technion.ac.il/~shlomif/

An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.

	Falk Fish

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Paul L Lussier <pl...@lanminds.com>.

>>>>> On Mon, 7 Jul 2003, "Shlomi" == Shlomi Fish wrote:

  Shlomi> Here's a patch for this issue. I hope I got it right.

Added URL of archive e-mail containing patch to issue 1313:

	http://subversion.tigris.org/issues/show_bug.cgi?id=1313
-- 

Seeya,
Paul



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by cm...@collab.net.

Ben Collins-Sussman <su...@collab.net> writes:

> In this case, I'm not sure how it would even be possible.  We open a
> session to a URL, then get a commit-editor from the RA layer, then
> tell it to copy one path to another.  The RA layer never even gets a
> chance to *see* the second URL. 

Well, we *could* open the session to the common parent of the source
and dest URLs.  If that fails, the two URLs aren't in the same repos.
The downside is that you now have to either a) re-open the session to
the destination URL, or b) do a (potential) bunch of open_dir()s down
to the dest's parent.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Philip Martin <ph...@codematters.co.uk>.

Branko Äibej <br...@xbc.nu> writes:

> Philip Martin wrote:
>
>>It just works if the URLs refer to the same repository, no need to
>>check UUIDs.  However, if the URLs refer to separate repositories then
>>the the current code
>>
>>   common_url = get_longest_ancestor (source_url, target_url)
>>   open_ra_session (common_url)
>>
>>is going to generate common_url that does not refer to a repository,
>>that's going to cause an error, isn't it?  If the error really must be
>>"improved", then perhaps the solution is to check paths in the
>>repository, rather than UUIDs.
>>  
>>
> "It ain't necessarily so."
>
> It's entirely possible to have two completely different URLs refer to
> the same repository. Through symlinks in the filesystem, interesting
> Apache configuration, etc. etc. So to make sure, you have to open both
> URLs and check the UUIDs.

I don't see how that is relevant.

Lets assume the URLs "http://server/foo/" and "http://server/bar/" are
repository roots that refer to the same repository, in which case
"http://server/" is not a repository.  If you attempt to copy
something from a foo URL into a bar URL the current code will attempt
to use a single session opened on /.  It will fail, and checking UUIDs
won't make it work.

Now I suppose we could add UUID checks, identify that the two URLs are
the same repository, and attempt to rewrite one URL in terms of the
other.  Are you suggesting we try that?  I don't see that doing it
would be particularly useful.

My original argument is that for cases where the copy works there is
no need to check UUIDs, and for cases where the the UUIDs don't match
the copy will already fail.  Admittedly, when we support cross
repository copies things may change, but when that happens the whole
common_url stuff will need to be rewritten.

Apart from a "neater" error message, what does the UUID check achieve?
If the improved error message is deemed to be important then perhaps
the UUID check could be done after the copy attempt has failed.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Branko Čibej <br...@xbc.nu>.

Philip Martin wrote:

>It just works if the URLs refer to the same repository, no need to
>check UUIDs.  However, if the URLs refer to separate repositories then
>the the current code
>
>   common_url = get_longest_ancestor (source_url, target_url)
>   open_ra_session (common_url)
>
>is going to generate common_url that does not refer to a repository,
>that's going to cause an error, isn't it?  If the error really must be
>"improved", then perhaps the solution is to check paths in the
>repository, rather than UUIDs.
>  
>
"It ain't necessarily so."

It's entirely possible to have two completely different URLs refer to
the same repository. Through symlinks in the filesystem, interesting
Apache configuration, etc. etc. So to make sure, you have to open both
URLs and check the UUIDs.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Philip Martin <ph...@codematters.co.uk>.

Ben Collins-Sussman <su...@collab.net> writes:

> Philip Martin <ph...@codematters.co.uk> writes:
>
>> My objection is more that three connections are being made while it
>> should be possible to get away with only one. There is no way that a
>> single session can handle cross repository copies, so attempts to do
>> it must already fail.
>
> That implies that each function in each RA layer must be in charge of
> noticing multiple UUIDs.  Oof.

No, I don't think so.

> In this case, I'm not sure how it would even be possible.  We open a
> session to a URL, then get a commit-editor from the RA layer, then
> tell it to copy one path to another.  The RA layer never even gets a
> chance to *see* the second URL. 

It just works if the URLs refer to the same repository, no need to
check UUIDs.  However, if the URLs refer to separate repositories then
the the current code

   common_url = get_longest_ancestor (source_url, target_url)
   open_ra_session (common_url)

is going to generate common_url that does not refer to a repository,
that's going to cause an error, isn't it?  If the error really must be
"improved", then perhaps the solution is to check paths in the
repository, rather than UUIDs.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Ben Collins-Sussman <su...@collab.net>.

Philip Martin <ph...@codematters.co.uk> writes:

> My objection is more that three connections are being made while it
> should be possible to get away with only one. There is no way that a
> single session can handle cross repository copies, so attempts to do
> it must already fail.

That implies that each function in each RA layer must be in charge of
noticing multiple UUIDs.  Oof.

In this case, I'm not sure how it would even be possible.  We open a
session to a URL, then get a commit-editor from the RA layer, then
tell it to copy one path to another.  The RA layer never even gets a
chance to *see* the second URL. 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Philip Martin <ph...@codematters.co.uk>.

Ben Collins-Sussman <su...@collab.net> writes:

> Philip Martin <ph...@codematters.co.uk> writes:
>
>> This looks a wrong to me.  At present an URL to URL copy uses a single
>> RA session because only copies within a repository are supported.
>> Your patch introduces the overhead of two additional RA sessions just
>> to handle an error case.  For those cases where there is no error
>> (which is likely to be the common case) three RA sessions will be
>> opened onto the same repository.
>
> Well, I was thinking maybe his new block of code could just use a
> subpool to open the 2 sessions, then destroy the subpool.

My objection is more that three connections are being made while it
should be possible to get away with only one. There is no way that a
single session can handle cross repository copies, so attempts to do
it must already fail.  Adding the overhead introduced by the patch to
copies that work, simply to get some sort of "neat" error message for
cases that already fail, just doesn't seem right to me.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Ben Collins-Sussman <su...@collab.net>.

Philip Martin <ph...@codematters.co.uk> writes:

> This looks a wrong to me.  At present an URL to URL copy uses a single
> RA session because only copies within a repository are supported.
> Your patch introduces the overhead of two additional RA sessions just
> to handle an error case.  For those cases where there is no error
> (which is likely to be the common case) three RA sessions will be
> opened onto the same repository.

Well, I was thinking maybe his new block of code could just use a
subpool to open the 2 sessions, then destroy the subpool.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1313 - svn copy URL1 URL2 fails silently

Posted by Philip Martin <ph...@codematters.co.uk>.

Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:

> Here's a patch for this issue. I hope I got it right.

No log message.

> --- subversion/subversion/libsvn_client/copy.c.orig	2003-07-07 20:39:59.000000000 +0300
> +++ subversion/subversion/libsvn_client/copy.c	2003-07-07 21:13:17.000000000 +0300

Hmm, you don't appear to be using 'svn diff'.  Any reason for that?

> @@ -321,11 +321,55 @@
>  
>    /* Get the RA vtable that matches URL. */
>    SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> -  SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, top_url, pool));
>  
>    /* Get the auth dir. */
>    SVN_ERR (svn_client__dir_if_wc (&auth_dir, "", pool));
>  
> +  /* This entire code block is meant to find out if the user is attempting
> +   * a cross-repository copy, which is not supported yet.*/
> +  {
> +    svn_ra_plugin_t * src_ra_lib, * dest_ra_lib;
> +    const char * src_uuid = NULL, * dest_uuid = NULL;
> +    svn_error_t * src_err, *dest_err;
> +    void * src_sess, * dest_sess;
> +
> +    SVN_ERR(svn_ra_get_ra_library (&src_ra_lib, ra_baton, src_url, pool));
> +    SVN_ERR(svn_ra_get_ra_library (&dest_ra_lib, ra_baton, dst_url, pool));
> +
> +    /* Open an RA session for the URL. Note that we don't have a local
> +       directory, nor a place to put temp files or store the auth data. */
> +    SVN_ERR (svn_client__open_ra_session (&src_sess, src_ra_lib, src_url,
> +                                          auth_dir,
> +                                          NULL, NULL, FALSE, TRUE,
> +                                          ctx, pool));
> +
> +    /* Open an RA session for the URL. Note that we don't have a local
> +       directory, nor a place to put temp files or store the auth data. */
> +    SVN_ERR (svn_client__open_ra_session (&dest_sess, dest_ra_lib, dst_url,
> +                                          auth_dir,
> +                                          NULL, NULL, FALSE, TRUE,
> +                                          ctx, pool));
> +
> +    /* Get the repository uuid of SRC_URL */
> +    src_err = src_ra_lib->get_uuid(src_sess, &src_uuid, pool);
> +    if (src_err && src_err->apr_err != SVN_ERR_RA_NO_REPOS_UUID)
> +      return src_err;
> +
> +    /* Get the repository uuid of DEST_URL */
> +    dest_err = dest_ra_lib->get_uuid(dest_sess, &dest_uuid, pool);
> +    if (dest_err && dest_err->apr_err != SVN_ERR_RA_NO_REPOS_UUID)
> +      return dest_err;

This looks a wrong to me.  At present an URL to URL copy uses a single
RA session because only copies within a repository are supported.
Your patch introduces the overhead of two additional RA sessions just
to handle an error case.  For those cases where there is no error
(which is likely to be the common case) three RA sessions will be
opened onto the same repository.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org