This is an archive of the discontinued Mercurial Phabricator instance.

Differential D768

blackbox: do not prevent 'chg init' from working
ClosedPublic

Authored by quark on Sep 21 2017, 2:13 PM.

Details

Reviewers
akushner
Group Reviewers
hg-reviewers
Commits
rHGb1d4ac068961: blackbox: do not prevent 'chg init' from working
Summary

Previously, blackbox always appends to blackbox.log and creates the
directory for that file on demand. That could be an issue if:

  1. chg starts from $REPO directory, so ui._bbrepo is set.
  2. rm -rf $REPO.
  3. chg init $REPO, blackbox writes something and init will fail because $REPO directory is non-empty.

This patch fixes that by verifying whether vfs exists before re-using it.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Sep 21 2017, 2:13 PM
akushner added a subscriber: akushner.Sep 21 2017, 4:28 PM
akushner added inline comments.
tests/test-blackbox.t
292

Is 'repo2' needed here?

quark added inline comments.Sep 21 2017, 4:50 PM
tests/test-blackbox.t
292

Nope. Good catch. I was using hg clone in test and then simplified it to init.

quark marked an inline comment as done.Sep 21 2017, 4:52 PM
quark updated this revision to Diff 1974.
akushner accepted this revision.Sep 21 2017, 5:15 PM
quark updated this revision to Diff 1975.Sep 21 2017, 5:49 PM
Closed by commit rHGb1d4ac068961: blackbox: do not prevent 'chg init' from working (authored by quark). · Explain WhySep 22 2017, 8:50 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/blackbox.py (10 lines)
Mtests/test-blackbox.t (8 lines)

Diff 1970

hgext/blackbox.py

) )
lastui = None lastui = None
def wrapui(ui): def wrapui(ui):
class blackboxui(ui.__class__): class blackboxui(ui.__class__):
@property @property
def _bbvfs(self): def _bbvfs(self):
vfs = None
repo = getattr(self, '_bbrepo', None) repo = getattr(self, '_bbrepo', None)
if repo: if repo:
return repo.vfs vfs = repo.vfs
if not vfs.isdir('.'):
vfs = None
return vfs
@util.propertycache @util.propertycache
def track(self): def track(self):
return self.configlist('blackbox', 'track', ['*']) return self.configlist('blackbox', 'track', ['*'])
def _openlogfile(self): def _openlogfile(self):
def rotate(oldpath, newpath): def rotate(oldpath, newpath):
try: try:
else: else:
# certain ui instances exist outside the context of # certain ui instances exist outside the context of
# a repo, so just default to the last blackbox that # a repo, so just default to the last blackbox that
# was seen. # was seen.
ui = lastui ui = lastui
if not ui: if not ui:
return return
vfs = ui._bbvfs
if not vfs:
return
repo = getattr(ui, '_bbrepo', None) repo = getattr(ui, '_bbrepo', None)
if not lastui or repo: if not lastui or repo:
lastui = ui lastui = ui
if getattr(ui, '_bbinlog', False): if getattr(ui, '_bbinlog', False):
# recursion and failure guard # recursion and failure guard
return return
ui._bbinlog = True ui._bbinlog = True
default = self.configdate('devel', 'default-date') default = self.configdate('devel', 'default-date')

tests/test-blackbox.t

$ $PYTHON showsize.py .hg/blackbox* $ $PYTHON showsize.py .hg/blackbox*
.hg/blackbox.log: < 500 .hg/blackbox.log: < 500
.hg/blackbox.log.1: >= 500 .hg/blackbox.log.1: >= 500
.hg/blackbox.log.2: >= 500 .hg/blackbox.log.2: >= 500
$ cd .. $ cd ..
With chg, blackbox should not log into other repos
$ hg init repo1
$ hg init repo2
akushnerUnsubmitted
Done

Is 'repo2' needed here?

akushner: Is 'repo2' needed here?
quarkAuthorUnsubmitted
Not Done

Nope. Good catch. I was using hg clone in test and then simplified it to init.

quark: Nope. Good catch. I was using `hg clone` in test and then simplified it to `init`.
$ hg --config extensions.a=! -R repo1 log
$ rm -rf $TESTTMP/repo1
$ hg --config extensions.a=! init repo1
#endif #endif
blackbox should work if repo.ui.log is not called (issue5518) blackbox should work if repo.ui.log is not called (issue5518)
$ cat > $TESTTMP/raise.py << EOF $ cat > $TESTTMP/raise.py << EOF
> from __future__ import absolute_import > from __future__ import absolute_import
> from mercurial import registrar, scmutil > from mercurial import registrar, scmutil
> cmdtable = {} > cmdtable = {}