Hi,
During the phase of removing code that is not relevant anymore since the dependency bump, I fatally came out to write_data_to_disk(). Now we can assume we have GIO, it may seem sensible to drop completely the C API file saving code...
...However, GIO saving seems to cause some problems on some complex situations:
https://sourceforge.net/tracker/?func=detail&atid=787791&aid=3220784... https://sourceforge.net/tracker/index.php?func=detail&aid=3184594&gr... https://sourceforge.net/tracker/index.php?func=detail&aid=3126535&gr...
(and maybe more -- at least one on IRC).
So I'm wondering: do we really want to drop the C API-using code path, or do we want to make it configurable (like safe file saving)?
Well, since I'm not completely sure of what causes the problems and what are the downsides (either I don't remember or I did not participate to the discussion...), I prefer to ask you what do you think/know/remember/whatever about this.
I join a sample patch that make GIO saving configurable (just like safe file saving) with it enabled by default, if anybody wants to check it out.
So, what's your opinion?
Cheers, Colomban
Hi Columban.
On 14 June 2011 08:54, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hi,
During the phase of removing code that is not relevant anymore since the dependency bump, I fatally came out to write_data_to_disk(). Now we can assume we have GIO, it may seem sensible to drop completely the C API file saving code...
...However, GIO saving seems to cause some problems on some complex situations:
https://sourceforge.net/tracker/?func=detail&atid=787791&aid=3220784... https://sourceforge.net/tracker/index.php?func=detail&aid=3184594&gr... https://sourceforge.net/tracker/index.php?func=detail&aid=3126535&gr...
(and maybe more -- at least one on IRC).
So I'm wondering: do we really want to drop the C API-using code path, or do we want to make it configurable (like safe file saving)?
Well, since I'm not completely sure of what causes the problems and what are the downsides (either I don't remember or I did not participate to the discussion...), I prefer to ask you what do you think/know/remember/whatever about this.
Having been involved in some of the previous discussions here is what I remember.
The methods of saving files are:
1. g_file_set_contents, saves to a temporary file and renames, our safe file saving setting. Safe, but permissions can change. On windows has to close the old file and delete it before renaming the temporary, on Unix just renames. Thus won't work if running on Unix and accessing Windows files.
2. g_file_replace, tries to do the same as g_file_set_contents but checks if it can change the permissions first and, if it can't, falls back to copying the old file to a temporary then truncating the old file and writing over it. On slow remote filesystems this gives lousy performance as it transfers data three times not just once (subject of several complaints). Because it has better knowledge of which filesystems are which, can work on windows files from Unix. Not safe, unless set to provide a permanent backup, will still delete the temporary file even if writing the new file fails. Option for keeping the backup is in Geany SVN version only.
3. c library write, unsafe but works fast on all filesystems.
At the moment the c library option is only available if compile time determines GIO is not available or the user configures to not use GIO.
Looking at these methods, they each address differing situations. As Enrico once said, 'who knew it was so hard to just write a file' :-).
So I would say that all three need to stay, and the choice between GIO and the c library needs to be made a preference not a compile time option.
So your patch looks good to me (by inspection only).
Cheers Lex
I join a sample patch that make GIO saving configurable (just like safe file saving) with it enabled by default, if anybody wants to check it out.
So, what's your opinion?
Cheers, Colomban
Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On Tue, 14 Jun 2011 10:28:00 +1000, Lex wrote:
Hi Columban.
On 14 June 2011 08:54, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hi,
During the phase of removing code that is not relevant anymore since the dependency bump, I fatally came out to write_data_to_disk(). Now we can assume we have GIO, it may seem sensible to drop completely the C API file saving code...
...However, GIO saving seems to cause some problems on some complex situations:
https://sourceforge.net/tracker/?func=detail&atid=787791&aid=3220784... https://sourceforge.net/tracker/index.php?func=detail&aid=3184594&gr... https://sourceforge.net/tracker/index.php?func=detail&aid=3126535&gr...
(and maybe more -- at least one on IRC).
So I'm wondering: do we really want to drop the C API-using code path, or do we want to make it configurable (like safe file saving)?
Well, since I'm not completely sure of what causes the problems and what are the downsides (either I don't remember or I did not participate to the discussion...), I prefer to ask you what do you think/know/remember/whatever about this.
Having been involved in some of the previous discussions here is what I remember.
The methods of saving files are:
- g_file_set_contents, saves to a temporary file and renames, our
safe file saving setting. Safe, but permissions can change. On windows has to close the old file and delete it before renaming the temporary, on Unix just renames. Thus won't work if running on Unix and accessing Windows files.
- g_file_replace, tries to do the same as g_file_set_contents but
checks if it can change the permissions first and, if it can't, falls back to copying the old file to a temporary then truncating the old file and writing over it. On slow remote filesystems this gives lousy performance as it transfers data three times not just once (subject of several complaints). Because it has better knowledge of which filesystems are which, can work on windows files from Unix. Not safe, unless set to provide a permanent backup, will still delete the temporary file even if writing the new file fails. Option for keeping the backup is in Geany SVN version only.
- c library write, unsafe but works fast on all filesystems.
At the moment the c library option is only available if compile time determines GIO is not available or the user configures to not use GIO.
Looking at these methods, they each address differing situations. As Enrico once said, 'who knew it was so hard to just write a file' :-).
This sentence still comes to my mind everytime we speak about GIO and file saving here :(. If we all only could use local ext2/3 filesystems with huge capacity, all these problems were gone and we could simply use the C API as it worked for the past decades :D. But let's stop dreaming and coming back to reality:
So I would say that all three need to stay, and the choice between GIO and the c library needs to be made a preference not a compile time option.
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Regards, Enrico
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Yes, thats a bit of a problem, summarised as one liners we get:
1. safe but might change permissions and won't work on windows shares from *ix, no backups 2. can be unsafe, can be slow, complicated, but should work on all mounted filesystems and some urls, can keep the temp file as backup backup 3. risky, simple, works on all mounted filesystems, no backups
Not really a marketing mans dream :-(
What to choose if some of the files you have open are on your webserver via ftpfs and some local? And there is no safe method if windows shares are involved.
Of course save-actions plugin can save a backup, but it always reads and writes the old file contents which can be slow over a network. And its autosaving periodically can also lose your file as it saves it, if its using one of 2 or 3, not happy.
Cheers Lex
PS just for info last time I looked Gedit used option 2 but with special implementation dependent hacking to make it safe. Maybe stealing that is the best option, then its biggest downside is being slow sometimes.
Regards, Enrico
-- Get my GPG key from http://www.uvena.de/pub.asc
Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Le 19/06/2011 14:41, Lex Trotman a écrit :
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Yes, thats a bit of a problem, summarised as one liners we get:
- safe but might change permissions and won't work on windows shares
from *ix, no backups 2. can be unsafe, can be slow, complicated, but should work on all mounted filesystems and some urls, can keep the temp file as backup backup 3. risky, simple, works on all mounted filesystems, no backups
Just tell me -- what's the advantages of 2 over 3 then? OK, it wouldn't depend on gvfs-fuse.
[...]
PS just for info last time I looked Gedit used option 2 but with special implementation dependent hacking to make it safe. Maybe stealing that is the best option, then its biggest downside is being slow sometimes.
Yes, we could make GIO saving safe (e.g. handle writes failure) by haking on until #602412 [1] get fixed properly. This looks kinda ugly, but would at least provide safe saving, just leaving out the temporary file on failure.
But it wouldn't address the cases where GIO won't success to properly write (e.g. the bugs I linked in previous mail). Maybe it is fixable in GIO though, so reporting those to them might be a start point -- though I can't tell whether it's reproducible in recent GIO versions, nor what kind of problem it is.
Le 19/06/2011 13:42, Enrico Tröger a écrit :
On Tue, 14 Jun 2011 10:28:00 +1000, Lex wrote:
So I would say that all three need to stay, and the choice between >>
GIO and the c library needs to be made a preference not a compile
time option.
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Agreed. However, I can't see how to present this to the user in a way she can take a proper decision... In the meantime, I think I can commit the patch I proposed -- we will still be able to make it a visible pref later -- nope? Though, maybe it'll make us lazy to do the change... :D
Cheers, Colomban
Just tell me -- what's the advantages of 2 over 3 then? OK, it wouldn't depend on gvfs-fuse.
Yes, and it is safe (I think) if it keeps the backup file.
[...]
PS just for info last time I looked Gedit used option 2 but with special implementation dependent hacking to make it safe. Maybe stealing that is the best option, then its biggest downside is being slow sometimes.
Yes, we could make GIO saving safe (e.g. handle writes failure) by haking on until #602412 [1] get fixed properly. This looks kinda ugly, but would at least provide safe saving, just leaving out the temporary file on failure.
My understanding is that the nasty hack in Gedit is to work around this problem.
But it wouldn't address the cases where GIO won't success to properly write (e.g. the bugs I linked in previous mail). Maybe it is fixable in GIO though, so reporting those to them might be a start point -- though I can't tell whether it's reproducible in recent GIO versions, nor what kind of problem it is.
3220784 doesn't say if safe file saving is set (ie its using type1), its the default and doesn't use GIO but no one asked the OP also notes gedit works so stealing their hack would work. 3184594 is using GIO, the temporary file is a GIO name. The question is what sort of filesystem does virtual box report the host to the guest as, this may confuse GIO. 3126535 doesn't say if safe file saving is set (ie its using type1), its the default and doesn't use GIO but no one asked the OP
Le 19/06/2011 13:42, Enrico Tröger a écrit :
On Tue, 14 Jun 2011 10:28:00 +1000, Lex wrote:
So I would say that all three need to stay, and the choice between >>
GIO and the c library needs to be made a preference not a compile
time option.
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Agreed. However, I can't see how to present this to the user in a way she can take a proper decision... In the meantime, I think I can commit the patch I proposed -- we will still be able to make it a visible pref later -- nope? Though, maybe it'll make us lazy to do the change... :D
I agree that it is important enough to add the patch as an interim until the UI or some other fix can be worked out.
Cheers Lex
Le 20/06/2011 00:49, Lex Trotman a écrit :
Just tell me -- what's the advantages of 2 over 3 then? OK, it wouldn't depend on gvfs-fuse.
Yes, and it is safe (I think) if it keeps the backup file.
True. And it would be safe against write failures with GEdit's hack or similar.
[...]
PS just for info last time I looked Gedit used option 2 but with special implementation dependent hacking to make it safe. Maybe stealing that is the best option, then its biggest downside is being slow sometimes.
Yes, we could make GIO saving safe (e.g. handle writes failure) by haking on until #602412 [1] get fixed properly. This looks kinda ugly, but would at least provide safe saving, just leaving out the temporary file on failure.
My understanding is that the nasty hack in Gedit is to work around this problem.
Yes it is. I also wrote an example a while ago with a similar hack, and it worked -- will need to check which hack is the best, but still a hack...
But it wouldn't address the cases where GIO won't success to properly write (e.g. the bugs I linked in previous mail). Maybe it is fixable in GIO though, so reporting those to them might be a start point -- though I can't tell whether it's reproducible in recent GIO versions, nor what kind of problem it is.
3220784 doesn't say if safe file saving is set (ie its using type1), its the default and doesn't use GIO but no one asked the OP also notes gedit works so stealing their hack would work.
Right. Simply using GIO would work without the hack I guess, the hack just prevents data loss upon write failure.
3184594 is using GIO, the temporary file is a GIO name. The question is what sort of filesystem does virtual box report the host to the guest as, this may confuse GIO.
Yeah...
3126535 doesn't say if safe file saving is set (ie its using type1), its the default and doesn't use GIO but no one asked the OP
I guess she's using GIO (.goutputstream-KMEUMV is a GIO name), and and he seems to have the same problem with GEdit. Looks like the target file was still open ("Text file busy"), so it's either a GIO problem (failed to detect it needs closing the file on a Windows share), an OP's problem (she has the file opened on the remote side) or a sharing problem (whatever).
Le 19/06/2011 13:42, Enrico Tröger a écrit :
On Tue, 14 Jun 2011 10:28:00 +1000, Lex wrote:
So I would say that all three need to stay, and the choice between >>
GIO and the c library needs to be made a preference not a compile
time option.
I think so too. I'm only afraid how to give this choice to the user. This needs to be well documented and probably should go into a normal. not hidden pref and so should be in the preferences dialog.
Agreed. However, I can't see how to present this to the user in a way she can take a proper decision... In the meantime, I think I can commit the patch I proposed -- we will still be able to make it a visible pref later -- nope? Though, maybe it'll make us lazy to do the change... :D
I agree that it is important enough to add the patch as an interim until the UI or some other fix can be worked out.
Committed.
Cheers, Colomban
On Tue, 14 Jun 2011 00:54:48 +0200 Colomban Wendling lists.ban@herbesfolles.org wrote:
...However, GIO saving seems to cause some problems on some complex situations: [...]
The weak and strong points of the different save methods aside, I receintly upgraded my file manager to it's new, gvfs-based version, and... Please, keep the C-save as an option, or at least in a #define. I don't need it at the moment, but relying on g_whatever() _only_ gives me the creeps.