You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by John Szakmeister <jo...@szakmeister.net> on 2003/12/20 12:28:00 UTC
Re: 301 error on merge
On Sunday 14 December 2003 20:27, Ross Mark wrote:
> John Szakmeister wrote:
> >Can you post a script that causes this error? I tried following what was
> >listed in the mail yesterday and couldn't get it to fail. It would be
> > useful in tracking this problem down and getting rid of it. :-)
> >
> >-John
>
> I've attached a log for a complete session which shows the problem.
Sorry it's taken me so long to get back to you... I've had a couple of other
fish to fry, and my company just got bought out, which has left me little
time in the past couple of weeks.
Thanks for posting a script to repeat the problem. What's happening is that
in the case that the target is '.', it is converted to "" (an empty string).
Well, it turns out that in some other logic in svn_cl__merge() that "" is
being used as a flag to indicate 'find the wc path for me'. What ends up
happening is the following code snippet pulls the last part of the URL,
'prodA' and looks for that in the current directory. It, of course, finds a
file called 'prodA' and then passes that into svn_client_merge() as the
target.
if (! strcmp (targetpath, ""))
{
char *sp1_basename, *sp2_basename;
sp1_basename = svn_path_basename (sourcepath1, pool);
sp2_basename = svn_path_basename (sourcepath2, pool);
if (! strcmp (sp1_basename, sp2_basename))
{
svn_node_kind_t kind;
SVN_ERR (svn_io_check_path (sp1_basename, &kind, pool));
if (kind == svn_node_file)
{
targetpath = sp1_basename;
}
}
}
It appears we need to do some better checking here, but I'm not sure how to
proceed with this since it has some impact on how the command line client can
be used. Therefore, I've moved this to the dev@ list to get some more input
from other developers. I currently have a small patch that adds an explicit
target flag to make sure this doesn't happen even if target_path is "". The
problem is that 'svn merge -r X:Y url::/to/a/file ." will no longer work.
How does the rest of the community feel about that? Any other suggestions on
how to fix this problem?
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: 301 error on merge
Posted by John Szakmeister <jo...@szakmeister.net>.
On Thursday 01 January 2004 12:24, Greg Hudson wrote:
> On Thu, 2004-01-01 at 09:47, John Szakmeister wrote:
> > > ["If source1 and source2 are files with the same basename, then..."]
>
> Ah, I understand now. The actual test is:
>
> If source1 and source2 have the same basename and that basename
> maps to a file in the wc, then use that basename as the implicit
> target.
>
> which is a terrible test. I can see why you might want to move it into
> libsvn_client, where one could test the URLs rather than the working
> copy.
Yep. But I think even if we move the test into libsvn_client, that we still
face some problems. We can never be absolutely sure what the target of the
merge operation is supposed to be, unless it is specified.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: 301 error on merge
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2004-01-01 at 09:47, John Szakmeister wrote:
> > ["If source1 and source2 are files with the same basename, then..."]
Ah, I understand now. The actual test is:
If source1 and source2 have the same basename and that basename
maps to a file in the wc, then use that basename as the implicit
target.
which is a terrible test. I can see why you might want to move it into
libsvn_client, where one could test the URLs rather than the working
copy.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: 301 error on merge
Posted by John Szakmeister <jo...@szakmeister.net>.
On Thursday 01 January 2004 07:41, Greg Hudson wrote:
> On Mon, 2003-12-29 at 18:38, John Szakmeister wrote:
> > On Saturday 20 December 2003 07:28, John Szakmeister wrote:
> > > if (! strcmp (targetpath, ""))
> > > {
>
> ["If source1 and source2 are files with the same basename, then..."]
>
> > > targetpath = sp1_basename;
> >
> > The current algorithm doesn't always pick the correct target...
>
> I haven't seen the users thread; can you elaborate on that? It would
> seem to depend on what "correct" is.
Take a look at:
http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=3816.
In summary, if the URLs point to a directory (i.e., the goal here is merge
changes at a directory level), but that directory happens to have the same
name as a file in that directory and you haven't specified the target
explicitly, then this algorithm fails.
> > At the very least, I think that if I put '.' as the target for the
> > merge operation on the command line, that we should treat that as an
> > explicit target and not try to compute an implicit target.
>
> Agreed, but...
>
> > I think this gives users a more reasonable workaround than having to
> > type 'svn merge URL1 URL2 ../name_of_current_directory'.
>
> I'm missing something. If URL1 and URL2 are both files, when would this
> ever produce anything but an error?
The problem is that if the merge fails like it did in user thread, and you
type 'svn merge URL1 URL2 .', the '.' is treated as an implicit target rather
than an explicit one and we try to compute the target of the merge operation
exactly the same way as if we had typed 'svn merge URL1 URL2'. I tend to
think that if I explicity tell you the target of the merge, that's what you
should try to do. Unfortunately, in the case you have to use a command like
'svn merge URL1 URL2 ../name_of_current_directory' to prevent the command
line client from attempting to compute the target of the merge.
> (It certainly seems more appropriate to generate an error on "svn merge
> url-of-file1 url-of-file2 ." than it is to try to merge into
> basename-of-file1, so I agree that it's a bug. But more of an
> "unpredictable behavior in the corner case" bug than a usability bug.)
I guess I see 2 bugs. "svn merge url-of-file1 url-of-file2 ." treat my
explicit target the same as if I had not specified it at all. And I see the
case where the basename of the URL you're using matches the name of a file in
your WC and the target of the merge is the directory, but we erroneously
compute the target as being the file instead of the current directory.
I hope that helps in understanding the problem a little more.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: 301 error on merge
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2003-12-29 at 18:38, John Szakmeister wrote:
> On Saturday 20 December 2003 07:28, John Szakmeister wrote:
> > if (! strcmp (targetpath, ""))
> > {
["If source1 and source2 are files with the same basename, then..."]
> > targetpath = sp1_basename;
> The current algorithm doesn't always pick the correct target...
I haven't seen the users thread; can you elaborate on that? It would
seem to depend on what "correct" is.
> At the very least, I think that if I put '.' as the target for the
> merge operation on the command line, that we should treat that as an explicit
> target and not try to compute an implicit target.
Agreed, but...
> I think this gives users a more reasonable workaround than having to
> type 'svn merge URL1 URL2 ../name_of_current_directory'.
I'm missing something. If URL1 and URL2 are both files, when would this
ever produce anything but an error?
(It certainly seems more appropriate to generate an error on "svn merge
url-of-file1 url-of-file2 ." than it is to try to merge into
basename-of-file1, so I agree that it's a bug. But more of an
"unpredictable behavior in the corner case" bug than a usability bug.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: 301 error on merge
Posted by John Szakmeister <jo...@szakmeister.net>.
On Saturday 20 December 2003 07:28, John Szakmeister wrote:
> [snip]
>
> if (! strcmp (targetpath, ""))
> {
> char *sp1_basename, *sp2_basename;
> sp1_basename = svn_path_basename (sourcepath1, pool);
> sp2_basename = svn_path_basename (sourcepath2, pool);
>
> if (! strcmp (sp1_basename, sp2_basename))
> {
> svn_node_kind_t kind;
> SVN_ERR (svn_io_check_path (sp1_basename, &kind, pool));
> if (kind == svn_node_file)
> {
> targetpath = sp1_basename;
> }
> }
> }
>
> It appears we need to do some better checking here, but I'm not sure how to
> proceed with this since it has some impact on how the command line client
> can be used. Therefore, I've moved this to the dev@ list to get some more
> input from other developers. I currently have a small patch that adds an
> explicit target flag to make sure this doesn't happen even if target_path
> is "". The problem is that 'svn merge -r X:Y url::/to/a/file ." will no
> longer work. How does the rest of the community feel about that? Any other
> suggestions on how to fix this problem?
I've looked at this problem a little more, and I've come to the following
conclusion: I think we need to drop support for implicit targets. The
current algorithm doesn't always pick the correct target, and I don't think
that it can always be made to Do The Right Thing, at least not from the
command line client front-end (which is currently where the target is
computed). I've toyed with the idea of moving this process into
svn_client_merge() and I don't think that would be any better. We could at
least verify that the source URLs point to object of the same type as the
target (we should do this anyways) and try a different approach if they don't
match, but I think we'll still end up with the same (or at least similar)
problem. At the very least, I think that if I put '.' as the target for the
merge operation on the command line, that we should treat that as an explicit
target and not try to compute an implicit target. I think this gives users a
more reasonable workaround than having to type 'svn merge URL1 URL2 ../
name_of_current_directory'.
Does anyone else have some input? Should I file an issue for this problem?
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org