You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Chris Rose <ch...@messagingdirect.com> on 2008/01/07 16:20:01 UTC

Functionality regression from 1.4->1.5 in svn copy due to --use-merge-history changes

As a consequence of some changes associated with revsion r26495 in
libsvn_client/copy.c something that was possible in svn 1.4 can no
longer be accomplished in svn 1.5.  This might be intentional, but I
suspect it was an accidental casualty:

In 1.4, this sequence of operations is possible:
- check out a module 'm1' into non-wc directory 'base'
- cwd to a directory completely outside of 'base' that is a working copy
for another path in the same repository as 'm1', above.
- svn cp 'base/m1' `pwd`

In 1.5, this sequence of operations fails with this message:
# svn: 'base' is not a working copy
# svn: Can't open file 'base/.svn/entries': No such file or directory

I don't know the propagate_mergeinfo_within_wc method well enough to say
what it's doing with src_access (acquired by calling svn_wc_adm_open3 on
'base' in my example) or to patch the code to change the behaviour.

My question is:  is it necessary to get the parent directory's metadata
if it's not a working copy?  Perhaps somebody more involved in the
--use-merge-history work could comment, please?

Attached is a script that -- with the substitution of the correct paths
for svn 1.4 and 1.4 binaries -- will demonstrate the issue directly.

Thank you for your time.

-- 
Chris Rose
Developer    Planet Consulting Group
(780) 577-8433
crose@planetci.com

Re: [PATCH] allow unversioned parent for copy src again

Posted by Chris Rose <ch...@messagingdirect.com>.
To the extent that I'm competent to comment (not much!) that looks like 
what I was considering trying on my own.

Thank you for the fix.

Karl Fogel wrote:
> Chris Rose <ch...@messagingdirect.com> writes:
>> I'm sorry to prod on this, but I'm quite concerned about the change in
>> functionality, and would like to find out if this will remain in place
>> for 1.5, or if I should register it as a bug with the client?
>>
>> Can somebody weigh in either way, please?
> 
> It sounds like a bug to me.  I've filed an issue for it, and written a
> tentative patch.  Here's the issue:
> 
>    http://subversion.tigris.org/issues/show_bug.cgi?id=3068
> 
> The patch is below.  I'd love some other eyes on it before I commit,
> though, from people more familiar with wc mergeinfo propagation.
> 
> Thanks for the reproduction script, btw, that helped a lot.
> 
> Here's the patch:
> 
> [[[
> Fix issue #3068: wc->wc copy should work when src parent is unversioned.
> 
> * subversion/libsvn_client/copy.c
>   (do_wc_to_wc_copies): Propagate mergeinfo from parent only if possible.
> 
> * subversion/libsvn_wc/lock.c
>   (do_open): Document an error return that callers depend on.
> ]]]
> 
> Index: subversion/libsvn_client/copy.c
> ===================================================================
> --- subversion/libsvn_client/copy.c	(revision 28822)
> +++ subversion/libsvn_client/copy.c	(working copy)
> @@ -347,10 +347,18 @@
>          }
>        else
>          {
> -          SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
> -                                   pair->src_kind == svn_node_dir ? -1 : 0,
> -                                   ctx->cancel_func, ctx->cancel_baton,
> -                                   iterpool));
> +          err = svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
> +                                 pair->src_kind == svn_node_dir ? -1 : 0,
> +                                 ctx->cancel_func, ctx->cancel_baton,
> +                                 iterpool);
> +          /* The parent of a copy src might not be versioned at all. */
> +          if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> +            {
> +              src_access = NULL;
> +              svn_error_clear(err);
> +              err = NULL;
> +            }
> +          SVN_ERR(err);
>          }
>  
>        /* Perform the copy */
> @@ -365,12 +373,13 @@
>        if (err)
>          break;
>  
> -      err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
> -                                          ctx, pool);
> +      if (src_access)
> +        err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
> +                                            ctx, pool);
>        if (err)
>          break;
>  
> -      if (src_access != dst_access)
> +      if (src_access && src_access != dst_access)
>          SVN_ERR(svn_wc_adm_close(src_access));
>      }
>  
> Index: subversion/libsvn_wc/lock.c
> ===================================================================
> --- subversion/libsvn_wc/lock.c	(revision 28822)
> +++ subversion/libsvn_wc/lock.c	(working copy)
> @@ -542,6 +542,9 @@
>  /* This is essentially the guts of svn_wc_adm_open3, with the additional
>   * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
>   * admin directory during initial creation.
> + *
> + * If the working copy is already locked, return SVN_ERR_WC_LOCKED; if 
> + * it is not a versioned directory, return SVN_ERR_WC_NOT_DIRECTORY.
>   */
>  static svn_error_t *
>  do_open(svn_wc_adm_access_t **adm_access,

-- 
Chris Rose
Developer    Planet Consulting Group
(780) 577-8433
crose@planetci.com


Re: [PATCH] allow unversioned parent for copy src again

Posted by "C. Michael Pilato" <cm...@collab.net>.
Karl Fogel wrote:
> Chris Rose <ch...@messagingdirect.com> writes:
>   
>> I'm sorry to prod on this, but I'm quite concerned about the change in
>> functionality, and would like to find out if this will remain in place
>> for 1.5, or if I should register it as a bug with the client?
>>
>> Can somebody weigh in either way, please?
>>     
>
> It sounds like a bug to me.  I've filed an issue for it, and written a
> tentative patch.  Here's the issue:
>
>    http://subversion.tigris.org/issues/show_bug.cgi?id=3068
>
> The patch is below.  I'd love some other eyes on it before I commit,
> though, from people more familiar with wc mergeinfo propagation.
>   
I've reviewed the patch, made a few tweaks, and committed as r28825.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



Re: [PATCH] allow unversioned parent for copy src again

Posted by Daniel <da...@fastmail.fm>.
Lieven Govaerts wrote on Wed, 9 Jan 2008 at 22:44:00 +0100:
> Daniel wrote:
>> Lieven Govaerts wrote on Wed, 9 Jan 2008 at 22:12:00 +0100:
>>>
>>> * lgo volunteers writing the test.
>>
>> Could you review this one?
>>
>> [[[
>> Add regression test for issue #3068.
>>
>> * subversion/tests/cmdline/copy_tests.py
>>   (allow_unversioned_parent_for_copy_src): New test for issue #3068.
>>   (test_list): Run it.
>>
>> Suggested by: glasser
>> ]]]
>>
>
> Reviewed, tweaked a bit and committed in r28832. Good work, thanks!
>

Thanks for your review, Lieven; I'll look at the tweaks you made.

Could you fix the comment I put on the run_and_verify_svn() call?  It
speaks of "a copy between different repositories", while the test does
not use multiple repositories.

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

Re: [PATCH] allow unversioned parent for copy src again

Posted by Lieven Govaerts <sv...@mobsol.be>.
Daniel wrote:
> Lieven Govaerts wrote on Wed, 9 Jan 2008 at 22:12:00 +0100:
>> Karl Fogel wrote:
>>> Chris Rose <ch...@messagingdirect.com> writes:
>>>> I can look at it, but I'm not loaded with time right now.  I can keep
>>>> an eye on it and write one later.
>>>
>>> Okay, we won't count on you for the test, no problem.
>>>
>>
>> * lgo volunteers writing the test.
> 
> Could you review this one?
> 
> [[[
> Add regression test for issue #3068.
> 
> * subversion/tests/cmdline/copy_tests.py
>   (allow_unversioned_parent_for_copy_src): New test for issue #3068.
>   (test_list): Run it.
> 
> Suggested by: glasser
> ]]]
> 

Reviewed, tweaked a bit and committed in r28832. Good work, thanks!

Lieven

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

Re: [PATCH] allow unversioned parent for copy src again

Posted by Daniel <da...@fastmail.fm>.
Lieven Govaerts wrote on Wed, 9 Jan 2008 at 22:12:00 +0100:
> Karl Fogel wrote:
>> Chris Rose <ch...@messagingdirect.com> writes:
>>> I can look at it, but I'm not loaded with time right now.  I can keep
>>> an eye on it and write one later.
>>
>> Okay, we won't count on you for the test, no problem.
>>
>
> * lgo volunteers writing the test.

Could you review this one?

[[[
Add regression test for issue #3068.

* subversion/tests/cmdline/copy_tests.py
   (allow_unversioned_parent_for_copy_src): New test for issue #3068.
   (test_list): Run it.

Suggested by: glasser
]]]

Thanks,

Daniel

Re: [PATCH] allow unversioned parent for copy src again

Posted by Karl Fogel <kf...@red-bean.com>.
Lieven Govaerts <sv...@mobsol.be> writes:
> Karl Fogel wrote:
>> Chris Rose <ch...@messagingdirect.com> writes:
>>> I can look at it, but I'm not loaded with time right now.  I can keep
>>> an eye on it and write one later.
>> 
>> Okay, we won't count on you for the test, no problem.
>
> * lgo volunteers writing the test.

* kfogel kisses lgo

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

Re: [PATCH] allow unversioned parent for copy src again

Posted by Lieven Govaerts <sv...@mobsol.be>.
Karl Fogel wrote:
> Chris Rose <ch...@messagingdirect.com> writes:
>> I can look at it, but I'm not loaded with time right now.  I can keep
>> an eye on it and write one later.
> 
> Okay, we won't count on you for the test, no problem.
>

* lgo volunteers writing the test.

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

Re: [PATCH] allow unversioned parent for copy src again

Posted by Karl Fogel <kf...@red-bean.com>.
Chris Rose <ch...@messagingdirect.com> writes:
> I can look at it, but I'm not loaded with time right now.  I can keep
> an eye on it and write one later.

Okay, we won't count on you for the test, no problem.

> I've got a deadline of tomorrow to convert our change management
> toolchain to use subversion instead of CVS, and I'm going insane with
> the old spaghetti code :)

