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

Add cmd/fsnotify #463

Merged
merged 1 commit into from Jul 29, 2022
Merged

Add cmd/fsnotify #463

merged 1 commit into from Jul 29, 2022

Conversation

arp242
Copy link
Member

@arp242 arp242 commented Jul 21, 2022

This adds a little example that can be run:

% go run ./cmd/fsnotify **/*
12:76:11.4710 watching; press ^C to exit
12:76:13.6549   1 REMOVE                "cmd/fsnotify/4913"
12:76:13.6550   2 RENAME                "cmd/fsnotify/main.go"
12:76:13.6550   3 RENAME                "cmd/fsnotify/main.go"
12:76:13.6612   4 CREATE                "cmd/fsnotify/main.go"
12:76:13.6612   5 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:13.6614   6 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:13.6648   7 CHMOD                 "cmd/fsnotify/main.go"
12:76:15.2919   8 CREATE                "cmd/fsnotify/4913"
12:76:15.2919   9 REMOVE                "cmd/fsnotify/4913"
12:76:15.2922  10 CHMOD                 "cmd/fsnotify/main.go"
12:76:15.2923  11 REMOVE                "cmd/fsnotify/main.go"
12:76:15.2925  12 RENAME                "cmd/fsnotify/main.go"
12:76:15.2986  13 CREATE                "cmd/fsnotify/main.go"
12:76:15.2986  14 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:15.2988  15 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:15.3024  16 CHMOD                 "cmd/fsnotify/main.go"
^C

It's mostly the same as the example in the README, except that it takes
paths from the commandline and aligns things a bit nicer.

This is useful for documentation and experimentation. For example, while
reviewing some PRs I found it useful to quickly get an overview of "what
does fsnotify do now, and what does it do with this patch?"

This adds a little example that can be run:

	% go run ./cmd/fsnotify **/*
	12:76:11.4710 watching; press ^C to exit
	12:76:13.6549   1 REMOVE                "cmd/fsnotify/4913"
	12:76:13.6550   2 RENAME                "cmd/fsnotify/main.go"
	12:76:13.6550   3 RENAME                "cmd/fsnotify/main.go"
	12:76:13.6612   4 CREATE                "cmd/fsnotify/main.go"
	12:76:13.6612   5 WRITE      (modified) "cmd/fsnotify/main.go"
	12:76:13.6614   6 WRITE      (modified) "cmd/fsnotify/main.go"
	12:76:13.6648   7 CHMOD                 "cmd/fsnotify/main.go"
	12:76:15.2919   8 CREATE                "cmd/fsnotify/4913"
	12:76:15.2919   9 REMOVE                "cmd/fsnotify/4913"
	12:76:15.2922  10 CHMOD                 "cmd/fsnotify/main.go"
	12:76:15.2923  11 REMOVE                "cmd/fsnotify/main.go"
	12:76:15.2925  12 RENAME                "cmd/fsnotify/main.go"
	12:76:15.2986  13 CREATE                "cmd/fsnotify/main.go"
	12:76:15.2986  14 WRITE      (modified) "cmd/fsnotify/main.go"
	12:76:15.2988  15 WRITE      (modified) "cmd/fsnotify/main.go"
	12:76:15.3024  16 CHMOD                 "cmd/fsnotify/main.go"
	^C

It's mostly the same as the example in the README, except that it takes
paths from the commandline and aligns things a bit nicer.

This is useful for documentation and experimentation. For example, while
reviewing some PRs I found it useful to quickly get an overview of "what
does fsnotify do now, and what does it do with this patch?"
Copy link
Contributor

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

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

Just as my 2c I feel like log would probably suffice over line and/or fatal, and cmd seems odd for an example vs examples or contrib since it's more of a helper utility.

But it seems like a handy way to throw together a quick test overall.

@arp242
Copy link
Member Author

arp242 commented Jul 22, 2022

I mostly didn't use log because I wanted only the time with ms, and not the full date, which doesn't strike me as useful here and it saves 11 characters 😅 Maybe log can be configured (I never really used it), but just fmt seemed simpler.

I typically like putting these things in cmd as it's a "runnable program" so it seems to fit; but can put it somewhere else too; I don't care that much. I'd also like to expand this tool a little bit to add some more things like printing things like system information, volume information, etc. which might be useful for bug reports (but that's something for another day).

arp242 added a commit that referenced this pull request Jul 24, 2022
This is quite an odd check, leading to an inconsistent event stream,
which also doesn't match what other platforms do. If you do a "CREATE +
MODIFY + REMOVE" in quick succession then you probably *want* all three
events. If you don't want to operate on non-existing files, then you can
check this in your application code.

You need to do that already, since this check is far from reliable. In
the time between this check and the application code doing something
with an event the file may have been deleted already.

I looked a bit at the history of this, and looks like it was added in
2013 with cc2c34e; issue 36 refers to this issue on the old repo, which
mentions it fixes a memory leak: howeyc/fsnotify#36

I can't reproduce that at all; using the CLI from #463 modified to print
the memory and running:

	for i in $(seq 0 10000); { touch $i; rm $i }

Memory stays at about 100/110K in both the current main branch and this.

So I think it should be safe to remove.
@arp242 arp242 merged commit d9c9fa5 into main Jul 29, 2022
@arp242 arp242 deleted the cmd branch July 29, 2022 18:28
arp242 added a commit that referenced this pull request Jul 29, 2022
This is quite an odd check, leading to an inconsistent event stream,
which also doesn't match what other platforms do. If you do a "CREATE +
MODIFY + REMOVE" in quick succession then you probably *want* all three
events. If you don't want to operate on non-existing files, then you can
check this in your application code.

You need to do that already, since this check is far from reliable. In
the time between this check and the application code doing something
with an event the file may have been deleted already.

I looked a bit at the history of this, and looks like it was added in
2013 with cc2c34e; issue 36 refers to this issue on the old repo, which
mentions it fixes a memory leak: howeyc/fsnotify#36

I can't reproduce that at all; using the CLI from #463 modified to print
the memory and running:

	for i in $(seq 0 10000); { touch $i; rm $i }

Memory stays at about 100/110K in both the current main branch and this.

So I think it should be safe to remove.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants