Better check for a file change on disk by also monitoring the file size.

[StefanOberhumer]

Geany only checks if the modification time is newer than the previous fetched.
Having a script generating files and touching those files with an explicit time stamp, geany does not notify the file has changed.
So it's a better check to monitor modification time AND file size.

[codebrainz]

Can you explain how it's better?

[elextr]

I don't see it adds much, you have to absolutely know that the newly generated file is a different length. And generating files and touching their mod time is going to break much more than just Geany's detection.

What could be sensible though is to check mtime as not equals rather than less than, the file is different if its moved back in time as well as forward in time.

[StefanOberhumer]

I don't see it adds much, you have to absolutely know that the newly generated file is a different length.

With the current mtime checking you have to absolutly know that the newly generated file has a greater modification time.
A file must have been changed if the file size changes!
Checking both (mtime and file size) is a better file change check for geany than just checking mtime.

And generating files and touching their mod time

To get deterministic release tar balls, it is necessary for me to touch the files with an explicit time stamp before "tarring" them.

is going to break much more than just Geany's detection.

That's why I added the file size check - to get a better geany.
The fstab() already includes the file size - so there should be nearly no performance costs.

What could be sensible though is to check mtime as not equals rather than less than,
the file is different if its moved back in time as well as forward in time.

I agree - but I didn't wanted change the existing mtime check behavior.

So the technical facts are:
1.) If the file size changes on disk, the file has changed!
2.) If the file size changes and the modification time is not updated geany does NOT notify the file change.
3.) The behind laying fstat() function has the file size included - there will be nearly no performance impacts.

[codebrainz]

What if the contents change but the size remains the same, will Geany still notify of the change or just silently clobber the file on disk next save?

[StefanOberhumer]

What if the contents change but the size remains the same, will Geany still notify of the change or just silently clobber the file on disk next save?

If the file size and mtime are unchanged, Geany silently clobber the file on disk next save.
I thought about this problem but generating a checksum or comparing the whole file generated performance problems in my tests.
So for now I just added the file size to the change check.

[codebrainz]

If the file size and mtime are unchanged...

What if the file size is unchanged but the mtime is changed?

[StefanOberhumer]

If the file size and mtime are unchanged...

What if the file size is unchanged but the mtime is changed?

Same behavior as in current Geany.
Detail:

• Geany only notify a file change if the modification time is greater (file on disk is newer)
  than last time loaded/saved.
• It does not inform the user if the mtime on disk is less (file on disk is older) than last time loaded/saved.

I recognized this but I didn't want to change this check as I thought someone explicit designed it that way.

[elextr]

I am not sure I understand your use-case, if you change mtime backward geany won't trigger, but if the file size is changed the tarball won't be consistent anyway, so there is not reason to set mtime backward, so geany will trigger.

But as you say the change is minimal for local filesystems.

I would want to know this works for all sorts of funny file systems, for GIO and non-GIO situations etc since we have always had issues with annoying unneccessary notifications and with missed notifications. We seem to have just got that calmed down a bit and I wouldn't want to stir it up again.

[StefanOberhumer]

I am not sure I understand your use-case, if you change mtime backward geany won't trigger, but if the file size is changed the tarball won't be consistent anyway, so there is not reason to set mtime backward, so geany will trigger.

I don't want discuss and analyse my use-case.
Fact: If file size on disk changes the file has been changed!

I would want to know this works for all sorts of funny file systems, for GIO and non-GIO situations etc since we have always had issues with annoying unneccessary notifications and with missed notifications. We seem to have just got that calmed down a bit and I wouldn't want to stir it up again.

I only added code to get also the file size from the ( already used ) functions g_stat() and g_file_query_info().
So if they work as expected this should run on all funny file systems.

[elextr]

I don't want discuss and analyse my use-case.

Ok, but as I pointed out, this needs testing in uncommon configurations, that means effort by somebody, and the relative urgency is informed by the assessment of the use-case. We were wondering why you didn't use tar --mtime= to get consistent mtimes?

So if they work as expected...

Which is what needs testing, we thought they worked as expected previously, but they didn't, and hence the issues we had. It would be nice if its all ok as expected, but past experience leads to caution.

[StefanOberhumer]

OK - I ll give up - I close the pull request!

• This discussion cost more time than updating my patches just for me (as I do it since Geany 0.18).
• I also need more lines to explain and to justify (still discussing my (a sample) use-case) than source code lines - please accept:
  If file size on disk changes the file has been changed and Geany does not notice this (regardless any use-cases)!

I thought I start submitting a really small and reasonable patch of my patchset and try to get a better public Geany.

[elextr]

If file size on disk changes the file has been changed and Geany does not notice this (regardless any use-cases)!

Agreed, and its great that you offered a pull request. The discussion is not about that, but the benefits.

As I said there have been problems in the past with change detection, with things like the failure of notification based detection, the need to separate GIO operations and Clib operations, and having to not mix their code paths. So I have some concern that the change should be tested in more than the local disk case, which always worked, all the time, as expected. This essentially means in various networked environments (SSHFS anybody), and in Windows and SMB shares. None of these are the normal environment for Geany devs, so it costs them time and effort to set up and test it, or to find other contributors to do so.

It is perfectly understandable that you would not count that cost when you initially raised the "simple" PR, but it is something project contributors need to consider and weigh against the risks and benefits of the change.

So it is perfectly reasonable to enquire how much of a benefit the change brings and if there is an easy workaround if its not applied. That allows people to see how many others might benefit, to see how much effort needs to be applied and how urgently, especially as there is not (that I could find) an existing issue that indicates a need for the change.

I'm sorry that you made your first contribution in a sensitive area and found unexpected questioning of the need for it, but I hope the extended explanation helps you to understand the other side.

[codebrainz]

If you actually want this merged, why close the pull request?