( )⚙ D5586 watchman: add verbose config knob

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5586

watchman: add verbose config knob
ClosedPublic

Authored by lothiraldan on Jan 15 2019, 11:57 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG18adb747a332: watchman: add verbose config knob
rHG158962bf4a52: watchman: add verbose config knob
Summary

This new config knob allows to silent watchman log and warning messages when
watchman is unavailable.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

lothiraldan created this revision.Jan 15 2019, 11:57 AM
indygreg added a subscriber: indygreg.Jan 16 2019, 2:40 PM

This new config knob defaults to off and we're therefore changing the behavior to not warn when watchman is unavailable. This is a bit concerning to me because someone could have fsmonitor enabled and I don't think they'd ever get an error if watchman isn't working correctly.

I support introducing the config knob. But I don't like silently suppressing watchman errors that could lead to false user expectations.

Is my understanding here correct?

(I would accept a patch that defaults the config option to on. I will continue reviewing this series.)

pulkit added a subscriber: pulkit.Jan 17 2019, 5:17 AM
In D5586#82915, @indygreg wrote:

This new config knob defaults to off and we're therefore changing the behavior to not warn when watchman is unavailable. This is a bit concerning to me because someone could have fsmonitor enabled and I don't think they'd ever get an error if watchman isn't working correctly.
I support introducing the config knob. But I don't like silently suppressing watchman errors that could lead to false user expectations.

I on +1 with what @indygreg said. These warning messages have proved much helpful for us internally.

lothiraldan added a comment.Jan 17 2019, 11:29 AM
In D5586#83031, @pulkit wrote:
In D5586#82915, @indygreg wrote:

This new config knob defaults to off and we're therefore changing the behavior to not warn when watchman is unavailable. This is a bit concerning to me because someone could have fsmonitor enabled and I don't think they'd ever get an error if watchman isn't working correctly.
I support introducing the config knob. But I don't like silently suppressing watchman errors that could lead to false user expectations.

I on +1 with what @indygreg said. These warning messages have proved much helpful for us internally.

I will update this patch to activate it by default.

Closed by commit rHG158962bf4a52: watchman: add verbose config knob (authored by lothiraldan). · Explain WhyJan 31 2019, 11:34 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/fsmonitor/__init__.py (10 lines)

Diff 13411

hgext/fsmonitor/__init__.py

default=False, default=False,
) )
configitem('fsmonitor', 'timeout', configitem('fsmonitor', 'timeout',
default='2', default='2',
) )
configitem('fsmonitor', 'blacklistusers', configitem('fsmonitor', 'blacklistusers',
default=list, default=list,
) )
configitem('hgwatchman', 'verbose',
default=True,
)
configitem('experimental', 'fsmonitor.transaction_notify', configitem('experimental', 'fsmonitor.transaction_notify',
default=False, default=False,
) )
# This extension is incompatible with the following blacklisted extensions # This extension is incompatible with the following blacklisted extensions
# and will disable itself when encountering one of these: # and will disable itself when encountering one of these:
_blacklist = ['largefiles', 'eol'] _blacklist = ['largefiles', 'eol']
def _handleunavailable(ui, state, ex): def _handleunavailable(ui, state, ex):
"""Exception handler for Watchman interaction exceptions""" """Exception handler for Watchman interaction exceptions"""
if isinstance(ex, watchmanclient.Unavailable): if isinstance(ex, watchmanclient.Unavailable):
if ex.warn: # experimental config: hgwatchman.verbose
if ex.warn and ui.configbool('hgwatchman', 'verbose'):
ui.warn(str(ex) + '\n') ui.warn(str(ex) + '\n')
if ex.invalidate: if ex.invalidate:
state.invalidate() state.invalidate()
# experimental config: hgwatchman.verbose
if ui.configbool('hgwatchman','verbose'):
ui.log('fsmonitor', 'Watchman unavailable: %s\n', ex.msg) ui.log('fsmonitor', 'Watchman unavailable: %s\n', ex.msg)
else: else:
ui.log('fsmonitor', 'Watchman exception: %s\n', ex) ui.log('fsmonitor', 'Watchman exception: %s\n', ex)
def _hashignore(ignore): def _hashignore(ignore):
"""Calculate hash for ignore patterns and filenames """Calculate hash for ignore patterns and filenames
If this information changes between Mercurial invocations, we can't If this information changes between Mercurial invocations, we can't
rely on Watchman information anymore and have to re-scan the working rely on Watchman information anymore and have to re-scan the working