Daniel Shahaf <[email protected]> writes: > I don't think it depends on the implementation details; I think it's > a valid revision file, full stop. The revision file invariant is that > everything reachable from the two offsets in the last line of the file > be valid --- and that indeed is the case for the revs/0/1 file in your > repository. > > Yes, there are some byte offsets, node-revs, reps, etc in the file which > are not reachable from the two offsets in the last line, but that > doesn't make the file invalid.
Yes, I agree. Provided the parser only parses good offsets it will ignore the extra data, and that's what happens when starting with the root node indicated by the final offsets in the file. I still think it's unfortunate that the extra data gets into the revision file. There is also a problem associated with transaction properties. The code removes the temporary properties SVN_FS__PROP_TXN_CHECK_OOD and SVN_FS__PROP_TXN_CHECK_LOCKS that may have been set when the transaction was created. If the commit fails after removing these properties the checks they trigger will not occur on any subsequent attempts to modify the transaction. CHECK_OOD isn't implemented yet and CHECK_LOCKS only affects the timing of the checks, not the final result, so there should be no effect on any revision that does get committed. I still think it's unfortunate that the properties are lost. Finally, there is a window between moving the protorev file into place and moving the revprop file into place. If interrupted during this period the revision does not exist because the revprop file is missing but the transaction is broken because there is no protorev file. Retrying the commit will fail. The client will have to repeat the whole commit and create a new transaction. -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*
