| Submitter | Bryan O'Sullivan |
|---|---|
| Date | March 19, 2013, 11 p.m. |
| Message ID | <9a44c44337c701d62124.1363734037@australite.thefacebook.com> |
| Download | mbox | patch |
| Permalink | /patch/1139/ |
| State | Accepted |
| Commit | ed39a8f94e957455ae49faad040fe35154a36dba |
| Headers | show |
Comments
On 3/19/13 4:00 PM, "Bryan O'Sullivan" <bos@serpentine.com> wrote: ># HG changeset patch ># User Bryan O'Sullivan <bryano@fb.com> ># Date 1363734026 25200 ># Node ID 9a44c44337c701d6212454b993caa5f778254f56 ># Parent de0601436fb6156c173abc2d7a6099f0c0b6b7cb >blackbox: prevent failed I/O from causing hg to abort > >Instead, we simply print a warning message if opening the blackbox log >file fails, or if writing to it fails. Overall looks good. Should we even bother writing out a warning? I can imagine the case where the repo is readonly for most users, but still wants the blackbox on for admin users. In that case the normal users would see the warning every time right?
On Tue, Mar 19, 2013 at 4:50 PM, Durham Goode <durham@fb.com> wrote: > Overall looks good. Should we even bother writing out a warning? I can > imagine the case where the repo is readonly for most users, but still > wants the blackbox on for admin users. In that case the normal users > would see the warning every time right? > That's true. I'll change the use of warn to debug, so that if something goes wrong with logging, you can have *some* hope of finding out.
On Wed, Mar 20, 2013 at 1:23 PM, Bryan O'Sullivan <bos@serpentine.com>wrote: > > I'll change the use of warn to debug, so that if something goes wrong with > logging, you can have *some* hope of finding out. > Patches are now in crew, with your suggested changes. Thanks for the quick review.
Patch
diff --git a/hgext/blackbox.py b/hgext/blackbox.py --- a/hgext/blackbox.py +++ b/hgext/blackbox.py @@ -57,11 +57,20 @@ def wrapui(ui): date = util.datestr(None, '%Y/%m/%d %H:%M:%S') user = getpass.getuser() formattedmsg = msg[0] % msg[1:] - blackbox.write('%s %s> %s' % (date, user, formattedmsg)) + try: + blackbox.write('%s %s> %s' % (date, user, formattedmsg)) + except IOError, err: + self.warn(_('warning: cannot write to blackbox.log: %s\n') % + err.strerror) lastblackbox = blackbox def setrepo(self, repo): - self._blackbox = repo.opener('blackbox.log', 'a') + try: + self._blackbox = repo.opener('blackbox.log', 'a') + except IOError, err: + self.warn(_('warning: cannot write to blackbox.log: %s\n') % + err.strerror) + self._blackbox = None ui.__class__ = blackboxui diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t --- a/tests/test-blackbox.t +++ b/tests/test-blackbox.t @@ -61,6 +61,30 @@ clone, commit, pull 1970/01/01 00:00:00 bob> 1 incoming changes - new heads: d02f48003e62 1970/01/01 00:00:00 bob> pull exited None after * seconds (glob) +we must not cause a failure if we cannot write to the log + + $ hg rollback + repository tip rolled back to revision 1 (undo pull) + $ chmod 000 .hg/blackbox.log + $ hg pull + warning: cannot write to blackbox.log: Permission denied + pulling from $TESTTMP/blackboxtest + searching for changes + adding changesets + adding manifests + adding file changes + added 1 changesets with 1 changes to 1 files + (run 'hg update' to get a working copy) + +a failure reading from the log is fine + + $ hg blackbox -l 3 + warning: cannot write to blackbox.log: Permission denied + abort: Permission denied: $TESTTMP/blackboxtest2/.hg/blackbox.log + [255] + + $ chmod 600 .hg/blackbox.log + backup bundles get logged $ touch d