[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [rdiff-backup-users] HFS+ Resource Fork Patch
From: |
Ben Escoto |
Subject: |
Re: [rdiff-backup-users] HFS+ Resource Fork Patch |
Date: |
Wed, 16 Jul 2003 12:27:14 -0700 |
>>>>> "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