[Geany-Devel] Simplifying file saving options

Lex Trotman elextr at xxxxx
Wed Aug 19 23:59:28 UTC 2015


On 19 August 2015 at 23:39, Jiří Techet <techet at gmail.com> wrote:
>
> On Wed, Aug 19, 2015 at 12:43 PM, Lex Trotman <elextr at gmail.com> wrote:
>>
>> On 19 August 2015 at 19:48, Jiří Techet <techet at gmail.com> wrote:
>> > Hi,
>> >
>> > after some time spent debugging
>> >
>> > https://github.com/geany/geany/issues/605
>> >
>> > I think the file saving options are a bit confusing and misleading and
>> > should be simplified.
>>
>> Agree the usability is poor with several options that interact with each
>> other.
>>
>> >
>> > 1. Both g_file_set_contents() and g_file_replace_contents() do
>> > absolutely
>> > the same thing (saving to temporary file, then renaming the file),
>>
>> No they are not the same, set_contents does not work on file systems
>> where rename is not available or is not allowed over existing files
>> (windows), and it creates a new file with new file metadata (eg
>> permissions), whereas replace_contents is much more complex than set,
>> it tries to compensate for these issues but can do several copies of
>> the data during the attempt, which is very costly on slow remote
>> filesystems and has a bug (see below).
>
>
> 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:
>
> https://github.com/bratsche/glib/blob/master/glib/gfileutils.c#L1087

And its wrong. The choice is made at compile time for the platform
Glib is being compiled for, not at runtime based on the target
filesystem.  So it is only right for local native disks/partitions of
the machine it runs on, and is wrong for example on cifs shares
mounted on Linux.

>
>>
>>
>> > the only
>> > difference is the first uses POSIX calls while the second GIO. The
>> > use_gio_unsafe_file_saving name is very confusing because it's not
>> > unsafe in
>> > any way.
>>
>> It is unsafe due to a long standing bug where a failure deletes the
>> copy of the original data without copying it back to the original
>> file, so you are left with a truncated original file and no backup of
>> the original data.  You still have the new data in the Geany buffer of
>> course.  But it regularly has caught people out if they closed the
>> buffer after a failure assuming they had the original data safe.
>
>
> Uh, just checked the code and it really seems to be the case. Might fix that
> if I have time.

That would be great, but it would still be "some time" before the fix
propagates to all machines using GIO so we could not rely on it until
then.

>
>>
>>
>> >
>> > 2. It is now possible to set
>> >
>> > use_gio_unsafe_file_saving=true
>> > use_atomic_file_saving=true
>> >
>> > and to users it's unclear which of the options will be actually used
>> > (it's
>> > the use_atomic_file_saving case because it's checked first in the code).
>>
>> Yeah, an enum not a set of bools would be better, IIUC the current
>> system grew all the options over time rather than being planned.  For
>> sure cleaning it up would be good.
>>
>> >
>> > 3. The fallback "ordinary" file saving when
>> >
>> > use_gio_unsafe_file_saving=false
>> > use_atomic_file_saving=false
>> >
>> > doesn't bring any benefit compared to the two above
>>
>> It just overwrites the file, nothing else, one data transfer, fast, no
>> rename, works on *all* filesystems, keeps the files metadata.  Whilst
>> its not in any way "safe" its got the best performance.
>
>
> OK, might make sense to keep it then.
>
> Jiri
>
> _______________________________________________
> Devel mailing list
> Devel at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>


More information about the Devel mailing list