Subject: Re: [PATCH v3 00/33] nd/sha1-name-c-wo-the-repository updates

From: SZEDER Gábor <[email-masked]>
Date: Thu, 11 Apr 2019 22:58:57 +0200

On Thu, Apr 11, 2019 at 10:51:46PM +0200, SZEDER Gábor wrote:
> On Wed, Apr 10, 2019 at 10:56:52PM +0200, Johannes Schindelin wrote:
> > >         ...>}
> > >
> > >       @@
> > >     + expression c;
> > >     ++expression r;
> > >     + expression s;
> > >     + @@
> > >     +-- get_commit_tree(c) = s
> > >     ++- repo_get_commit_tree(r, c) = s
> > >     + + c->maybe_tree = s
> > 
> > I think this is wrong, and admittedly I had the very same version
> > originally.
> > 
> > When you have an arbitrary `r` in any `repo_get_commit_tree(r, c)` (as
> > opposed to `the_repository`), the conversion to `c->maybe_tree` is most
> > likely incorrect.
> > 
> > Therefore, I don't think that we can do that.
> 
> So, as far as I understand, the goal of these 'c->maybe_tree'-related
> semantic patches is to prevent "generic" parts of Git from accessing
> this field directly, as it might not be initialized in a
> commit-graph-enabled repository.
> 
> Only three functions are explicitly exempt, while this last semantic
> patch in question implicitly allows a few more that assign a value to
> 'c->maybe_tree'.  These functions are release_commit_memory() and
> parse_commit_buffer() in 'commit.c' and fill_commit_in_graph() in
> 'commit-graph.c', and after a quick look these functions seem to be
> rather fundamenal in the life-cycle of a commit object.

Erm, not "commit object"; I meant the life-cycle of a 'struct commit'
instance.

> I think they deserve to be explicitly exempted, too, and then we could
> remove this last semantic patch altogether.
> 
>