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 621917 - backups not implemented on ftp backend
backups not implemented on ftp backend
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: ftp backend
unspecified
Other All
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2010-06-17 18:12 UTC by Olivier Sessink
Modified: 2015-02-11 23:22 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
ftp: Implement backups for replace (3.10 KB, patch)
2015-02-07 22:18 UTC, Ross Lagerwall 		needs-work Details | Review
ftp: Implement backups for replace (3.37 KB, patch)
2015-02-10 23:00 UTC, Ross Lagerwall 		committed Details | Review

Description Olivier Sessink 2010-06-17 18:12:37 UTC 
bluefish uses g_file_replace_async() with backup=TRUE, but on the ftp backend the backup alwais fails.

starting

GVFS_DEBUG=1 /usr/lib/gvfs/gvfsd

reveals:

send_reply(0x1ba80c0), failed=0 ()
backend_dbus_handler org.gtk.vfs.Mount:OpenForWrite
Queued new job 0x1c01810 (GVfsJobOpenForWrite)
send_reply(0x1c01810), failed=1 (backups not supported yet)
Comment 1 Ross Lagerwall 2015-02-07 22:18:47 UTC 
Created attachment 296353 [details] [review]
ftp: Implement backups for replace
Comment 2 Ondrej Holy 2015-02-10 16:50:05 UTC 
Review of attachment 296353 [details] [review]:

Looks good otherwise...

::: daemon/gvfsbackendftp.c
@@ +998,3 @@
+          if (!g_vfs_ftp_task_send (&task,
+                                    G_VFS_FTP_PASS_550,
+                                    "DELE %s", g_vfs_ftp_file_get_ftp_path (backupfile)))

I don't like to see those multiline condition statements, wouldn't be better something like:
res = g_vfs_ftp_task_send (...)
if (!res) { ...

@@ +1001,3 @@
+            {
+              g_vfs_ftp_file_free (backupfile);
+              goto out;

I wonder if we should fail with G_IO_ERROR_CANT_CREATE_BACKUP in those cases...

@@ +1024,3 @@
+
+          g_vfs_ftp_dir_cache_purge_file (ftp->dir_cache, file);
+          g_vfs_ftp_dir_cache_purge_file (ftp->dir_cache, backupfile);

g_vfs_ftp_file_free (backupfile);
Comment 3 Ross Lagerwall 2015-02-10 23:00:25 UTC 
Created attachment 296548 [details] [review]
ftp: Implement backups for replace
Comment 4 Ondrej Holy 2015-02-11 08:52:41 UTC 
Looks good, thanks :-)
Comment 5 Ross Lagerwall 2015-02-11 23:22:28 UTC 
Pushed to master as 5620bd8acb05c0a4eb93a792b25c349c5c6efebd. Thanks for the review!