Re: [rdiff-backup-users] HFS+ Resource Fork Patch

[Ben Escoto]

>>>>> "DH" == Daniel Hazelbaker <address@hidden>
>>>>> wrote the following on Wed, 16 Jul 2003 08:50:56 -0700

  >> 1.  You could have picked a more descriptive identifier than '1'
  >> in the C section.  :-) Also python strings can contain arbitrary
  >> data, so the hex conversion may have been unnecessary?

  DH> *grin* sorry, like I said it was a quick patch.  I don't know
  DH> how you configure build-time options (like this) with setup.py,
  DH> so I just did #if 1 for the moment for my own testing.  The
  DH> cmodule should at least be configurable to not do resource
  DH> forks, since it could wreak havoc on a non forked FS.  Again,
  DH> not sure how that would be done with setup.py?  I was thinking
  DH> for the long term something "generic" like HAS_RFORKS.

I know you are more comfortable with C, but I would prefer to have
resource fork abilities autodetected at runtime, so they use a system
similar to that used for extended attributes.  This would set
something like Globals.read_resource_forks, and then in RPath.__init__
there could be an extra step that tests for
Globals.read_resource_forks and then sets self.data['resourcefork'] if
it is true.

You could still use the same C code for reading the resource fork and
turning the raw data into hex if you exported one more function, like
C.get_resource_fork.  However the code would still be
[en/dis]-ableable at runtime or using command line switches.

  DH> As for the hex, it needs to be hex in the metadata file at
  DH> least.  Since it is a text file I don't think it would be good
  DH> to be spitting in nulls and line feed characters as part of the
  DH> resource fork.  In memory though I can probably leave it as
  DH> binary since that will cut down on transmission time by half.

Sorry, I didn't read your patch closely enough, and I missed the part
where you edited metadata.py.  Earlier I thought it wasn't being
written to that file.

  >> 2.  Your file does not compare the resource fork data.  If a
  >> file's resource fork changes but not its actual data,
  >> permissions, etc., I'm not sure it will be detected.

  DH> It gets compared in rpath's equal method by the "default
  DH> handler": elif (not other.data.has_key(key) or self.data[key] !=
  DH> other.data[key]): return None

  DH> I think this should make it check if the file has changed by
  DH> making sure the resource fork data is the same. I am guessing
  DH> anyway, that this method is used as part of the comparison to
  DH> make sure files have been changed before being backed up?

Yes, you are quite right, when I scanned your patch I didn't notice
that the equality patch applied to equal_loose instead of __eq__.

  DH> On the other hand, in compare_loose I had to make it skip
  DH> comparing the resource fork because after a backup the actual
  DH> file would not have a resource fork, thus the resourcefork entry
  DH> would be empty in the array causing it to say it had not backed
  DH> up properly and abort that file. Very annoying :P.  Not sure why
  DH> it doesn't use the metadata it has stored with the file, that
  DH> seems like a Bad Thing.  But that method is only used a couple
  DH> of places and from a quick look through it seemed safe to make
  DH> that change.

equal_loose is used to make sure that the destination file is as close
to the original as it can be given file system constraints.  It is
used as an extra check to make sure that a file hasn't changed while
it was being backed up (besides possibly catching the file in a bad
state, this will mess up the regress code).  So you're method seems
correct.

  DH> Hmm... On thinking of this, actually, unless it loads the
  DH> metadata when it compares a file before backing it up (which it
  DH> should, am I correct?) then __eq__ won't work either...

Yes, when deciding which files to back up it should only read the
metadata file, not the file system.

  DH> I am going to try and get a script together that will diff both
  DH> regular file data and then diff the resource fork data so I can
  DH> run this on a rather large data repository and make sure
  DH> everything is restored properly.  With any luck that will show
  DH> no differences in the data.

If you find any mistakes you may want (and perhaps even if you don't)
you may want to add to rdiff-backup's test suite.

So, to summarize:

1.  I didn't read your patch well enough, everything seems to be fine
    except for the index[0] thing.

2.  A few things are necessary (like not having resource forks a build
    option, and probably a test case) are necessary before this could
    go into the main version.  But resource forks seems very similar
    to the EA/ACL stuff, so there's no reason this couldn't go into
    0.13.0.


-- 
Ben Escoto

pgpGbeI_m4HG2.pgp
Description: PGP signature