Yup, know the feeling :-).

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

Re: [PATCH] allow unversioned parent for copy src again

Posted by Chris Rose <ch...@messagingdirect.com>.
I can look at it, but I'm not loaded with time right now.  I can keep an 
eye on it and write one later.

I've got a deadline of tomorrow to convert our change management 
toolchain to use subversion instead of CVS, and I'm going insane with 
the old spaghetti code :)

Karl Fogel wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
>> Looks good to me, though a test would be nice.
> 
> Agreed.  Chris, want to have a go at writing a new test for
> subversion/tests/cmdline/copy_tests.py ?  If not, that's fine, just
> let us know either way.  Thanks.
> 
> -Karl
> 
>> On Jan 9, 2008 1:14 PM, Karl Fogel <kf...@red-bean.com> wrote:
>>> Chris Rose <ch...@messagingdirect.com> writes:
>>>> I'm sorry to prod on this, but I'm quite concerned about the change in
>>>> functionality, and would like to find out if this will remain in place
>>>> for 1.5, or if I should register it as a bug with the client?
>>>>
>>>> Can somebody weigh in either way, please?
>>> It sounds like a bug to me.  I've filed an issue for it, and written a
>>> tentative patch.  Here's the issue:
>>>
>>>    http://subversion.tigris.org/issues/show_bug.cgi?id=3068
>>>
>>> The patch is below.  I'd love some other eyes on it before I commit,
>>> though, from people more familiar with wc mergeinfo propagation.
>>>
>>> Thanks for the reproduction script, btw, that helped a lot.
>>>
>>> Here's the patch:
>>>
>>> [[[
>>> Fix issue #3068: wc->wc copy should work when src parent is unversioned.
>>>
>>> * subversion/libsvn_client/copy.c
>>>   (do_wc_to_wc_copies): Propagate mergeinfo from parent only if possible.
>>>
>>> * subversion/libsvn_wc/lock.c
>>>   (do_open): Document an error return that callers depend on.
>>> ]]]
>>>
>>> Index: subversion/libsvn_client/copy.c
>>> ===================================================================
>>> --- subversion/libsvn_client/copy.c     (revision 28822)
>>> +++ subversion/libsvn_client/copy.c     (working copy)
>>> @@ -347,10 +347,18 @@
>>>          }
>>>        else
>>>          {
>>> -          SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
>>> -                                   pair->src_kind == svn_node_dir ? -1 : 0,
>>> -                                   ctx->cancel_func, ctx->cancel_baton,
>>> -                                   iterpool));
>>> +          err = svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
>>> +                                 pair->src_kind == svn_node_dir ? -1 : 0,
>>> +                                 ctx->cancel_func, ctx->cancel_baton,
>>> +                                 iterpool);
>>> +          /* The parent of a copy src might not be versioned at all. */
>>> +          if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
>>> +            {
>>> +              src_access = NULL;
>>> +              svn_error_clear(err);
>>> +              err = NULL;
>>> +            }
>>> +          SVN_ERR(err);
>>>          }
>>>
>>>        /* Perform the copy */
>>> @@ -365,12 +373,13 @@
>>>        if (err)
>>>          break;
>>>
>>> -      err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
>>> -                                          ctx, pool);
>>> +      if (src_access)
>>> +        err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
>>> +                                            ctx, pool);
>>>        if (err)
>>>          break;
>>>
>>> -      if (src_access != dst_access)
>>> +      if (src_access && src_access != dst_access)
>>>          SVN_ERR(svn_wc_adm_close(src_access));
>>>      }
>>>
>>> Index: subversion/libsvn_wc/lock.c
>>> ===================================================================
>>> --- subversion/libsvn_wc/lock.c (revision 28822)
>>> +++ subversion/libsvn_wc/lock.c (working copy)
>>> @@ -542,6 +542,9 @@
>>>  /* This is essentially the guts of svn_wc_adm_open3, with the additional
>>>   * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
>>>   * admin directory during initial creation.
>>> + *
>>> + * If the working copy is already locked, return SVN_ERR_WC_LOCKED; if
>>> + * it is not a versioned directory, return SVN_ERR_WC_NOT_DIRECTORY.
>>>   */
>>>  static svn_error_t *
>>>  do_open(svn_wc_adm_access_t **adm_access,
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>>
>>>
>>
>>
>> -- 
>> David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

-- 
Chris Rose
Developer    Planet Consulting Group
(780) 577-8433
crose@planetci.com


Re: [PATCH] allow unversioned parent for copy src again

Posted by Karl Fogel <kf...@red-bean.com>.
"David Glasser" <gl...@davidglasser.net> writes:
> Looks good to me, though a test would be nice.

Agreed.  Chris, want to have a go at writing a new test for
subversion/tests/cmdline/copy_tests.py ?  If not, that's fine, just
let us know either way.  Thanks.

-Karl

> On Jan 9, 2008 1:14 PM, Karl Fogel <kf...@red-bean.com> wrote:
>> Chris Rose <ch...@messagingdirect.com> writes:
>> > I'm sorry to prod on this, but I'm quite concerned about the change in
>> > functionality, and would like to find out if this will remain in place
>> > for 1.5, or if I should register it as a bug with the client?
>> >
>> > Can somebody weigh in either way, please?
>>
>> It sounds like a bug to me.  I've filed an issue for it, and written a
>> tentative patch.  Here's the issue:
>>
>>    http://subversion.tigris.org/issues/show_bug.cgi?id=3068
>>
>> The patch is below.  I'd love some other eyes on it before I commit,
>> though, from people more familiar with wc mergeinfo propagation.
>>
>> Thanks for the reproduction script, btw, that helped a lot.
>>
>> Here's the patch:
>>
>> [[[
>> Fix issue #3068: wc->wc copy should work when src parent is unversioned.
>>
>> * subversion/libsvn_client/copy.c
>>   (do_wc_to_wc_copies): Propagate mergeinfo from parent only if possible.
>>
>> * subversion/libsvn_wc/lock.c
>>   (do_open): Document an error return that callers depend on.
>> ]]]
>>
>> Index: subversion/libsvn_client/copy.c
>> ===================================================================
>> --- subversion/libsvn_client/copy.c     (revision 28822)
>> +++ subversion/libsvn_client/copy.c     (working copy)
>> @@ -347,10 +347,18 @@
>>          }
>>        else
>>          {
>> -          SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
>> -                                   pair->src_kind == svn_node_dir ? -1 : 0,
>> -                                   ctx->cancel_func, ctx->cancel_baton,
>> -                                   iterpool));
>> +          err = svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
>> +                                 pair->src_kind == svn_node_dir ? -1 : 0,
>> +                                 ctx->cancel_func, ctx->cancel_baton,
>> +                                 iterpool);
>> +          /* The parent of a copy src might not be versioned at all. */
>> +          if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
>> +            {
>> +              src_access = NULL;
>> +              svn_error_clear(err);
>> +              err = NULL;
>> +            }
>> +          SVN_ERR(err);
>>          }
>>
>>        /* Perform the copy */
>> @@ -365,12 +373,13 @@
>>        if (err)
>>          break;
>>
>> -      err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
>> -                                          ctx, pool);
>> +      if (src_access)
>> +        err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
>> +                                            ctx, pool);
>>        if (err)
>>          break;
>>
>> -      if (src_access != dst_access)
>> +      if (src_access && src_access != dst_access)
>>          SVN_ERR(svn_wc_adm_close(src_access));
>>      }
>>
>> Index: subversion/libsvn_wc/lock.c
>> ===================================================================
>> --- subversion/libsvn_wc/lock.c (revision 28822)
>> +++ subversion/libsvn_wc/lock.c (working copy)
>> @@ -542,6 +542,9 @@
>>  /* This is essentially the guts of svn_wc_adm_open3, with the additional
>>   * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
>>   * admin directory during initial creation.
>> + *
>> + * If the working copy is already locked, return SVN_ERR_WC_LOCKED; if
>> + * it is not a versioned directory, return SVN_ERR_WC_NOT_DIRECTORY.
>>   */
>>  static svn_error_t *
>>  do_open(svn_wc_adm_access_t **adm_access,
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>>
>
>
>
> -- 
> David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

Re: [PATCH] allow unversioned parent for copy src again (was: Re: Functionality regression from 1.4->1.5 in svn copy due to --use-merge-history changes)

Posted by David Glasser <gl...@davidglasser.net>.
Looks good to me, though a test would be nice.

--dave

On Jan 9, 2008 1:14 PM, Karl Fogel <kf...@red-bean.com> wrote:
> Chris Rose <ch...@messagingdirect.com> writes:
> > I'm sorry to prod on this, but I'm quite concerned about the change in
> > functionality, and would like to find out if this will remain in place
> > for 1.5, or if I should register it as a bug with the client?
> >
> > Can somebody weigh in either way, please?
>
> It sounds like a bug to me.  I've filed an issue for it, and written a
> tentative patch.  Here's the issue:
>
>    http://subversion.tigris.org/issues/show_bug.cgi?id=3068
>
> The patch is below.  I'd love some other eyes on it before I commit,
> though, from people more familiar with wc mergeinfo propagation.
>
> Thanks for the reproduction script, btw, that helped a lot.
>
> Here's the patch:
>
> [[[
> Fix issue #3068: wc->wc copy should work when src parent is unversioned.
>
> * subversion/libsvn_client/copy.c
>   (do_wc_to_wc_copies): Propagate mergeinfo from parent only if possible.
>
> * subversion/libsvn_wc/lock.c
>   (do_open): Document an error return that callers depend on.
> ]]]
>
> Index: subversion/libsvn_client/copy.c
> ===================================================================
> --- subversion/libsvn_client/copy.c     (revision 28822)
> +++ subversion/libsvn_client/copy.c     (working copy)
> @@ -347,10 +347,18 @@
>          }
>        else
>          {
> -          SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
> -                                   pair->src_kind == svn_node_dir ? -1 : 0,
> -                                   ctx->cancel_func, ctx->cancel_baton,
> -                                   iterpool));
> +          err = svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
> +                                 pair->src_kind == svn_node_dir ? -1 : 0,
> +                                 ctx->cancel_func, ctx->cancel_baton,
> +                                 iterpool);
> +          /* The parent of a copy src might not be versioned at all. */
> +          if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> +            {
> +              src_access = NULL;
> +              svn_error_clear(err);
> +              err = NULL;
> +            }
> +          SVN_ERR(err);
>          }
>
>        /* Perform the copy */
> @@ -365,12 +373,13 @@
>        if (err)
>          break;
>
> -      err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
> -                                          ctx, pool);
> +      if (src_access)
> +        err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
> +                                            ctx, pool);
>        if (err)
>          break;
>
> -      if (src_access != dst_access)
> +      if (src_access && src_access != dst_access)
>          SVN_ERR(svn_wc_adm_close(src_access));
>      }
>
> Index: subversion/libsvn_wc/lock.c
> ===================================================================
> --- subversion/libsvn_wc/lock.c (revision 28822)
> +++ subversion/libsvn_wc/lock.c (working copy)
> @@ -542,6 +542,9 @@
>  /* This is essentially the guts of svn_wc_adm_open3, with the additional
>   * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
>   * admin directory during initial creation.
> + *
> + * If the working copy is already locked, return SVN_ERR_WC_LOCKED; if
> + * it is not a versioned directory, return SVN_ERR_WC_NOT_DIRECTORY.
>   */
>  static svn_error_t *
>  do_open(svn_wc_adm_access_t **adm_access,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>



-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

[PATCH] allow unversioned parent for copy src again (was: Re: Functionality regression from 1.4->1.5 in svn copy due to --use-merge-history changes)

Posted by Karl Fogel <kf...@red-bean.com>.
Chris Rose <ch...@messagingdirect.com> writes:
> I'm sorry to prod on this, but I'm quite concerned about the change in
> functionality, and would like to find out if this will remain in place
> for 1.5, or if I should register it as a bug with the client?
>
> Can somebody weigh in either way, please?

It sounds like a bug to me.  I've filed an issue for it, and written a
tentative patch.  Here's the issue:

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

The patch is below.  I'd love some other eyes on it before I commit,
though, from people more familiar with wc mergeinfo propagation.

Thanks for the reproduction script, btw, that helped a lot.

Here's the patch:

[[[
Fix issue #3068: wc->wc copy should work when src parent is unversioned.

* subversion/libsvn_client/copy.c
  (do_wc_to_wc_copies): Propagate mergeinfo from parent only if possible.

* subversion/libsvn_wc/lock.c
  (do_open): Document an error return that callers depend on.
]]]

Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c	(revision 28822)
+++ subversion/libsvn_client/copy.c	(working copy)
@@ -347,10 +347,18 @@
         }
       else
         {
-          SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
-                                   pair->src_kind == svn_node_dir ? -1 : 0,
-                                   ctx->cancel_func, ctx->cancel_baton,
-                                   iterpool));
+          err = svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE,
+                                 pair->src_kind == svn_node_dir ? -1 : 0,
+                                 ctx->cancel_func, ctx->cancel_baton,
+                                 iterpool);
+          /* The parent of a copy src might not be versioned at all. */
+          if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
+            {
+              src_access = NULL;
+              svn_error_clear(err);
+              err = NULL;
+            }
+          SVN_ERR(err);
         }
 
       /* Perform the copy */
