<div dir="ltr"><br><div class="gmail_extra">On Wed, Aug 19, 2015 at 12:43 PM, Lex Trotman <span dir="ltr"><<a href="mailto:elextr@gmail.com" target="_blank">elextr@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 19 August 2015 at 19:48, Jiří Techet <<a href="mailto:techet@gmail.com">techet@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> after some time spent debugging<br>
><br>
> <a href="https://github.com/geany/geany/issues/605" rel="noreferrer" target="_blank">https://github.com/geany/geany/issues/605</a><br>
><br>
> I think the file saving options are a bit confusing and misleading and<br>
> should be simplified.<br>
<br>
</span>Agree the usability is poor with several options that interact with each other.<br>
<span class=""><br>
><br>
> 1. Both g_file_set_contents() and g_file_replace_contents() do absolutely<br>
> the same thing (saving to temporary file, then renaming the file),<br>
<br>
</span>No they are not the same, set_contents does not work on file systems<br>
where rename is not available or is not allowed over existing files<br>
(windows), and it creates a new file with new file metadata (eg<br>
permissions), whereas replace_contents is much more complex than set,<br>
it tries to compensate for these issues but can do several copies of<br>
the data during the attempt, which is very costly on slow remote<br>
filesystems and has a bug (see below).<br></blockquote><div><br></div><div>I meant "same" in the sense they both should be safe. Yes, you are right, g_file_replace_contents() tries to mitigate the problems of g_file_set_contents(). By the way g_file_set_contents() works on Windows, it just isn't atomic:</div><div><br></div><div><a href="https://github.com/bratsche/glib/blob/master/glib/gfileutils.c#L1087">https://github.com/bratsche/glib/blob/master/glib/gfileutils.c#L1087</a><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> the only<br>
> difference is the first uses POSIX calls while the second GIO. The<br>
> use_gio_unsafe_file_saving name is very confusing because it's not unsafe in<br>
> any way.<br>
<br>
</span>It is unsafe due to a long standing bug where a failure deletes the<br>
copy of the original data without copying it back to the original<br>
file, so you are left with a truncated original file and no backup of<br>
the original data.  You still have the new data in the Geany buffer of<br>
course.  But it regularly has caught people out if they closed the<br>
buffer after a failure assuming they had the original data safe.<br></blockquote><div><br></div><div>Uh, just checked the code and it really seems to be the case. Might fix that if I have time.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
><br>
> 2. It is now possible to set<br>
><br>
> use_gio_unsafe_file_saving=true<br>
> use_atomic_file_saving=true<br>
><br>
> and to users it's unclear which of the options will be actually used (it's<br>
> the use_atomic_file_saving case because it's checked first in the code).<br>
<br>
</span>Yeah, an enum not a set of bools would be better, IIUC the current<br>
system grew all the options over time rather than being planned.  For<br>
sure cleaning it up would be good.<br>
<span class=""><br>
><br>
> 3. The fallback "ordinary" file saving when<br>
><br>
> use_gio_unsafe_file_saving=false<br>
> use_atomic_file_saving=false<br>
><br>
> doesn't bring any benefit compared to the two above<br>
<br>
</span>It just overwrites the file, nothing else, one data transfer, fast, no<br>
rename, works on *all* filesystems, keeps the files metadata.  Whilst<br>
its not in any way "safe" its got the best performance.<br></blockquote><div><br></div><div>OK, might make sense to keep it then.</div><div><br></div><div>Jiri</div></div></div></div>