[Updated] D10031: revlog-index: add `replace` method

[marmoute (Pierre-Yves David)]

This revision now requires changes to proceed.
marmoute added inline comments.
marmoute requested changes to this revision.

INLINE COMMENTS

> revlog.c:519-536
> +	data = self->added + self->hdrsize * (rev - self->length);
> +	putbe32(offset_flags >> 32, data);
> +	putbe32(offset_flags & 0xffffffffU, data + 4);
> +	putbe32(comp_len, data + 8);
> +	putbe32(uncomp_len, data + 12);
> +	putbe32(base_rev, data + 16);
> +	putbe32(link_rev, data + 20);

Should we explicitly check that no value other than the sidedata have been changed ?

> parsers.py:120-124
> +        """
> +        Replace an existing index entry with a new value. This should
> +        not be used outside of the context of sidedata rewriting, inside the
> +        transaction that creates the revision `i`.
> +        """

The changeset description state that we don't check the transaction thing in the python version. I assume that this is because we have some extra information in the C code that the python version don't have. Could we fix it ?

> parsers.py:128-134
> +        if i >= self._lgt:
> +            self._extra[i - self._lgt] = _pack(self.index_format, *tup)
> +        else:
> +            index = self._calculate_index(i)
> +            self._data[index : index + self.index_size] = _pack(
> +                self.index_format, *tup
> +            )

Same feedback as the C code, could we validate than we don't modify anything else than the side-data index ?

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers, marmoute
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20210303/bbf36e49/attachment-0001.html>