@@ -365,12 +373,13 @@
       if (err)
         break;
 
-      err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
-                                          ctx, pool);
+      if (src_access)
+        err = propagate_mergeinfo_within_wc(pair, src_access, dst_access,
+                                            ctx, pool);
       if (err)
         break;
 
-      if (src_access != dst_access)
+      if (src_access && src_access != dst_access)
         SVN_ERR(svn_wc_adm_close(src_access));
     }
 
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c	(revision 28822)
+++ subversion/libsvn_wc/lock.c	(working copy)
@@ -542,6 +542,9 @@
 /* This is essentially the guts of svn_wc_adm_open3, with the additional
  * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
  * admin directory during initial creation.
+ *
+ * If the working copy is already locked, return SVN_ERR_WC_LOCKED; if 
+ * it is not a versioned directory, return SVN_ERR_WC_NOT_DIRECTORY.
  */
 static svn_error_t *
 do_open(svn_wc_adm_access_t **adm_access,

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

Re: Functionality regression from 1.4->1.5 in svn copy due to --use-merge-history changes

Posted by Chris Rose <ch...@messagingdirect.com>.
I'm sorry to prod on this, but I'm quite concerned about the change in 
functionality, and would like to find out if this will remain in place 
for 1.5, or if I should register it as a bug with the client?

Can somebody weigh in either way, please?

Chris Rose wrote:
> As a consequence of some changes associated with revsion r26495 in
> libsvn_client/copy.c something that was possible in svn 1.4 can no
> longer be accomplished in svn 1.5.  This might be intentional, but I
> suspect it was an accidental casualty:
> 
> In 1.4, this sequence of operations is possible:
> - check out a module 'm1' into non-wc directory 'base'
> - cwd to a directory completely outside of 'base' that is a working copy
> for another path in the same repository as 'm1', above.
> - svn cp 'base/m1' `pwd`
> 
> In 1.5, this sequence of operations fails with this message:
> # svn: 'base' is not a working copy
> # svn: Can't open file 'base/.svn/entries': No such file or directory
> 
> I don't know the propagate_mergeinfo_within_wc method well enough to say
> what it's doing with src_access (acquired by calling svn_wc_adm_open3 on
> 'base' in my example) or to patch the code to change the behaviour.
> 
> My question is:  is it necessary to get the parent directory's metadata
> if it's not a working copy?  Perhaps somebody more involved in the
> --use-merge-history work could comment, please?
> 
> Attached is a script that -- with the substitution of the correct paths
> for svn 1.4 and 1.4 binaries -- will demonstrate the issue directly.
> 
> Thank you for your time.
> 

-- 
Chris Rose
Developer    Planet Consulting Group
(780) 577-8433
crose@planetci.com