Am 13.05.2017 um 08:19 schrieb Guillaume MM <[email protected]>:
> 
> Le 11/05/2017 à 09:59, Stephan Witt a écrit :
>>>> 
>>>> The delay is defined in Buffer::Impl::blockFileMonitor (Buffer.cpp:388)
>>>> currently at 10ms. Can you try to increase this value and see if that
>>>> helps? It will likely help, but this is not a nice solution.
>> 
>> I tried it with a delay of 100ms. The effect is now LyX presents the external
>> modification message box every 2nd or 3rd save. So it is not the solution.
> 
> Thanks for the test, this helps. BTW is there a delay before the message
> appears and if so how long? Do you see the following error message with
> the files debug flag:
>  LYXERR(Debug::FILES, "Could not add path to QFileSystemWatcher: " <<
> filename_);
> ?

This is the output of the un-patched LyX on save with false-positive message:

support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 3874605062 lasted 1 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/Documents/newfile12-XXXXXX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/Documents/newfile12-P37773.lyx' created.
Buffer.cpp (1435): Saving to /Users/stephan/Documents/newfile12-P37773.lyx
BufferParams.cpp (321): Checking whether document is in a system dir... no
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 3874605062 lasted 1 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/Documents/newfile12.lyx~
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile12.lyx~ to 
/Users/stephan/Documents/newfile12.lyx
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile12.lyx to 
/Users/stephan/Documents/newfile12-P37773.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 2474078819 lasted 1 ms.

>>>> Ideally I would like to have everything work with a delay of 0ms, to be
>>>> sure that everyone experiences the same. Can you help me and see if it
>>>> is possible to flush the file operations in FileName::copyTo and
>>>> FileName::moveTo and if it makes a difference ? There is QFile::flush
>>>> but I do not really see how to use it in this context and I cannot test
>>>> the situation.
>> 
>> I cannot see how to do it here.
>> 
>> The save operation in this scenario is done with one create (temp), one
>> rename (backup) and another rename (final name).
>> 
>> Are you sure the file monitor is able to follow these steps?
> 
> What is expected to happen is:
> 
> In Buffer::save:
> 
> 1. Disconnect from QFileSystemWatcher
> 2. Perform various file operations
> 3. Queue the reconnection to QFileSystemWatcher
> 
> Later:
> 
> 4. QFileSystemWatcher notifies of the deletion
> 5. The new file is moved
> 6. Reconnect to QFileSystemWatcher and refresh (watch the new file)
> 
> Given the behaviour that you report above, it seems that the
> reconnection happens too early. The number and the nature of the
> operations is not important.
> 
> I notice that the QSaveFile class which is provided by Qt (but not used
> in LyX) calls a function fileEngine->syncToDisk() not accessible from
> the API, and below there are the comments:
> // atomically replace old file with new file
> // Can't use QFile::rename for that, must use the file engine directly
> 
> I suspect that using QSaveFile instead of the current implementation
> would solve the bug and make file saving safer (even).
> 
> Can you please try the attached patch?

This is the output of the patched one - no false-positive message:
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 2101337338 lasted 0 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/Documents/newfile13-XXXXXX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/Documents/newfile13-B38676.lyx' created.
Buffer.cpp (1435): Saving to /Users/stephan/Documents/newfile13-B38676.lyx
BufferParams.cpp (321): Checking whether document is in a system dir... no
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 2101337338 lasted 1 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/Documents/newfile13.lyx~
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile13.lyx~ to 
/Users/stephan/Documents/newfile13.lyx
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile13.lyx to 
/Users/stephan/Documents/newfile13-B38676.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 834715601 lasted 2 ms.
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 834715601 lasted 0 ms.

And this is for a big document (on a SSD based machine):

support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 1327983642 lasted 35 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-XXXXXX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx' created.
Buffer.cpp (1435): Saving to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx
BufferParams.cpp (315): Checking whether document is in a system dir... yes
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 1327983642 lasted 37 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-lyxformat-541.lyx~
support/FileName.cpp (267): Moving 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-lyxformat-541.lyx~ to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx
support/FileName.cpp (267): Moving 
/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 582248824 lasted 35 ms.
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 582248824 lasted 37 ms.

> Before deciding which solution is
> the best I will still need to know the answer to the questions at the
> beginning, if you please can help with that.
> 
> Thank you
> Guillaume
> 
> <0001-Prevent-false-positives-in-external-modifications.patch>

Reply via email to