[PATCH] Changesets for bzr

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> Aaron Bentley wrote:
> 
>>John Arbash Meinel wrote:
> Sorry I was unclear. I was meaning the returned object would be capable
> of installing. Not the read() function. I want to be able to read
> changesets without acting on them.

Okay, great.

>>>>I'm thinking we should at least not import it into builtins.py until we
>>>>get it cleaned up.
>>
>>I don't think we are.
> 
> 
> Well, I can run 'bzr send-changeset' and have it fail. So somehow we are.

~/bzr/w-changeset$ ./bzr send-changeset
bzr: ERROR: unknown command 'send-changeset'

That's the kind of failure I get.

>>>>I think _validate_inventory is now bogus, since it expects an exact
>>>>inventory serialization. Which we no longer guarantee.
>>
>>It's not entirely bogus.  For old revisions, you may get a warning, but
>>for new revisions you won't.  I was considering it a last line of
>>defence against corrupt changesets.  Perhaps we should include the real
>>sha1 and use that instead?
> 
> 
> I thought that is what we were using StrictTestament for.

Well, StrictTestament was the first line of defence.  I have trouble
trusting it completely, because it's possible for us to forget about
something.

>>>>I realize I am wanting to use
>>>>that fact in the current format. But what happens in the future when you
>>>>have a new format, and you have an add. It means a 'note_add' *always*
>>>>has to be accompanied by a 'note_executable', 
>>
>>Where's the harm in that?
> 
> 
> I don't really prefer an api where you always have to call a second
> function if you call the first. It just feels wrong. If one requires the
> other to be called, then it should call it.

If you want to get technical, node_add, note_file_id, and usually
note_patch or note_target are all required in order to have a valid
ChangesetTree.

The design was a precursor to TreeTransform, and like TreeTransform
gives you the freedom to make the changes in any order.  A future format
might note the add long after the execute bit status was known.  Or vice
versa.

But also like TreeTransform, I'm happy to build convenience methods on
top of the basic API.

>>>>I thought with the inventory fixes we might either have no executable
>>>>attribute, or it would at least be None for non-files.
>>
>>Your ChangesetTree patch also sets an execute bit value for non-files.
>>
>>But this is actually correct.  The execute bit is a member of
>>InventoryEntry, so it applies to symlinks and directories.  It is set
>>False by default.  So now I'm asserting that all inventory entries have
>>an execute bit, and it's False for non-files.

> I don't know if you followed it, but there was work done on making
> InventoryEntry uses __slots__ everywhere, rather than just on the base
> class. And at that time they removed a bunch of the members that don't
> make sense to all classes.

But just as you said "if a file doesn't exist, then it's not
executable", one could say "if it's not a file, then it's not executable".

Anyhow, w-changeset is compatible with the current semantics.  If they
change, we can update it.

> But we can clean it up when that gets merged. (I've +1'd it, but waiting
> on a second review).

I don't understand slots, so I haven't reviewed it.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEdxe10F+nu1YWqI0RAuBTAJ4uhnnOsa1cKte+zwKL7nSQJUuzcwCbBCtZ
Ysz69+aWBLuKA+NOENiGvZY=
=dj8j
-----END PGP SIGNATURE-----