Re: What's cooking in git.git (Aug 2018, #03; Wed, 15)

From: Stefan Beller <sbeller@google.com>
Date: Aug 16, 2018, 14:43 UTC
Message-ID: <CAGZ79kZ3PzqpGzXWcmxjzi98gA+LT2MBOf8KaA89hOa-Qig=Og@mail.gmail.com>
In-Reply-To: <85c6eb4c-a083-4fb7-4860-b01a8ce9fa4f@gmail.com>

On Thu, Aug 16, 2018 at 6:15 AM Derrick Stolee <stolee@gmail.com> wrote:

Show 33 quoted lines

>
> On 8/15/2018 7:01 PM, Junio C Hamano wrote:
> > * ds/commit-graph-with-grafts (2018-07-19) 8 commits
> >    (merged to 'next' on 2018-08-02 at 0ee624e329)
> >   + commit-graph: close_commit_graph before shallow walk
> >   + commit-graph: not compatible with uninitialized repo
> >   + commit-graph: not compatible with grafts
> >   + commit-graph: not compatible with replace objects
> >   + test-repository: properly init repo
> >   + commit-graph: update design document
> >   + refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
> >   + refs.c: migrate internal ref iteration to pass thru repository argument
> >
> >   The recently introduced commit-graph auxiliary data is incompatible
> >   with mechanisms such as replace & grafts that "breaks" immutable
> >   nature of the object reference relationship.  Disable optimizations
> >   based on its use (and updating existing commit-graph) when these
> >   incompatible features are in use in the repository.
> >
> >   Perhaps eject and replace with another reroll when it comes.
> >   cf. <a3640919-95cf-cca4-d552-4715a031dd7f@gmail.com>
> >   cf. <86bmap7l7a.fsf@gmail.com>
>
> Yes, I am working on a re-roll. Feel free to eject in the meantime.
>
> For one, I need to base it on jk/core-use-replace-refs. This is not
> hard, as it is already in master.
>
> For two, I was waiting for resolution on the way to handle arbitrary
> repositories in for_each_replace_ref. Stefan had a few proposed patches,
> but they don't seem to have completed. Can someone fill me in on the
> status there? I could re-roll with the two simple patches that I have,
> which send the struct repository pointer into the 'void *' data parameter.

Yeah, I was exploring the design space there and IIRC these two patches are the best for the commit graph to do for now IMHO. I might resend [1] to remove the first repository argument to make the API cleaner again.

As Duy says in [2], we should
(A) change the construction of ref stores to not take a gitdir path, but
    a repository argument, which is stored internally
(B) change the signature of all the callback functions to take a ref store
    as a first argument, and then we can have a function
      get_repo_from(ref store)
    or put the repo in the callback cookie as you do in this series.

Wth this we do not treat the repository argument any special in the refs code and only if we have the pressing need to have it always available we might want to implement the get_repo_from_refstore instead of treating it just like any other data passed through the cb cookie.

[1] https://public-inbox.org/git/20180730194731.220191-3-sbeller@google.com/ [2] https://public-inbox.org/git/CACsJy8DwaLCxY-ryV+=OwRytzwwQZCfVmfXo0z91z9YRMMT0VA@mail.gmail.com/

tl;dr: please keep these two patches for now, I'll send patches on top if that is where I'll go next.

Thanks, Stefan