D6808: revlog: introduce a `sidedata` method
marmoute (Pierre-Yves David)
phabricator at mercurial-scm.org
Sun Sep 8 04:46:25 EDT 2019
marmoute added a comment.
In D6808#100077 <https://phab.mercurial-scm.org/D6808#100077>, @indygreg wrote:
> I think I see where you are going with this and it seems potentially very useful.
> I do wish the flag processors code could have been refactored without involving `sidedata`, as I would have taken the patches later in this series to remove the temporary `mixin` in a heartbeat. But since `sidedata` is involved, I want to have others weigh in.
The point of the refactoring is to introduce `sidedata` ;-). I could rework the series to get rid of the mixin sooner, but after discussing with Augie on IRC it seemed fine to just do a final cleanup (the mixin discussion arrived after I wrote the other patches).
> It would be extremely useful to see an actual use case for `sidedata`.
My first target is to use it for copy tracing. After the current series I have:
1. a series to add actual storage for sidedata (using a flag processors)
2. a series to refactor access to copy tracing information
3. a series to use sidedata to store copy tracing information
(1) is almost in a visible state (2) and (3) need a bit more work. Which one are you the most earger to see ?
> As it stands, I have questions like whether we'll need to further evolve the storage APIs to accommodate things like cherry-picking which pieces of `sidedata` are desired.
I feel like the API in this patch offer enough room for that already. By all mean, this is the early stage of implementation, it will be easy to adjust the API once we are further down the road.
> Also, incorporating `sidedata` in the revlog write APIs (in later patches) seems a bit odd, as the revlog format is rather stable and I'm not sure how additional data will be incorporated without introducing a new flag processor to accommodate extra data next to the revision.
It will be done by introduce a flag processors ;-)
Having the sidedata specified at `addrevision` time seems okay to me. It highlight the fact these data needs to be immutable property of the revision.
> Again, I'd really like to see a real world use case for `sidedata` to help give me confidence this is the correct storage abstraction.
CHANGES SINCE LAST ACTION
To: marmoute, yuja, durin42, indygreg, #hg-reviewers
More information about the Mercurial-devel