You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by pb...@apache.org on 2013/01/09 23:04:24 UTC
svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Author: pburba
Date: Wed Jan 9 22:04:24 2013
New Revision: 1431114
URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
Log:
Fix issue #4139 'Subversion cannot perform merge if there's a file with
the same name as directory'.
* subversion/svn/merge-cmd.c
(svn_cl__merge): If the basename of the source is the same as the
basename of the current working directory, then assume the cwd is the
target.
Modified:
subversion/trunk/subversion/svn/merge-cmd.c
Modified: subversion/trunk/subversion/svn/merge-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/merge-cmd.c?rev=1431114&r1=1431113&r2=1431114&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/merge-cmd.c (original)
+++ subversion/trunk/subversion/svn/merge-cmd.c Wed Jan 9 22:04:24 2013
@@ -386,9 +386,9 @@ svn_cl__merge(apr_getopt_t *os,
if (sourcepath1 && sourcepath2 && strcmp(targetpath, "") == 0)
{
/* If the sourcepath is a URL, it can only refer to a target in
- the current working directory. However, if the sourcepath is
- a local path, it can refer to a target somewhere deeper in
- the directory structure. */
+ the current working directory or which is the current working
+ directory. However, if the sourcepath is a local path, it can
+ refer to a target somewhere deeper in the directory structure. */
if (svn_path_is_url(sourcepath1))
{
const char *sp1_basename = svn_uri_basename(sourcepath1, pool);
@@ -396,12 +396,29 @@ svn_cl__merge(apr_getopt_t *os,
if (strcmp(sp1_basename, sp2_basename) == 0)
{
- svn_node_kind_t kind;
+ const char *target_url;
+ const char *target_base;
- SVN_ERR(svn_io_check_path(sp1_basename, &kind, pool));
- if (kind == svn_node_file)
+ SVN_ERR(svn_client_url_from_path2(&target_url, targetpath, ctx,
+ pool, pool));
+ target_base = svn_uri_basename(target_url, pool);
+
+ /* If the basename of the source is the same as the basename of
+ the cwd assume the cwd is the target. */
+ if (strcmp(sp1_basename, target_base) != 0)
{
- targetpath = sp1_basename;
+ svn_node_kind_t kind;
+
+ /* If the basename of the source differs from the basename
+ of the target. We still might assume the cwd is the
+ target, but first check if there is a file in the cwd
+ with the same name as the source basename. If there is,
+ then assume that file is the target. */
+ SVN_ERR(svn_io_check_path(sp1_basename, &kind, pool));
+ if (kind == svn_node_file)
+ {
+ targetpath = sp1_basename;
+ }
}
}
}
RE: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Paul Burba [mailto:ptburba@gmail.com]
> Sent: donderdag 10 januari 2013 20:26
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1431114 -
> /subversion/trunk/subversion/svn/merge-cmd.c
>
> On Thu, Jan 10, 2013 at 2:15 PM, Bert Huijben <be...@qqmail.nl> wrote:
> >
> >> -----Original Message-----
> >> From: Paul Burba [mailto:ptburba@gmail.com]
> >> Sent: donderdag 10 januari 2013 19:59
> >> To: Bert Huijben
> >> Cc: dev@subversion.apache.org
> >> Subject: Re: svn commit: r1431114 -
> >> /subversion/trunk/subversion/svn/merge-cmd.c
> >>
> >> On Thu, Jan 10, 2013 at 12:38 PM, Bert Huijben <be...@qqmail.nl> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: pburba@apache.org [mailto:pburba@apache.org]
> >> >> Sent: woensdag 9 januari 2013 23:04
> >> >> To: commits@subversion.apache.org
> >> >> Subject: svn commit: r1431114 -
> >> /subversion/trunk/subversion/svn/merge-
> >> >> cmd.c
> >> >>
> >> >> Author: pburba
> >> >> Date: Wed Jan 9 22:04:24 2013
> >> >> New Revision: 1431114
> >> >>
> >> >> URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
> >> >> Log:
> >> >> Fix issue #4139 'Subversion cannot perform merge if there's a file with
> >> >> the same name as directory'.
> >> >>
> >> >> * subversion/svn/merge-cmd.c
> >> >> (svn_cl__merge): If the basename of the source is the same as the
> >> >> basename of the current working directory, then assume the cwd is
> the
> >> >> target.
> >> >
> >> > I never heard of and/or noticed this behavior
> >>
> >> Hi Bert,
> >>
> >> Which behavior are you referring to: The old behavior, the bug with
> >> the old behavior, or the new behavior?
> >>
> >> The old behavior was this:
> >>
> >> 'svn merge ^/src/base-name .' and 'svn merge ^src/base-name' both
> used
> >> the cwd at the merge target *unless* there is a file in the cwd with
> >> the same name as the source basename. In that case the file was the
> >> merge target.
> >
> > The question I asked was, shouldn't we make that first case think the "." is
> an explicit target?
> >
> > "I tell svn where to merge to and I would have guessed that it just did what
> I asked."
>
> Ok, seems you are saying what I said before:
>
> > Merge target: "." --> No guessing, the target is *always* the cwd.
> >
> > Merge target: "" --> Target guessing heuristics apply.
>
> I'm good with that. If nobody objects I can make that change.
>
> > That we want to be smart for the second case is a different question, but
> my preference would be that this explicit '.' case would be handled as the
> plain api.
>
> Mildly puzzled: Which API do you mean? Everything we are talking
> about is in svn_cl__merge(), prior to calling any public APIs. I
> think I know what you mean...but I'm not entirely sure :-)
Answering your puzzle:
I always ask the SharpSvn users to try and reproduce what they try to do with 'svn'.
In almost every case 'svn' just wraps 'svn_client.h' with only some minor changes and so does SharpSvn.
In this case it appears to do more than a bit of wrapping (since somewhere in 2003), and I don't think what it does is obvious to the user either.
But I can certainly see that this guessing of a target would be useful.
Bert
Re: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Jan 10, 2013 at 2:15 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>> -----Original Message-----
>> From: Paul Burba [mailto:ptburba@gmail.com]
>> Sent: donderdag 10 januari 2013 19:59
>> To: Bert Huijben
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1431114 -
>> /subversion/trunk/subversion/svn/merge-cmd.c
>>
>> On Thu, Jan 10, 2013 at 12:38 PM, Bert Huijben <be...@qqmail.nl> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: pburba@apache.org [mailto:pburba@apache.org]
>> >> Sent: woensdag 9 januari 2013 23:04
>> >> To: commits@subversion.apache.org
>> >> Subject: svn commit: r1431114 -
>> /subversion/trunk/subversion/svn/merge-
>> >> cmd.c
>> >>
>> >> Author: pburba
>> >> Date: Wed Jan 9 22:04:24 2013
>> >> New Revision: 1431114
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
>> >> Log:
>> >> Fix issue #4139 'Subversion cannot perform merge if there's a file with
>> >> the same name as directory'.
>> >>
>> >> * subversion/svn/merge-cmd.c
>> >> (svn_cl__merge): If the basename of the source is the same as the
>> >> basename of the current working directory, then assume the cwd is the
>> >> target.
>> >
>> > I never heard of and/or noticed this behavior
>>
>> Hi Bert,
>>
>> Which behavior are you referring to: The old behavior, the bug with
>> the old behavior, or the new behavior?
>>
>> The old behavior was this:
>>
>> 'svn merge ^/src/base-name .' and 'svn merge ^src/base-name' both used
>> the cwd at the merge target *unless* there is a file in the cwd with
>> the same name as the source basename. In that case the file was the
>> merge target.
>
> The question I asked was, shouldn't we make that first case think the "." is an explicit target?
>
> "I tell svn where to merge to and I would have guessed that it just did what I asked."
Ok, seems you are saying what I said before:
> Merge target: "." --> No guessing, the target is *always* the cwd.
>
> Merge target: "" --> Target guessing heuristics apply.
I'm good with that. If nobody objects I can make that change.
> That we want to be smart for the second case is a different question, but my preference would be that this explicit '.' case would be handled as the plain api.
Mildly puzzled: Which API do you mean? Everything we are talking
about is in svn_cl__merge(), prior to calling any public APIs. I
think I know what you mean...but I'm not entirely sure :-)
> (Our testsuite doesn't care: it only tests the second variant)
>
> Bert
>
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
RE: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Paul Burba [mailto:ptburba@gmail.com]
> Sent: donderdag 10 januari 2013 19:59
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1431114 -
> /subversion/trunk/subversion/svn/merge-cmd.c
>
> On Thu, Jan 10, 2013 at 12:38 PM, Bert Huijben <be...@qqmail.nl> wrote:
> >
> >
> >> -----Original Message-----
> >> From: pburba@apache.org [mailto:pburba@apache.org]
> >> Sent: woensdag 9 januari 2013 23:04
> >> To: commits@subversion.apache.org
> >> Subject: svn commit: r1431114 -
> /subversion/trunk/subversion/svn/merge-
> >> cmd.c
> >>
> >> Author: pburba
> >> Date: Wed Jan 9 22:04:24 2013
> >> New Revision: 1431114
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
> >> Log:
> >> Fix issue #4139 'Subversion cannot perform merge if there's a file with
> >> the same name as directory'.
> >>
> >> * subversion/svn/merge-cmd.c
> >> (svn_cl__merge): If the basename of the source is the same as the
> >> basename of the current working directory, then assume the cwd is the
> >> target.
> >
> > I never heard of and/or noticed this behavior
>
> Hi Bert,
>
> Which behavior are you referring to: The old behavior, the bug with
> the old behavior, or the new behavior?
>
> The old behavior was this:
>
> 'svn merge ^/src/base-name .' and 'svn merge ^src/base-name' both used
> the cwd at the merge target *unless* there is a file in the cwd with
> the same name as the source basename. In that case the file was the
> merge target.
The question I asked was, shouldn't we make that first case think the "." is an explicit target?
"I tell svn where to merge to and I would have guessed that it just did what I asked."
That we want to be smart for the second case is a different question, but my preference would be that this explicit '.' case would be handled as the plain api.
(Our testsuite doesn't care: it only tests the second variant)
Bert
Re: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Jan 10, 2013 at 12:38 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: pburba@apache.org [mailto:pburba@apache.org]
>> Sent: woensdag 9 januari 2013 23:04
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-
>> cmd.c
>>
>> Author: pburba
>> Date: Wed Jan 9 22:04:24 2013
>> New Revision: 1431114
>>
>> URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
>> Log:
>> Fix issue #4139 'Subversion cannot perform merge if there's a file with
>> the same name as directory'.
>>
>> * subversion/svn/merge-cmd.c
>> (svn_cl__merge): If the basename of the source is the same as the
>> basename of the current working directory, then assume the cwd is the
>> target.
>
> I never heard of and/or noticed this behavior
Hi Bert,
Which behavior are you referring to: The old behavior, the bug with
the old behavior, or the new behavior?
The old behavior was this:
'svn merge ^/src/base-name .' and 'svn merge ^src/base-name' both used
the cwd at the merge target *unless* there is a file in the cwd with
the same name as the source basename. In that case the file was the
merge target.
The problem in issue #4139 centered on the case where the base name of
the source, the base name of the cwd's repos path, and a file in the
cwd all share the same value:
1.7.x>svn ls -R ^^/
branches/
branches/1.0/
branches/1.0/foo/
branches/1.0/foo/foo
^^^
file 'foo' in dir 'foo'
trunk/
trunk/foo/
trunk/foo/foo
^^^
file 'foo' in dir 'foo'
1.7.x>svn info
Path: .
Working Copy Root Path:
C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\4139-1.7-WC
URL: http://localhost/svn-test-work/repositories/4139-REPOS/branches/1.0/foo
Repository Root: http://localhost/svn-test-work/repositories/4139-REPOS
Repository UUID: 3c30021b-156f-8648-ba21-a13a0af7285d
Revision: 4
Node Kind: directory
Schedule: normal
Last Changed Author: jrandom
Last Changed Rev: 2
Last Changed Date: 2013-01-10 13:34:55 -0500 (Thu, 10 Jan 2013)
What happened was that the aforementioned behavior would select the
*file* child as the merge target, resulting in this error:
1.7.x>svn merge -c4 ^^/trunk/foo
..\..\..\subversion\svn\util.c:913: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10955: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10909: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10909: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10879: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:8857: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:6775: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:5191: (apr_err=160017)
..\..\..\subversion\libsvn_ra_neon\fetch.c:711: (apr_err=160017)
svn: E160017: Can't get text contents of a directory
1.7.x>svn merge -c4 ^^/trunk/foo .
..\..\..\subversion\svn\util.c:913: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10955: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10909: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10909: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:10879: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:8857: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:6775: (apr_err=160017)
..\..\..\subversion\libsvn_client\merge.c:5191: (apr_err=160017)
..\..\..\subversion\libsvn_ra_neon\fetch.c:711: (apr_err=160017)
svn: E160017: Can't get text contents of a directory
The workaround is pretty simple, set the cwd to the parent of the
earlier target:
1.7.x>cd ..
1.7.x>svn merge -c4 ^^/trunk/foo 4139-1.7-WC
--- Merging r4 into '4139-1.7-WC':
U 4139-1.7-WC\foo
--- Recording mergeinfo for merge of r4 into '4139-1.7-WC':
U 4139-1.7-WC
However that obviously doesn't work if the cwd is the drive root. In
that case you'd have to switch the WC to the repos parent of
^^/branches/1.0/foo. But even that leaves the situation (albeit
pretty unlikely) that the WC root is the drive root *and* represents
the repos root. In that case you had no choice but to checkout (or
simply move) the WC to a non-drive root.
My change in r1431114 kept the earlier behavior of targeting a file in
the implied WC target if it had the same name as the basename in the
merge source, but before it got to that point it checks if the repos
path basename of the implied WC target is the same as the basename of
the merge source. In that case it selected the implied WC target
(i.e. the cwd) as the merge target:
1.8-dev>svn info
Path: .
Working Copy Root Path:
C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\4139-WC
URL: http://localhost/svn-test-work/repositories/4139-REPOS/branches/1.0/foo
Relative URL: ^/branches/1.0/foo
Repository Root: http://localhost/svn-test-work/repositories/4139-REPOS
Repository UUID: 3c30021b-156f-8648-ba21-a13a0af7285d
Revision: 4
Node Kind: directory
Schedule: normal
Last Changed Author: jrandom
Last Changed Rev: 2
Last Changed Date: 2013-01-10 13:34:55 -0500 (Thu, 10 Jan 2013)
1.8-dev>svn merge -c4 ^^/trunk/foo
--- Merging r4 into '.':
U foo
--- Recording mergeinfo for merge of r4 into '.':
U .
This change does add a wrinkle if the merge source is a file with the
same basename as the repos path of the cwd:
1.8-dev>svn revert -Rq .
1.8-dev>svn merge -c4 ^^/trunk/foo/foo
..\..\..\subversion\svn\util.c:553: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:11234: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:11200: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:9383: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:9088: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:8950: (apr_err=160005)
..\..\..\subversion\libsvn_client\merge.c:5366: (apr_err=160005)
..\..\..\subversion\libsvn_ra_serf\update.c:2870: (apr_err=160005)
..\..\..\subversion\libsvn_ra_serf\util.c:949: (apr_err=160005)
svn: E160005: Cannot replace a directory from within
But at least the workaround there is simple, we just need to
explicitly state the file target:
1.8-dev>svn merge -c4 ^^/trunk/foo/foo foo
--- Merging r4 into 'foo':
U foo
--- Recording mergeinfo for merge of r4 into 'foo':
U foo
> Should we also apply something like this patch to make sure that this 'guessing' is disabled if the user adds an explicit "." as last argument?
Possibly. I was trying to preserve as much of the old behavior as
possible, but maybe it makes sense to treat "." and "" slightly
differently, at least as far as the targeting heuristics go.
> The argument normalization functions would normalize the dot to "", which triggers this guessing behavior of where to apply, while I would say the user clearly provided the current directory as target.
Are you suggesting that:
Merge target: "." --> No guessing, the target is *always* the cwd.
Merge target: "" --> Target guessing heuristics apply.
?
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> [[
> * subversion/svn/merge.c
> (svn_cl__merge): Only guess the intended target if the user didn't provide a target, instead of also when the target is . or the empty string.
> ]]
>
> Bert
RE: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-cmd.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: pburba@apache.org [mailto:pburba@apache.org]
> Sent: woensdag 9 januari 2013 23:04
> To: commits@subversion.apache.org
> Subject: svn commit: r1431114 - /subversion/trunk/subversion/svn/merge-
> cmd.c
>
> Author: pburba
> Date: Wed Jan 9 22:04:24 2013
> New Revision: 1431114
>
> URL: http://svn.apache.org/viewvc?rev=1431114&view=rev
> Log:
> Fix issue #4139 'Subversion cannot perform merge if there's a file with
> the same name as directory'.
>
> * subversion/svn/merge-cmd.c
> (svn_cl__merge): If the basename of the source is the same as the
> basename of the current working directory, then assume the cwd is the
> target.
I never heard of and/or noticed this behavior
Should we also apply something like this patch to make sure that this 'guessing' is disabled if the user adds an explicit "." as last argument?
The argument normalization functions would normalize the dot to "", which triggers this guessing behavior of where to apply, while I would say the user clearly provided the current directory as target.
[[
* subversion/svn/merge.c
(svn_cl__merge): Only guess the intended target if the user didn't provide a target, instead of also when the target is . or the empty string.
]]
Bert