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

serve: double reload on backup files deletion #383

Closed
dns2utf8 opened this issue Jul 28, 2017 · 11 comments · Fixed by #2325
Closed

serve: double reload on backup files deletion #383

dns2utf8 opened this issue Jul 28, 2017 · 11 comments · Fixed by #2325
Labels
C-bug Category: A bug, incorrect or unintended behavior C-papercut Category: A small usability bug Command-serve Command: serve Command-watch Command: watch S-Duplicate Status: Duplicate

Comments

@dns2utf8
Copy link

The problem arises when working with an editor like Kate which keeps your current edits in a swap file.

When saving the swapped contents to the real file an event occurs and triggers the rebuild of the book.
While the book is building another event occurs because the swap file is deleted.

On my machine this creates a race.
Sometimes the second thread is fast enough to delete the folder before the first finishes working and nothing happens.
But around 50% of the time the deletion happens right after the first build finished. This means the browser will receive a reload command at the same time as the second thread starts deleting the files. In that case the browser gets a 404 and the reload function is dead. A manual reload is required now.

I see a probable solution by triggering the rebuild with some delay on the server.
Delaying the client is also possible but the client would either have to poll the server for the file or delay a fixed amount of time.
What do you think?

@budziq
Copy link
Contributor

budziq commented Jul 28, 2017

Hi @dns2utf8 thanks for the bug report.
Trying to fix race conditions by sleep tuning is universally a bad idea 😉 . We will most likely need to introduce some synchronization with a command queue

@azerupi azerupi added the C-papercut Category: A small usability bug label Jul 29, 2017
@dns2utf8
Copy link
Author

dns2utf8 commented Aug 7, 2017

Another way to solve this would could be the 404 page. If it had code included for may displaying the index or detect the page beeing served (maybe again).
This would cover the scenario where the user switched from one branch of the book to another or merges.

@azerupi
Copy link
Contributor

azerupi commented Aug 8, 2017

Maybe we could add a command line argument to blacklist certain patterns of files, would that help?

@dns2utf8
Copy link
Author

dns2utf8 commented Sep 2, 2017

What if mdbook were to ignore all changes coming from files marked by the .gitignore (or similar) file?

@budziq
Copy link
Contributor

budziq commented Sep 6, 2017

@dns2utf8 could you look on this PR https://github.com/azerupi/mdBook/pull/427? It solves multiple reloads on my system with vim (using notify crates builtin event aggregation logic which we have circumvented so far). It might not solve your case yet though.

@azerupi
Copy link
Contributor

azerupi commented Sep 7, 2017

Release 0.0.24 includes the fix from @budziq.
Could anyone try with the new version and see if it fixed the issues?

@budziq
Copy link
Contributor

budziq commented Sep 7, 2017

@azerupi I've checked it against kate today. The situation is better but not fixed. Kate emmits writes to tempfiles on each edit. This code at least deduplicates the events but you get the write notifies after each few edits which is as expected. Imho it its quite a nice side effect (you get live render preview with not too many reloads without need to save). But if someone thinks we should completely ignore the temp files we could use ignore crate

@dns2utf8
Copy link
Author

Happy new year everybody. With the latest 0.1.1 release this behavior has become more intrusive. Even though I have this ignore file the reloads get triggerd while I type.

book
.DS_Store
.directory
*.kate-swp

On my system the actual file does not change while typing, so I get no live preview. The edits go into the .filename.kate-swp and I get many rebuilds and reloads without any effect. As soon as I save I get a 404 because in the very moment it builds the original file is removed and then kate moves the swap file to the original location.

@Michael-F-Bryan
Copy link
Contributor

I'm looking through the notify docs and I don't know if you can give it some sort of ignore pattern. You can unset specific paths, but I'm not sure whether it'll keep ignoring a file once it's been deleted.

Even though I have this ignore file the reloads get triggerd while I type.

I don't think we pay any attention to your .gitignore at the moment. Instead we'll unconditionally watch everything in your src/, as well as the themes/ directory (if it exists), and book.toml.

@dns2utf8
Copy link
Author

I did some work with the ignore crate: see this branch
My current problem is new and recreated (unlink + create) files. So I asked on the issue tracker but I will have to look at it tomorrow.

@KFearsoff
Copy link
Contributor

@rustbot label +C-bug +Command-serve +Command-watch +S-Duplicate

See #2102 (comment)

@rustbot rustbot added C-bug Category: A bug, incorrect or unintended behavior Command-serve Command: serve Command-watch Command: watch S-Duplicate Status: Duplicate labels Nov 14, 2023
KFearsoff added a commit to KFearsoff/mdBook that referenced this issue Nov 18, 2023
KFearsoff added a commit to KFearsoff/mdBook that referenced this issue Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: A bug, incorrect or unintended behavior C-papercut Category: A small usability bug Command-serve Command: serve Command-watch Command: watch S-Duplicate Status: Duplicate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants