You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@wandisco.com> on 2011/10/13 17:33:30 UTC
Re: svn commit: r1182904 - /subversion/trunk/subversion/libsvn_wc/entries.c
On Thu, Oct 13, 2011 at 10:25 AM, <st...@apache.org> wrote:
> Author: stsp
> Date: Thu Oct 13 15:25:16 2011
> New Revision: 1182904
>
> URL: http://svn.apache.org/viewvc?rev=1182904&view=rev
> Log:
> * subversion/libsvn_wc/entries.c
> (write_entry): If a bad base MD5 checksum is found, return a proper error
> message instead of asserting. Since 1.7.0 was released an overwhelming
> amount of TortoiseSVN users keep reporting this assertion on users@.
> TortoiseSVN asks users to send reports about assertions, but doesn't do
> so for normal errors.
So the implication here is that we're just trying to hide this error
from ourselves?
-Hyrum
> ...
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1182904 -
/subversion/trunk/subversion/libsvn_wc/entries.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 13, 2011 at 11:20:36AM -0500, Hyrum K Wright wrote:
> I'm just thinking of a scenario where we'd had this in from the
> beginning. We likely would have missed out on the fact that this is
> an issue, and wouldn't have been able to attempt to fix it.
How so? Users might still have reported the problem.
In this case we hit the "copy-paste error into email and hit send"
reflex of some TSVN users. So this problem is reported a lot of times
and it stands out like a big red pimple on a nose. But, thankfully, those
aren't the only kinds of problem reports we get.
> And if the checksum is somehow mangled in 1.6, that may likely be a
> problem of our doing too.
I agree. But for all we know, it could just as well be some tool people
run which happens to modify text-bases. The reports don't provide enough
information about the source of the checksum error.
Re: svn commit: r1182904 - /subversion/trunk/subversion/libsvn_wc/entries.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Oct 13, 2011 at 11:12 AM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, Oct 13, 2011 at 10:33:30AM -0500, Hyrum K Wright wrote:
>> On Thu, Oct 13, 2011 at 10:25 AM, <st...@apache.org> wrote:
>> > Author: stsp
>> > Date: Thu Oct 13 15:25:16 2011
>> > New Revision: 1182904
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1182904&view=rev
>> > Log:
>> > * subversion/libsvn_wc/entries.c
>> > (write_entry): If a bad base MD5 checksum is found, return a proper error
>> > message instead of asserting. Since 1.7.0 was released an overwhelming
>> > amount of TortoiseSVN users keep reporting this assertion on users@.
>> > TortoiseSVN asks users to send reports about assertions, but doesn't do
>> > so for normal errors.
>>
>> So the implication here is that we're just trying to hide this error
>> from ourselves?
>>
>
> Not at all.
>
> The problem is that a base text in a 1.6 working copy somehow changed
> its MD5. There are various ways this can happen. There are also various
> ways to deal with the problem.
>
> One is to just assert(), which is what 1.7.0 does.
> This is the worst option in my opinion.
>
> Another is to send an error that points people at the bad file.
> That is what this commit is doing.
>
> Maybe it is also possible to make the upgrade code handle this condition
> gracefully and make the upgrade succeed. However, as a quick fix I
> prefer to print an error which points out the bad file. This is a very
> safe change we can easily backport to improve the status quo. Any
> additional enhancements to the upgrade code can come later but shouldn't
> block a change which improves the error message.
>
> Does this make sense?
Sure. I'm just thinking of a scenario where we'd had this in from the
beginning. We likely would have missed out on the fact that this is
an issue, and wouldn't have been able to attempt to fix it.
And if the checksum is somehow mangled in 1.6, that may likely be a
problem of our doing too.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1182904 -
/subversion/trunk/subversion/libsvn_wc/entries.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 13, 2011 at 10:33:30AM -0500, Hyrum K Wright wrote:
> On Thu, Oct 13, 2011 at 10:25 AM, <st...@apache.org> wrote:
> > Author: stsp
> > Date: Thu Oct 13 15:25:16 2011
> > New Revision: 1182904
> >
> > URL: http://svn.apache.org/viewvc?rev=1182904&view=rev
> > Log:
> > * subversion/libsvn_wc/entries.c
> > (write_entry): If a bad base MD5 checksum is found, return a proper error
> > message instead of asserting. Since 1.7.0 was released an overwhelming
> > amount of TortoiseSVN users keep reporting this assertion on users@.
> > TortoiseSVN asks users to send reports about assertions, but doesn't do
> > so for normal errors.
>
> So the implication here is that we're just trying to hide this error
> from ourselves?
>
Not at all.
The problem is that a base text in a 1.6 working copy somehow changed
its MD5. There are various ways this can happen. There are also various
ways to deal with the problem.
One is to just assert(), which is what 1.7.0 does.
This is the worst option in my opinion.
Another is to send an error that points people at the bad file.
That is what this commit is doing.
Maybe it is also possible to make the upgrade code handle this condition
gracefully and make the upgrade succeed. However, as a quick fix I
prefer to print an error which points out the bad file. This is a very
safe change we can easily backport to improve the status quo. Any
additional enhancements to the upgrade code can come later but shouldn't
block a change which improves the error message.
Does this make sense?