Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn if the fsmonitor-daemon is running #2891

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

LenaWil
Copy link

@LenaWil LenaWil commented May 18, 2023

Less extreme than #2890.

LenaWil and others added 3 commits May 18, 2023 21:10
Comment on lines 52 to 53
if cmd_output('git', 'fsmonitor--daemon', 'status')[1].startswith('fsmonitor-daemon is watching'):
logger.warning('The fsmonitor-daemon is running; a bug in the fsmonitor-daemon sometimes causes data loss when run with pre-commit.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will break on old versions of git

this will also add this subprocess overhead to every execution -- ~20ms on my machine

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will break on old versions of git

Good point, switched it to checking for the return code.

this will also add this subprocess overhead to every execution -- ~20ms on my machine

I added two extra checks, a platform check, since it’s only available on Windows & macOS, and a check for the ipc file that should by default be there, although it’s seems to be possible to change it on macOS. Does this speed it up enough?

@LenaWil LenaWil requested a review from asottile May 20, 2023 10:21
@LenaWil
Copy link
Author

LenaWil commented Jun 14, 2023

@asottile Is this okay? Do I need to write tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants