Re: Fragile GnomeVFSXfer copying code - suggestions?

[Alexander Larsson <alexl redhat com>]

On Fri, 2006-03-24 at 11:14 +0100, Christian Neumair wrote:
> I've found the current Xfer code to be very fragile when copying, and
> doing an overwrite.
> 
> When you move a directory, and force to overwrite an existing one, the
> directory is removed before the move process begins, i.e. it is really
> replaced.
> 
> When copying, the Xfer code however tries to merge the old directory
> entries with the new ones, overwriting only conflicting ones.
> 
> The current code checks for conflicts using
> gnome_vfs_directory_visit_uri (invoked from
> handle_merged_directory_name_conflicts), which visits the directory
> non-recursively (cf. gnome-vfs-directory.c:directory_visit_internal,
> recursive is always FALSE). Whenever it encounters two directories, it
> does not bail on conflict, the code obviously originally was meant to
> merge the whole old with the whole new hierarchy recursively.
> 
> So when you had a directory like
> 
> /target/foo/bar
> 
> and copy from a directory like
> 
> /source/foo/bar

Are you sure this is what happens?
handle_merged_directory_name_conflicts calls
gnome_vfs_directory_visit_uri() with handle_merged_name_conflict_visit
as callback, and that sets *recurse to true unless that would cause a
loop, so it should be doing a recursive visit.

> a) Use the same overwrite semantics for copy as for move, i.e. when two
> directories conflict, and an overwrite was demanded, just remove the
> previous directory. This would mean that conflicts are only handled
> (consistently!) non-recursively. In the example above, foo in target
> would simple be rmdir'ed before the source directory is be copied over.
> b) Write some code similar to handle_name_conflicts which handles
> conflicts recursively, i.e. somehow invokes
> handle_merged_directory_name_conflicts and does the same conflict check
> that we do for the toplevel directory for the subdirectories before
> doing its copy_ foo.
> c) Make gnome_vfs_visit_uri work recursively. I don't have any expertise
> in this area, and fear that I'll break something - some functions may
> rely on its non-recursive semantics, maybe we need a RECURSIVE flag.
> 
> I personally prefer a), because it is the simplest implementation (we
> can reuse move code), and because that's what I'd personally expect,
> i.e. whether you pick copy or move should IMHO only be a flag for
> "source files will still be in place after copy", and not for some
> additional target merging voodoo.
> 
> Comments, suggestions?

We used to have the OSX semantics for directory overwrite (remove the
whole target directory), but a lot of people reported major data loss
due to this, so we switched to a merging semantics. I think we should
use such merging semantics for both move and copy. In fact, I think we
already do this. Whenever we detect a name conflict when moving a
directory we switch that folder from a move to a copy just in order to
handle this case. Unless you've found a bug that causes this to not
happen.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a deeply religious dishevelled assassin who must take medication to keep 
him sane. She's a chain-smoking goth vampire with the soul of a mighty 
warrior. They fight crime!