Changesets feature complete

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
> 
>>> Now that we are using Testaments, I'm not planning on trying as hard to
>>> be able to pre-validate the changeset before we install stuff. As long
>>> as we add in signatures, and check them before we pull a new revision,
>>> we can be reasonably guaranteed that the changeset is valid.
> 
> Well, signatures prove changesets are authentic, but not that they're
> valid, because a changeset that failed to properly represent changes to
> the last-modified attribute would produce a revision with a correct
> Testament that was not a true copy of the source revision.
> 
> My most recent changes add a new StrictTestament that includes
> last-modified, and we should probably also include the execute bit in
> StrictTestaments.

I'm not talking about signing the changeset. I'm talking about including
the revision signature. Which should have been generated on the original
revision, and would thus prove that the revision matches the original.

(I do think we want to be able to sign a changeset, but that can be done
at a higher level)

If the execute bit wasn't in Testament, I think that is a bug. I'm also
not sure about the last-modified. It is unfortunate that we will need a
new testament format to handle any changes, but my understanding of
Testament is that it is a streamlined and well defined list, but should
include anything that you truly want to declare about the revision. So
if 'last-modified' is important, then it should be in there.

> 
>>> Well, I guess I would like to say "set it for every change, but consider
>>> the default to be 'no' for new files".
>>> I think it is a very reasonable default, and in my head, when the file
>>> didn't exist, it certainly wasn't executable. :)
> 
> That's fine with me, as long as the changes are in
> ReadChangeset._update_tree, rather than ChangesetTree.

I don't see the problem with changing ChangesetTree. you just have a
3-conditional if statement:

if file_id in self._executable:
  # It was present in the changeset
  return self._executable[file_id]
elif file_id in self._base_inventory:
  return self._base_inventory[file_id].executable
elif:
  # This is a new addition, default to non-executable
  return False

We could change the reader so that it always supplies the executable bit
to the ChangesetTree, but that seems unnecessary.

> 
>>> Well, in my mind, the text format won't change much after the
>>> refactoring. It is just changes to the internal code. So how about we
>>> merge it, and just make sure that we are clear it needs to be cleaned up.
> 
> In that case, I will make a few tweaks, and get a merge request together.
> 
>>> The point of the serializer was just to have a factory which could be
>>> used to read and write changesets. The api for a ChangesetSerializer was
>>> *very* simple, just a read() and write() function.
> ...
>>> As far as what 'read' would return, I wasn't fully clear on that myself.
>>> At one point, I was thinking that it should be a set of ChangesetTrees,
>>> where each tree was equivalent to a RevisionTree (it has the revision
>>> info, and the inventory info).
> 
> But the problem is that the trees use each other as bases, so
> Serializer.read() would have to install them in a repository.
> 
>>> However, I could also see returning a more complicated object than a
>>> list, (similar to ChangesetReader/ChangesetInfo). This object would have
>>> the information to create a ChangesetTree on request by revision_id.
>>> This makes it easier to do late parsing of the changeset, which helps in
>>> the case that you already have some of the changeset revisions.
> 
> I think this is a better way to go.  How about this API:
> 
> ChangesetInstaller(object):
>     def install(repository, revisions=None):
>         pass
> 
>     def revision_ids():
>         pass
> 
>     def target_revision_id():
>         pass
> 
> 
>>> In my refactoring, I was working on moving non-parsing stuff out of
>>> ChangsetReader and into ChangesetInfo, and making that the return value
>>> of read().
>>>
>>> (I also changed the upfront merge command, so that it could grab a
>>> changeset over a Transport).
> 
> Cool.  And now that branches use leftmost ancestors for revision
> history, we can implement pull, too.
> 
> Aaron

We certainly could. Actually the trickiest part is that Transports
return all sorts of things when you 'get()' a directory. Http might
return an index, LocalTransport raises an exception, and sftp doesn't
raise an exception until you try to read() the directory. (I don't know
what Ftp does).

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060525/411b7fb2/attachment.pgp