I see this too...for what I can see we're not doing anything fancy in Nautilus to restore files from trash (it should be just a simple g_file_move ()), so the bug may be in GVfs...reassigning.

Files are trashed with g_file_trash(), which uses g_rename(). This is very snappy compared to the full-fledged file copy machinery (i.e. g_file_move()).

Although it is very fast, I am not sure whether this kind of trashing is a good idea. For instance, it does _not preserve file metadata_ (it is even explicitly removed in libnautilus-private/nautilus-file-operations.c:trash_files()).

I forgot to mention that this also involves another issue: We do not mmap trash:/// URIs back to local files before carrying out operations on them. This could allow the g_file_move() code to use the GLocalFile move code, without falling back to copy [which is slow].

*** Bug 537224 has been marked as a duplicate of this bug. ***

Setting priority to High, this is a very visible regression.

Can somebody come up with a patch for 2.22.3 (June 30)? I can only second Cosimo here.

Hmm, pity. Anyway, anybody willing to come up with a patch?

I just discussed this with Alex, and he proposed to add this functionality to gvfs in the following way:

* daemon/gvfsbackend.c - GVFSBackend should have a try_download() member function (cf. try_upload())
* client/gdaemonfile.c - g_daemon_file_copy should use try_download() for moving a file from a daemon source to a non-daemon destination (cf. G_VFS_DBUS_MOUNT_OP_UPLOAD)
* the trash backend should implement this try_download() function to fall back to g_rename(), in a similar way the burn backend uses try_upload().

Unfortunately, my exams are very near. Feel invited to work on it, though :)

Created attachment 115039 [details] [review]
extremely ugly first take

Quick pass at this. Already notice there are going to be problems; we're assuming the copy+delete path, however g_rename() moves the file, so you get a pretty "Can't delete file" dialog after the copy operation succeeds. We may also need to patch Nautilus to correctly check for free space.

Probably needs some sanity checking, and a good overview of the fallback system would be very much appreciated. And there are a hundred other things I'm likely missing, feel free to point them out.

*** Bug 545684 has been marked as a duplicate of this bug. ***

while it's an annoying regression wasting electricity, cpu cycles and some cats, i think we have more important stuff that are real blockers. removing target milestone.

Oh yeah, this _is_ a real issue. Please somebody take a look at this to get rid of this from 2.24. Thanks

I am already at this.

Maybe I should explain whats going on:

1) Only implementing a "download" method that natively handles pulling from remote backends to the local filessystem is not enough since we also need to distinguish between copy and moves here.
Restoring from trash is going needs to kick in not in the copy control flow but in the move control flow, thus we don't need a download but rather a "steal" method. Ryan come up with the name.

The question that remains is that whether we should have seperate "Download" + "Steal" methods (and then maybe also a "Give" method to complete the "Upload" method) or just have a "Pull" and "Push" method with an additional "gboolean remove_source" in case of move (port upload to push, and add an aditional pull method).

Opinions, Alex?

Created attachment 117625 [details] [review]
First take on "Download" + "Steal"

Here is my fist take which adds "Download" PLUS "Steal" and implements those inside the HTTP Backend and also in the Trash backend.
I tested this and that patch would indeed fix the bug.

gicmo@photon ~/Devel/gvfs % cat download.patch| diffstat
 client/gdaemonfile.c        |  210 +++++++++++++++++++++++++++------------
 common/gvfsdaemonprotocol.h |    2 
 daemon/Makefile.am          |    2 
 daemon/gvfsbackend.c        |   10 +
 daemon/gvfsbackend.h        |   30 +++++
 daemon/gvfsbackendhttp.c    |  235 +++++++++++++++++++++++++++++++++++++++++---
 daemon/gvfsbackendtrash.c   |  130 ++++++++++++++++++++++++
 daemon/gvfsjobdownload.c    |  211 +++++++++++++++++++++++++++++++++++++++
 daemon/gvfsjobdownload.h    |   66 ++++++++++++
 daemon/gvfsjobsteal.c       |  226 ++++++++++++++++++++++++++++++++++++++++++
 daemon/gvfsjobsteal.h       |   66 ++++++++++++

Created attachment 117798 [details] [review]
Another take ...

This time we only have Push and Pull, not Give, Steal, Upload, Download. 

I quite the Master:
"I don't think it matters a lot really, but I'd personally lean towards
the "less methods" way."

Committed a slightly different version to svn trunk. Hopefully this is dead now. ;-)

2008-09-01  Christian Kellner  <gicmo@gnome.org>

	* daemon/gvfsjobpull.c:
	* daemon/gvfsjobpull.h:
	New daemon method to pull files from daemons to the local
	file system.

	* daemon/gvfsjobpush.c:
	* daemon/gvfsjobpush.h:
	New daemon method to push local files to daemons. (Replaces
	Upload)

	* daemon/gvfsjobupload.c:
	* daemon/gvfsjobupload.h:
	Replaced by Push.

	* common/gvfsdaemonprotocol.h:
	* daemon/Makefile.am:
	* daemon/gvfsbackend.c:
	* daemon/gvfsbackend.h:
	Add Push and Pull. Remove Upload.

	* client/gdaemonfile.c:	
	New transfer logic. In case of move try push/pull (with
	remove_source set to TRUE) first then fallback. In case of copy
	try push/pull first then fallback. Use the same logic for
	both cases.

	* daemon/gvfsbackendburn.c:
	Port Upload to Push.
	
	* daemon/gvfsbackendtrash.c:
	Implement the Pull method. That should fix bug #529971