Re: [PATCH] Add directory diff options for shallow file comparison mode

[Kai Willadsen <kai willadsen gmail com>]

On 28 December 2012 14:18, Cristian Dinu <goc9000 gmail com> wrote:
> Hello all,
>
> I'm attaching a patch that adds an option for performing 'shallow' directory
> comparisons, i.e. trusting the file size and modification time. This is
> useful for folks like me who would also like to use Meld as a
> general-purpose tool for comparing their current data (documents, music
> etc.) to what is stored on a backup drive. Normally Meld always performs a
> costly binary comparison even if the size and mtime match, which is
> unreasonable and prohibitively expensive for such a scenario.

I've resisted adding this option in the past, but I think I'm ready to
give up at this point and just add something. However, I do want to be
careful about how it's added, and what we support.

> Because such comparisons may occur between different filesystems (e.g.
> backing up your ext4 files on a NTFS or FAT drive), I have also added two
> more options to compensate for two problems that may occur. First, file
> permissions may not translate exactly across filesytems, so there is the
> option to ignore differences in permissions (however, files differing in
> other aspects of the mode are still distinguished).

Is this a problem for anything other than FAT drives in practice? And
in any case, does it really make sense to declare that files with
differing modes are different? (I know that some tools do, but that
doesn't mean we have to.)

> Second, different
> filesystems may and often do have different timestamp resolutions, so I've
> added an option whereby the user can normalize the timestamps to a common
> resolution, allowing for a correct comparison.

...and as above? Does anything other than FAT have a
worse-than-1-second resolution?

If these two options are here solely for the benefit of external
FAT(32)-formatted drives (and systems that don't support sub-second
resolution) then can we deal with these without additional
preferences?

If we do need to support a timestamp resolution preference, then I
think we should provide fixed options (e.g., exact, 100msec, 1sec,
2sec, 1 hour, 1 day) rather than a continuous slider.

> The three new options have been added to a "Directory Comparison" tab in the
> Preferences dialog.

Assuming this goes ahead, I think that tab will end up being the
second in the dialog, and we should definitely move the
follow-symlinks checkbox into it. However, that's a separate issue.

> Looking forward to your comments!

The patch is a good start! However, you're touching *two* things that
I'm cautious about --- UI and preferences --- so some patience might
be required to get this done.

I'd prefer to review the patch on bugzilla, and there is already at
least one bug requesting shallow comparisons that you could attach it
to. Having said that, here are some initial comments from a quick
look:
 * The changes in meldapp and the signal emission and such aren't
needed at all. Just add an on_preference_changed() method to DirDiff
and do the options handling there.
 * Getting PreferencesDialog to accumulate the preferences for you
feels unnecessary; again, just have DirDiff do it. Also, the
__getattr__ stuff is overkill... just manually grab the prefs from
their keys.
 * The superficial comparison branch in _files_same only ever
shortcuts for identical files. I'd expect instead that if _files_same
gets to the superficial comparison, we will either have DodgySame or
DodgyDifferent as a result right there. (Unless I'm missing something,
and there's a good reason to have it otherwise).

cheers,
Kai