On Sat, Apr 15, 2017 at 8:41 PM, Yuya Nishihara <y...@tcha.org> wrote:
> On Fri, 14 Apr 2017 00:44:10 -0700, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc <gregory.sz...@gmail.com> > > # Date 1492155656 25200 > > # Fri Apr 14 00:40:56 2017 -0700 > > # Node ID d1b31c378dc8e29d9827f9ded9fd023d5a00b0c9 > > # Parent 8525abda8397f2a5a94edc5db2279549ef53b8e8 > > bundle2: don't attempt I/O after an I/O error occurs (issue4784) > > > A subtle bug with this change is that *all* I/O errors will influence > > behavior, not just I/O errors on the bundle2 stream. e.g. an I/O error > > from the local filesystem could result in not seeking the bundle. This > > is the source of the test-acl.t test change, for example. I question > > whether we care. Considering pretty much all bundle2 stream I/O errors > > resulted in this code not running before (due to a fast I/O error > > re-raise), we seem to be getting along just fine without the code. And > > since this error recovery code is only tested in test-acl.t, I'm > > guessing it isn't important. Or, at least it isn't as important as > > accurately reporting the original I/O error. Considering these "stream > > ended unexpectedly" errors have been masking several problems, I > > think the new code is acceptable. We can always improve the stream > > error detection code later if the new model isn't good enough. > > > diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py > > --- a/mercurial/bundle2.py > > +++ b/mercurial/bundle2.py > > @@ -354,9 +354,13 @@ def processbundle(repo, unbundler, trans > > for nbpart, part in iterparts: > > _processpart(op, part) > > except Exception as exc: > > - for nbpart, part in iterparts: > > - # consume the bundle content > > - part.seek(0, 2) > > + # Don't attempt further I/O after we've encountered an I/O > failure. > > + # This will almost certainly just raise another I/O failure and > will > > + # only mask the underlying failure. > > + if not isinstance(exc, (IOError, error.RichIOError)): > > + for nbpart, part in iterparts: > > + # consume the bundle content > > + part.seek(0, 2) > > I think this is okay, but if we have to care about a local IOError, maybe > we > could try consuming bundle and suppress errors? > > except Exception as exc: > try: > # consume the bundle content > except (IOError, error.RichIOError): > pass > I went with something similar to this in the series I just sent. But it is different enough that we may want to discuss it further.
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
