You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2004/03/09 12:55:10 UTC
PATCH: maybe fix something?
Should this patch fix something? It makes no difference to the regression tests. I find it hard to believe that the original was right, though.
It is in this file's "delta_dirs" function.
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 8948)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -737,7 +737,7 @@
t_entry = apr_hash_get (t_entries, name, APR_HASH_KEY_STRING);
s_fullpath = s_path ? svn_path_join (s_path, name, subpool) : NULL;
s_entry = s_entries ?
- apr_hash_get (t_entries, name, APR_HASH_KEY_STRING) : NULL;
+ apr_hash_get (s_entries, name, APR_HASH_KEY_STRING) : NULL;
SVN_ERR (update_entry (b, s_rev, s_fullpath, s_entry, t_fullpath,
t_entry, dir_baton, e_fullpath, info,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Ben Collins-Sussman <su...@collab.net>.
On Wed, 2004-03-10 at 13:08, Julian Foad wrote:
> Will do. I'd like to include a regression test, so give me a little while. And of course I'll credit you for figuring it out.
You two guys are amazing! :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Wed, 2004-03-10 at 13:17, Greg Hudson wrote:
>
>>On Wed, 2004-03-10 at 13:06, Greg Hudson wrote:
>>
>>>Fascinating. Yes, it should fix something. I'm surprised the world
>>>doesn't come crashing down as a result of this bug, actually
>>
>>Okay, I figured out why: in this particular case, the value of s_entry
>>is actually never used, because there's always some report information
>>which takes precedence.
>
> See Greg. See Greg vacillate.
:-)
> There's a somewhat unusual corner case where what I said isn't true.
> Say you have a working dir with the top level at rev 1, with foo at rev
[...]
What fun! Thanks for investigating; I'm glad something good came of it.
> So, Julian, please go ahead and commit your patch. Sorry for the
> confusion.
Will do. I'd like to include a regression test, so give me a little while. And of course I'll credit you for figuring it out.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Wed, 2004-03-10 at 13:17, Greg Hudson wrote:
>
> Say you have a working dir with the top level at rev 1, with foo at rev
> 1, and with foo/bar at rev 2. The report will involve a
> set_path("foo/bar", 2). In this case, in the loop in question,
> fetch_path_info() will return "foo" with info set to NULL, which means
> "I have no overriding report information for foo, but I do for a child
> of foo". In this case, s_entry is not overridden, so we can't just pass
> NULL, and passing the wrong value as we currently do could cause
> problems. But it is not surprising that the test suite does not cover
> this case.
Well, it does cover this case now :-)
> So, Julian, please go ahead and commit your patch. Sorry for the
> confusion.
Committed in r8976. Thanks for your help.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2004-03-10 at 13:17, Greg Hudson wrote:
> On Wed, 2004-03-10 at 13:06, Greg Hudson wrote:
> > Fascinating. Yes, it should fix something. I'm surprised the world
> > doesn't come crashing down as a result of this bug, actually
>
> Okay, I figured out why: in this particular case, the value of s_entry
> is actually never used, because there's always some report information
> which takes precedence.
See Greg. See Greg vacillate.
There's a somewhat unusual corner case where what I said isn't true.
Say you have a working dir with the top level at rev 1, with foo at rev
1, and with foo/bar at rev 2. The report will involve a
set_path("foo/bar", 2). In this case, in the loop in question,
fetch_path_info() will return "foo" with info set to NULL, which means
"I have no overriding report information for foo, but I do for a child
of foo". In this case, s_entry is not overridden, so we can't just pass
NULL, and passing the wrong value as we currently do could cause
problems. But it is not surprising that the test suite does not cover
this case.
So, Julian, please go ahead and commit your patch. Sorry for the
confusion.
Reproduction recipe, in case anyone wants to make a test case:
<create an empty repos, check it out, and cd into the wc>
svn mkdir dir
svn mkdir dir/subdir # Could be a file, doesn't matter
svn ci -m logmsg
svn rm dir
svn ci -m logmsg
svn up -r1
svn up -r2 dir/subdir
svn up
With the current code, you get the error:
svn: Working copy path 'dir' does not exist in repository
With Julian's patch, the final update works fine.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2004-03-10 at 13:06, Greg Hudson wrote:
> Fascinating. Yes, it should fix something. I'm surprised the world
> doesn't come crashing down as a result of this bug, actually
Okay, I figured out why: in this particular case, the value of s_entry
is actually never used, because there's always some report information
which takes precedence. So it would be better to just always pass
NULL. I'm going to make sure that passes the test suite and then commit
it, so there's no need for Julian to commit his patch.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: maybe fix something?
Posted by Greg Hudson <gh...@MIT.EDU>.
Julian Foad writes:
> Should this patch fix something? It makes no difference to the
> regression tests. I find it hard to believe that the original was
> right, though.
Fascinating. Yes, it should fix something. I'm surprised the world
doesn't come crashing down as a result of this bug, actually; I'd
think simple cases like "update through a deletion of a file or
directory" would result in an erroneous complaint from update_entry().
I'm going to look into why so many things still work. Feel free to
commit the patch as long as it passes the test suite.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org