GNOME Bugzilla – Bug 509239
Drop gnome-vfs dependency
Last modified: 2008-03-14 21:02:15 UTC
It would be nice if EOG dropped gnome-vfs dependency and replaced it with the new gio API. I have started a patch for this, hopefully I'll have it ready before 2.22.
*** Bug 509328 has been marked as a duplicate of this bug. ***
From bug #509328: Luca Ferreti said: > Note that EoG is using GNOME-vfs to open non local files, but also to move > images to trash. GIO/GVFS implements the freedesktop.org Trash spec: this means > the GIO applications (i.e. Nautilus) will check for trashed items in > $XDG_DATA_HOME/Trash (that usually is ~/.local/share/Trash), not in ~/.Trash. > So, but I didn't checked, if you move an image to trash in EoG, you could not > find it in your Trash.
(In reply to comment #2) > > So, but I didn't checked, if you move an image to trash in EoG, you could not > > find it in your Trash. > Checked just now. Unfortunately I was right... Maybe ask fd.o trash support in gnome-vfs could be a better idea? PS sorry for the duplicate, I searched only for bugs with gio in summary.
Created attachment 103077 [details] [review] gio-migration v1 Attached patch drops gnome-vfs dependency for EOG. I'm still testing if everything works fine...trash at least does. Comments and review welcome :)
Forgot to mention that this patch depends on GLib 2.15.3 which is now SVN trunk, because of g_file_query_exists that was introduced yesterday.
Hi, I couldn't test it as I don't have a recent-enoug GLIB installed at the moment, but here are some things I noticed while having a quick look at it: 1. >- jsrcerr.filename = gnome_vfs_uri_get_path (priv->uri); >+ jsrcerr.filename = g_file_get_path (priv->file); This is as far as I can see a memleak. g_file_get_path() requires freeing of its returned string. 2. > static char* >-get_save_file_type_by_uri (const GnomeVFSURI *uri) >+get_save_file_type (GFile *file) > EogImageSaveInfo* >-eog_image_save_info_from_vfs_uri (GnomeVFSURI *uri, GdkPixbufFormat *format) >+eog_image_save_info (GFile *file, GdkPixbufFormat *format) You should possibly add some "from_gfile" (or something like it) suffix to these functions. The new names are not overly descriptive (especially the second one). 3. >+ if (file_info == NULL) { >+ mime_str = _("Unknown"); >+ type_str = g_strdup (_("Unknown")); >+ } else { >+ mime_str = g_file_info_get_content_type (file_info); >+ type_str = g_content_type_get_description (mime_str); >+ g_object_unref (file_info); >+ } This is a memleak in at least one case. Use use type_str as a const pointer although it isn't. It's not for sure in the g_strdup case. Unfortunately the API docs are not clear if g_content_type_get_description returns strings that need to be freed (although it looks a lot like it). 4. > /* The password gets stripped here because ~/.recently-used.xbel is > * readable by everyone (chmod 644). It also makes the workaround > * for the bug with gtk_recent_info_get_uri_display() easier > * (see the comment in eog_window_update_recent_files_menu()). */ >- text_uri = gnome_vfs_uri_to_string (uri, GNOME_VFS_URI_HIDE_PASSWORD); >+ text_uri = g_file_get_uri (file); Well, this is something I am not yet sure about. Does g_file_get_uri always strip username and password from the URI? Otherwise they might leak through the recent files bookmark file. That's all for now. :-) One question left so far: Is it certain that there will be a stable Glib-2.16 release for GNOME 2.22?
Created attachment 103102 [details] [review] gio-migration v2 Thanks for the comments! Attached patch applies on top of previous one and fixes the first three issues. Regarding the fourth thing, I just sent a mail to Alex to ask him. Glib 2.16 is planned before GNOME 2.22...Nautilus 2.22 and Epiphany 2.22 already depend on that. Next GTK+ instead is planned for 2.24.
Created attachment 103160 [details] [review] gio-migration v3 Attached patch fix some issues for opening files from commandline or via D-Bus and adds a FIXME comment about using the async version of g_input_stream_read (). IIRC the sync version was used with gnome-vfs too, but it should be possible to refactor the code to use the async function instead.
(In reply to comment #8) > IIRC the sync version was used with gnome-vfs too, but it should be > possible to refactor the code to use the async function instead. > Yes, it should be. One just needs to make sure the buffer stays valid until the MetadataReader is through with it. But lets finish this one first as it's quite large already. There is another problem I just spotted: The g_file_move in eog_image_copy_file should be a g_file_copy as that's the operation we want to do here (Happens when saving an unmodified image in the same format under a different name). I wonder if that fixes bug 343061.
Created attachment 103275 [details] [review] gio-migration v4 Attached patch fixes some GError leaks and the issue you mentioned in your last comment. Also, bug #343061 seems to be gone using gio :) Saving a symlinked image just works.
Tried it today and it seems to work good so far. :)
Created attachment 103573 [details] [review] Cosimo's V1-4 + fixes So this accumulates Cosimo's patches and tries to fix one crasher and some leaks I found. Crasher: The crasher could occur if you opened a directory by specifying a file inside. If your timing was bad it crashes EOG when closing the window if not it just prints a critical warning: (eog:10738): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed This is fixed by removing an unneeded g_object_unref in eog_list_store_add_files(). Leaks: ==27958== 975 bytes in 195 blocks are definitely lost in loss record 4,135 of 11,046 ==27958== at 0x4026DF8: malloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==27958== by 0x51FA4B5: g_malloc (gmem.c:131) ==27958== by 0x5212B28: g_strdup (gstrfuncs.c:92) ==27958== by 0x41B7AEF: g_local_file_get_uri_scheme (glocalfile.c:311) ==27958== by 0x4193755: g_file_get_uri_scheme (gfile.c:334) ==27958== by 0x80780F4: eog_image_get_caption (eog-image.c:1802) ==27958== by 0x8078323: eog_image_get_collate_key (eog-image.c:1866) ==27958== by 0x808478C: eog_list_store_compare_func (eog-list-store.c:124) ==27958== by 0x474A3F6: gtk_list_store_compare_func (gtkliststore.c:1679) ==27958== by 0x474A835: gtk_list_store_sort_iter_changed (gtkliststore.c:1726) ==27958== by 0x474C59B: gtk_list_store_set_valist (gtkliststore.c:901) ==27958== by 0x474C601: gtk_list_store_set (gtkliststore.c:934) ==27958== by 0x80834BE: eog_list_store_append_image (eog-list-store.c:333) ==27958== by 0x8083A95: eog_list_store_append_image_from_file (eog-list-store.c:353) ==27958== by 0x8083C0D: eog_list_store_append_directory (eog-list-store.c:467) ==27958== by 0x8083E75: eog_list_store_add_files (eog-list-store.c:550) ==27958== by 0x80874DC: eog_job_model_run (eog-jobs.c:386) ==27958== by 0x8085983: eog_render_thread (eog-job-queue.c:80) ==27958== by 0x5219EAE: g_thread_create_proxy (gthread.c:635) ==27958== by 0x4E464A2: start_thread (in /lib/libpthread-2.7.so) ==27958== 67 (20 direct, 47 indirect) bytes in 1 blocks are definitely lost in loss record 8,994 of 11,046 ==27958== at 0x4026DF8: malloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==27958== by 0x51FA4B5: g_malloc (gmem.c:131) ==27958== by 0x520F417: g_slice_alloc (gslice.c:824) ==27958== by 0x520FC04: g_slice_alloc0 (gslice.c:833) ==27958== by 0x51A9339: g_type_create_instance (gtype.c:1549) ==27958== by 0x518E751: g_object_constructor (gobject.c:1046) ==27958== by 0x518D052: g_object_newv (gobject.c:937) ==27958== by 0x518DBD5: g_object_new_valist (gobject.c:986) ==27958== by 0x518DD4F: g_object_new (gobject.c:795) ==27958== by 0x419B2D6: g_file_info_new (gfileinfo.c:268) ==27958== by 0x41BD90B: _g_local_file_info_get (glocalfileinfo.c:1375) ==27958== by 0x41B9635: g_local_file_query_info (glocalfile.c:1000) ==27958== by 0x4192A6F: g_file_query_info (gfile.c:934) ==27958== by 0x8067D88: eog_window_display_image (eog-window.c:1060) ==27958== by 0x80684DE: eog_job_load_cb (eog-window.c:1370) ==27958== by 0x519511E: g_cclosure_marshal_VOID__VOID (gmarshal.c:77) ==27958== by 0x5187737: g_closure_invoke (gclosure.c:490) ==27958== by 0x519D3C8: signal_emit_unlocked_R (gsignal.c:2440) ==27958== by 0x519EEA5: g_signal_emit_valist (gsignal.c:2199) ==27958== by 0x519F2E8: g_signal_emit (gsignal.c:2243) ==27958== 153 (76 direct, 77 indirect) bytes in 1 blocks are definitely lost in loss record 10,467 of 11,046 ==27958== at 0x4026DF8: malloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==27958== by 0x51FA4B5: g_malloc (gmem.c:131) ==27958== by 0x520F417: g_slice_alloc (gslice.c:824) ==27958== by 0x520FC04: g_slice_alloc0 (gslice.c:833) ==27958== by 0x51A9339: g_type_create_instance (gtype.c:1549) ==27958== by 0x518E751: g_object_constructor (gobject.c:1046) ==27958== by 0x518D052: g_object_newv (gobject.c:937) ==27958== by 0x518DBD5: g_object_new_valist (gobject.c:986) ==27958== by 0x518DD4F: g_object_new (gobject.c:795) ==27958== by 0x41BB20F: _g_local_file_enumerator_new (glocalfileenumerator.c:160) ==27958== by 0x41B96AC: g_local_file_enumerate_children (glocalfile.c:543) ==27958== by 0x4192DEF: g_file_enumerate_children (gfile.c:746) ==27958== by 0x8083B5D: eog_list_store_append_directory (eog-list-store.c:491) ==27958== by 0x8083E75: eog_list_store_add_files (eog-list-store.c:550) ==27958== by 0x80874DC: eog_job_model_run (eog-jobs.c:386) ==27958== by 0x8085983: eog_render_thread (eog-job-queue.c:80) ==27958== by 0x5219EAE: g_thread_create_proxy (gthread.c:635) ==27958== by 0x4E464A2: start_thread (in /lib/libpthread-2.7.so) ==21119== 44 bytes in 1 blocks are definitely lost in loss record 10,196 of 11,024 ==21119== at 0x4026DF8: malloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==21119== by 0x51FA4B5: g_malloc (gmem.c:131) ==21119== by 0x520F417: g_slice_alloc (gslice.c:824) ==21119== by 0x520FC04: g_slice_alloc0 (gslice.c:833) ==21119== by 0x51A9339: g_type_create_instance (gtype.c:1549) ==21119== by 0x518E751: g_object_constructor (gobject.c:1046) ==21119== by 0x518D052: g_object_newv (gobject.c:937) ==21119== by 0x518DBD5: g_object_new_valist (gobject.c:986) ==21119== by 0x518DD4F: g_object_new (gobject.c:795) ==21119== by 0x41BE9FB: _g_local_file_input_stream_new (glocalfileinputstream.c:127) ==21119== by 0x41B89C4: g_local_file_read (glocalfile.c:1101) ==21119== by 0x4192134: g_file_read (gfile.c:1221) ==21119== by 0x807967A: eog_image_real_load (eog-image.c:969) ==21119== by 0x807AB82: eog_image_load (eog-image.c:1251) ==21119== by 0x8087744: eog_job_load_run (eog-jobs.c:290) ==21119== by 0x80858D8: eog_render_thread (eog-job-queue.c:78) ==21119== by 0x5219EAE: g_thread_create_proxy (gthread.c:635) ==21119== by 0x4E464A2: start_thread (in /lib/libpthread-2.7.so)
Would be nice to get this in before the 2.21.91 of Feb. 11. That way it gets some more testing.
(In reply to comment #13) > Would be nice to get this in before the 2.21.91 of Feb. 11. That way it gets > some more testing. > Lucas, are you okay with committing this or should we postpone it to 2.23/2.24? The only drawback I was able to spot so far (I only tested it locally (no GVFS) yet) was that it makes fixing bug 490067 a little more complex (bug 513413). It fixes bug 343061 though.
Lucas, ping :) I think that at least the trash part is important for 2.22, because the user won't be able to see the images he deleted from EOG in the trash (trash applet also has moved to GIO).
Hi Cosimo, first of all, I'm really sorry for not being able to review your patch before. Unfortunately, it's too late to apply the whole patch at this point of the development cycle. We should commit the gio-based trash handling for now though. Could you please prepare a separate patch for that?
Created attachment 106249 [details] [review] port trash to GIO Attached patch ports only the trash function to GIO...seems to work just fine in my testing.
Cosimo, this patch breaks string freeze. No go. I'll just use the message for all errors. Patch commited with this change, thanks!
Created attachment 107306 [details] [review] updated patch against latest trunk The old patch didn't apply cleanly anymore,so... I rediffed the patch against current trunk to take the changes (like the trash implementation) in account.
Created attachment 107315 [details] [review] another update This one adds a fix to get the directory monitor working correctly. The problem was that the GFileInfo object was released too early which made the mimetype pointer invalid by the time it was needed. If nothing bad happens I plan to commit this one shortly.
Okay, I've finally committed it to trunk some it gets broader testing. Cosimo, thanks a lot for the patches. I'm splitting that compiler warning fix (eog-pixbuf-cell-renderer.h) off into an extra commit so I can "backport" it to 2.22 as well. 2008-03-14 Felix Riemann <> * configure.ac: * src/eog-application.c: (eog_application_get_file_window), (eog_application_open_file_list), (eog_application_open_uri_list), (eog_application_open_uris): * src/eog-application.h: * src/eog-error-message-area.c: (eog_no_images_error_message_area_new): * src/eog-error-message-area.h: * src/eog-file-chooser.c: (save_response_cb), (set_preview_pixbuf), (update_preview_cb): * src/eog-image-jpeg.c: (_save_jpeg_as_jpeg), (_save_any_as_jpeg): * src/eog-image-private.h: * src/eog-image-save-info.c: (eog_image_save_info_dispose), (is_local_file), (get_save_file_type_by_file), (eog_image_save_info_from_image), (eog_image_save_info_from_uri), (eog_image_save_info_from_file): * src/eog-image-save-info.h: * src/eog-image.c: (eog_image_dispose), (eog_image_init), (eog_image_new), (eog_image_new_file), (eog_image_get_file_info), (eog_image_real_load), (tmp_file_get), (transfer_progress_cb), (tmp_file_move_to_uri), (tmp_file_delete), (eog_image_link_with_target), (eog_image_save_by_info), (eog_image_copy_file), (eog_image_save_as_by_info), (eog_image_get_caption), (eog_image_get_file), (eog_image_get_uri_for_display): * src/eog-image.h: * src/eog-jobs.c: (eog_job_model_new), (filter_files), (eog_job_model_run), (eog_job_save_as_dispose), (eog_job_save_as_new), (eog_job_save_as_real_run): * src/eog-jobs.h: * src/eog-list-store.c: (foreach_monitors_free), (is_file_in_list_store), (is_file_in_list_store_file), (eog_job_thumbnail_cb), (eog_list_store_append_image_from_file), (file_monitor_changed_cb), (directory_visit), (eog_list_store_append_directory), (eog_list_store_add_files), (eog_list_store_remove_image), (eog_list_store_get_pos_by_image): * src/eog-list-store.h: * src/eog-pixbuf-util.c: (get_suffix_from_basename), (eog_pixbuf_get_format): * src/eog-pixbuf-util.h: * src/eog-properties-dialog.c: (pd_update_general_tab): * src/eog-save-as-dialog-helper.c: (set_default_values), (eog_save_as_dialog_new), (eog_save_as_dialog_get_converter): * src/eog-save-as-dialog-helper.h: * src/eog-thumb-view.c: (tb_on_drag_data_get_cb), (tb_on_query_tooltip_cb): * src/eog-thumbnail.c: (set_vfs_error), (eog_thumb_data_new), (eog_thumbnail_load): * src/eog-uri-converter.c: (eog_uri_converter_dispose), (eog_uri_converter_new), (get_file_directory), (split_filename), (append_filename), (build_absolute_file), (eog_uri_converter_do), (eog_uri_converter_preview), (eog_uri_converter_check): * src/eog-uri-converter.h: * src/eog-util.c: (eog_util_parse_uri_string_list_to_file_list), (eog_util_string_list_to_file_list), (eog_util_strings_to_file_list), (eog_util_string_array_to_list), (eog_util_string_array_make_absolute): * src/eog-util.h: * src/eog-window.c: (update_status_bar), (add_file_to_recent_files), (eog_window_display_image), (open_with_launch_application_cb), (eog_window_update_openwith_menu), (eog_window_retrieve_save_as_file), (eog_window_cmd_save_as), (move_to_trash_real), (eog_window_update_recent_files_menu), (eog_window_drag_data_received), (eog_window_dispose), (eog_job_model_cb), (eog_window_open_file_list): * src/eog-window.h: * src/test-eog-tb.c: (make_file), (string_array_to_list), (main): Stop using GnomeVFS. Use GIO instead from now on. Fixes bugs #509239 and #343061 (Cosimo Cecchi, Felix Riemann). ------------------------------------------------------------------------------ This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.