[Commented On] D11094: dirstate-v2: Support appending to the same data file

[marmoute (Pierre-Yves David)]

marmoute added inline comments.

INLINE COMMENTS

> SimonSapin wrote in dirstatemap.py:672
> I inlined `append` from `vfs.py` in order to be able to add the assert with `.tell()`. Is `vfs.py` also buggy?
> 
> Silently overwriting extra data seems very wrong to me. If someone else wrote it, overwriting it would interfere with them. However due to the combination of `repofilecache` + `wlock`, my understanding is that there should never be extra data.

I am refering to the issue pointed here: https://www.mercurial-scm.org/repo/hg/file/stable/mercurial/revlog.py#l2427

So `vfs.append` is possibly buggy :-/

There should never be extra data, except if we decide this is a good idea in some case to being friendlier to mmap user. For example, we might decide to do so in `hg rollback` or decide that truncate less transaction rollback is useful.
So, even if this is corner case, having code robust in this case seems better.

> on_disk.rs:666
> +        // they refer to.
> +        let start = self.curent_offset();
> +        let len = u32::try_from(nodes_len)

spotted on the go "curent_offset" instead of "current_offset"

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11094/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D11094

To: SimonSapin, #hg-reviewers
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210716/3ddbaa7d/attachment-0002.html>