After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 519651 - write support in gphoto2 backend
write support in gphoto2 backend
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: gphoto backend
unspecified
Other Linux
: Normal normal
: ---
Assigned To: David Zeuthen (not reading bugmail)
gvfs-maint
Depends on:
Blocks: 520121
 
 
Reported: 2008-02-29 23:29 UTC by David Zeuthen (not reading bugmail)
Modified: 2008-03-03 19:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add write support to gvfs gphoto2 backend (105.36 KB, patch)
2008-03-03 01:28 UTC, David Zeuthen (not reading bugmail)
none Details | Review

Description David Zeuthen (not reading bugmail) 2008-02-29 23:29:06 UTC
We want write support in the gphoto2 backend.
Comment 1 David Zeuthen (not reading bugmail) 2008-02-29 23:30:03 UTC
Taking ownership.
Comment 2 David Zeuthen (not reading bugmail) 2008-03-03 01:27:23 UTC
Here's a patch that adds write support. Some caveats; I'm just pasting this from the TODO section in the beginning of the file

- write support
  - it's in; we support writing. yay.
    - though there's no way to rename a whole folder yet
  - there's an assumption, for caching, that the device won't
    be able to put files while we're using it. May have to
    revisit that if such devices exist.
    - one solution: make cache items valid for only five seconds or something

  - Note that most PTP devices (e.g. digital cameras) don't support writing
    - Most MTP devices (e.g. digital audio players) do

  - However, some MTP devices are just busted when using ~ backup
    style; see below. This is with my (davidz) Sandisk Sansa
    e250. This is probably a firmware bug; when investigating
    libgphoto2 reports everything as peachy.

       $ pwd
       /home/davidz/.gvfs/gphoto2 mount on usb%3A001,051/foo
       $ echo a > a
       $ echo b > b
       $ ls -l
       total 0
       -rw------- 1 davidz davidz 2 2008-03-02 13:22 a
       -rw------- 1 davidz davidz 2 2008-03-02 13:22 b
       $ cat a
       a
       $ cat b
       b
       $ mv b a
       $ ls -l
       total 0
       -rw------- 1 davidz davidz 2 2008-03-02 13:22 a
       $ cat a
       b
       $ rm a

    See, this worked fine.. Now see what happens if we
    use different files names

       $ echo a > file.txt
       $ echo b > file.txt~
       $ ls -l
       total 0
       -rw------- 1 davidz davidz 2 2008-03-02 13:22 file.txt
       -rw------- 1 davidz davidz 2 2008-03-02 13:22 file.txt~
       $ cat file.txt
       a
       $ cat file.txt~
       b
       $ mv file.txt~ file.txt
       $ ls -l
       total 0
       -rw------- 1 davidz davidz 0 1969-12-31 18:59 file.txt
       $ cat file.txt 
       $

    Awesome. I hate hardware.

    - This is a bit bad as it affects most text editors (vim, emacs,
      gedit) and it actually results in data loss. However, there's
      little we can do about it.

    - Would be nice to test this on other MTP devices to verify
      it's indeed a firmware bug in the Sansa Sandisk e250.

    - This shouldn't affect stuff like Banshee or Rhythmbox using
      this backend for MTP support though; despite this bug basic
      file operations works nicely.

    - Need to test this with a native gio version of gedit that should
      use replace() directly instead of fooling around with ~-style
      backup files

Apart from those caveats it works pretty well. I've also made better use of the cache through implementing try_query_info, try_query_fs_info and try_enumerate that only succeeds if the cache is valid. Browsing (previously known and unchanged) folders in Nautilus is now a breeze even though heavy IO (e.g. copying data from/to the device) is happening.

Also, cleaned up the code a bit and added more docs.

One question I have is this. Where am I supposed to put the cursor in append_to()? At the beginning of the file or at the end? The only user of this is the fuse backend and it does a SEEK_SET anyway.

Alex, does this look OK to commit?
Comment 3 David Zeuthen (not reading bugmail) 2008-03-03 01:28:44 UTC
Created attachment 106442 [details] [review]
add write support to gvfs gphoto2 backend

$ diffstat gvfs-gphoto2-add-write-support.patch 
 gvfsbackendgphoto2.c | 2761 +++++++++++++++++++++++++++++++++++++++++++--------
Comment 4 David Zeuthen (not reading bugmail) 2008-03-03 01:32:27 UTC
Also, forgot to mention, reading is now a bit faster too as we don't have to schedule read() on an IO thread; I just replaced do_read(), do_seek_for_read() with their try_* versions. We can do this because the way reading works is that we fetch the whole file in open_for_read() and store it in memory [1].

[1] : I wish libgphoto2 had an API for partial reads. But it doesn't looks like it.
Comment 5 David Zeuthen (not reading bugmail) 2008-03-03 01:49:57 UTC
Btw, wrt to string freeze

> $ cat gvfs-gphoto2-add-write-support.patch |grep "_(\""
>    fuse_name = g_strdup_printf (_("gphoto2 mount on %s"), gphoto2_backend->gphoto2_port);
> +                        _("Can't open directory"));
> +                        _("No such file"));

These strings already exist in other backends. The remaining strings (only error messages) are marked for future translation.
Comment 6 David Zeuthen (not reading bugmail) 2008-03-03 19:38:00 UTC
Committed. I talked to alex on IRC and re comment 2 he told me to put the cursor at the end.

2008-03-03  David Zeuthen  <davidz@redhat.com>

        Add write support to gphoto2 backend. Also performance
        enhancements for querying, enumerating and reading.
        Fixes bug #519651

        * daemon/gvfsbackendgphoto2.c: (monitor_proxy_free), (DEBUG),
        (write_handle_free), (ensure_not_dirty), (dup_for_gphoto2),
        (monitors_emit_internal), (monitors_emit_created),
        (monitors_emit_deleted), (monitors_emit_changed),
        (caches_invalidate_all), (caches_invalidate_free_space),
        (caches_invalidate_dir), (caches_invalidate_file),
        (get_error_from_gphoto2), (release_device),
        (g_vfs_backend_gphoto2_finalize), (_gphoto2_logger_func),
        (g_vfs_backend_gphoto2_init), (find_udi_for_device),
        (_hal_device_removed), (split_filename_with_ignore_prefix),
        (add_ignore_prefix), (file_get_info), (is_directory), (is_regular),
        (is_directory_empty), (ensure_ignore_prefix), (do_mount),
        (try_mount), (do_unmount), (free_read_handle), (do_open_for_read),
        (try_read), (try_seek_on_read), (do_close_read), (do_query_info),
        (try_query_info), (do_enumerate), (try_enumerate),
        (do_query_fs_info), (try_query_fs_info), (do_make_directory),
        (do_slow_file_rename_in_same_dir), (do_file_rename_in_same_dir),
        (do_dir_rename_in_same_dir), (do_set_display_name), (do_delete),
        (do_create_internal), (do_create), (do_replace), (do_append_to),
        (do_write), (do_seek_on_write), (commit_write_handle),
        (do_close_write), (do_move), (vfs_dir_monitor_destroyed),
        (do_create_dir_monitor), (vfs_file_monitor_destroyed),
        (do_create_file_monitor), (g_vfs_backend_gphoto2_class_init):