[MERGE] journalled inventory serialiser

John Arbash Meinel john at arbash-meinel.com
Mon Jan 14 20:46:00 GMT 2008


John Arbash Meinel has voted comment.
Status is now: Semi-approved
Comment:
format. (Robert Collins)
-------------- This line and the fmllowing will be ignored 
--------------

modified:

^-- looks like you messed up the "the following" which caused the delta 
to be added as the comment....

It confused me for a second when I saw a patch to review which didn't 
quite match your comments. :)


I thought I would start with a review of the journaled inventory 
document, since that should be a higher overview, before we get into the 
implementation details. As this is development/work in progress, I'm 
probably okay with bringing it in while it is still rough. Though I'm 
curious about how we convey "*this* development format is ready for you 
to play with, *this* evolution is not" other than by its presence in 
bzr.dev.
I know it says "--development" but you are committing to at least a 
converter to the format and one to the next format.


=== modified file 'doc/developers/inventory.txt'
--- doc/developers/inventory.txt        2008-01-03 01:40:16 +0000
+++ doc/developers/inventory.txt        2008-01-07 22:15:40 +0000
...

+In a journalled inventory we represent hte inventory as a tree of
^- hte => the

...

+We will serialise the lines text for a new inventory as:
+'format: bzr journalled inventory v1' NL
+'parent:' SP BASIS_INVENTORY NL
+'version:' SP NULL_OR_REVISION NL
+DELTA_LINES

^- It might be good to have a line counter here, rather than just 
assuming
that the file is perfectly well formed. I know you have the validator
(which I assume is stored separately), it can just be nice to have a bit
more to go on.

I can understand if you would rather not because of buffering issues.
Though we should certainly validate before processing on the way in, 
which
requires either reading 2 times, or buffering.

Are we going to assume you can fit the full contents of a journal in
memory? It seems reasonable, but then there are people who tried
versioning all files of all programs in the debian source repository.


+DELTA_LINES ::= (DELTA_LINE NL)*
+DELTA_LINE ::= NEWPATH NULL file-id NULL PARENT_ID NULL LAST_MODIFIED 
NULL CONTENT
+SP ::= ' '
+NULL ::= \x00
+NEWPATH ::= NONE | PATH
+NONE ::= 'None'
+PATH ::= path
+PARENT_ID ::= FILE_ID | ''


^- Do we really want a literal "None" on disk? It seems like that would
prevent you from versioning a file named "None".
Why not just have it be an empty string?  I suppose the tree root has 
path
''. Either way, I think we need a clear "not a path", not a simple 
string
like 'None'.

Also, in 'DELTA_LINE ::= ...' you use 'file-id', but you have: PARENT_ID
::= FILE_ID | ''

You should either capitalize both, or leave them both lower case.

...

+There are two ways that we could create new journalled inventories. We
+could take the in memory inventory that we create during commit, and
+that for the basis, then make a new delta and serialise it to create 
the

^- I don't understand "during commit, and that for the". Do you mean 
"add
that" or "use that"?

+revision. Additionally doing this we could serialise the inventory in
+its regular form to get a legacy validator that has no reference to

^- Additionally, while doing this we could ...

+By using the same hueristic as knits do, we need to read the same 
amount
+of data to construct a given inventory. We will need to parse the 
entire
+data set of data rather than just the result of applying the text data.

^- "We will need to parse the entire set of data" not "data set of 
data".
And I think you mean "than applying the text delta".

...

+Iter-inventories
+^^^^^^^^^^^^^^^^
+We can apply deltas forward or backwards to move along a chain of
+inventories trivially, rather than the current requirement of
+deserialising them all individually.

^- You should have a blank space here. Also, as you have defined them,
these do not seem to be reversible deltas.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1199746830.1954.76.camel%40lifeless-64%3E



More information about the bazaar mailing list