Re: Fragile GnomeVFSXfer copying code - suggestions?

[Alexander Larsson <alexl redhat com>]

On Fri, 2006-04-21 at 14:07 +0200, Christian Neumair wrote:
> Am Donnerstag, den 20.04.2006, 17:09 +0200 schrieb Alexander Larsson:
> > 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.
> 
> Yes, you are right. The general weakness of the current code is that it
> completely visits the source directories during preflight, but doesn't
> handle errors properly that occur during the transfer. I wonder why we
> don't shift the whole conflict handling to the transfer.

Yes. That is the main weakness. Doing things in this order is inherently
racy, so it would be nice to redo it to do all checking while doing the
operation instead. Of course, from a UI perspective it nicer to get told
of all the conflicts initially instead of halfway into the operation,
which might not even be reversable at that point... I think that is the
reason why it was designed like it is.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a maverick day-dreaming cop on his last day in the job. She's a sarcastic 
red-headed barmaid operating on the wrong side of the law. They fight crime!