On 6/3/2022 5:30 AM, Ævar Arnfjörð Bjarmason wrote:
>
> On Thu, Jun 02 2022, Jonathan Tan wrote:
>
>> diff --git a/commit.c b/commit.c
>> index 59b6c3e455..1537ea73d0 100644
>> --- a/commit.c
>> +++ b/commit.c
>> @@ -120,6 +120,17 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid)
>> commit_graft_oid_access);
>> }
>>
>> +static void unparse_commit(struct repository *r, const struct object_id *oid)
>> +{
>> + struct commit *c = lookup_commit(r, oid);
>> +
>> + if (!c->object.parsed)
>> + return;
>> + free_commit_list(c->parents);
>> + c->parents = NULL;
>> + c->object.parsed = 0;
>> +}
This looks good. I took a quick inventory of 'struct commit' and
agree that this is all we need. There is no need to clear the date
or maybe_tree members. The commit_graph_data slab might have some
information if there were no grafts before (but are now), but the
existence of grafts should clear the commit-graph already and stop
that slab from being used.
>> int register_commit_graft(struct repository *r, struct commit_graft *graft,
>> int ignore_dups)
>> {
>> @@ -145,6 +156,7 @@ int register_commit_graft(struct repository *r, struct commit_graft *graft,
>> (r->parsed_objects->grafts_nr - pos - 1) *
>> sizeof(*r->parsed_objects->grafts));
>> r->parsed_objects->grafts[pos] = graft;
>> + unparse_commit(r, &graft->oid);
>> return 0;
>> }
>>
>> @@ -253,8 +265,10 @@ void reset_commit_grafts(struct repository *r)
>> {
>> int i;
>>
>> - for (i = 0; i < r->parsed_objects->grafts_nr; i++)
>> + for (i = 0; i < r->parsed_objects->grafts_nr; i++) {
>> + unparse_commit(r, &r->parsed_objects->grafts[i]->oid);
>> free(r->parsed_objects->grafts[i]);
>> + }
>> r->parsed_objects->grafts_nr = 0;
>> r->parsed_objects->commit_graft_prepared = 0;
>> }
>
> Are we going to have the same issue with tags, c.f. parse_tag() and
> there being no unparse_tag()?
>
> (I don't know offhand, just asking)
Grafts are only on commits. You cannot replace what a tag is pointing
at with a graft. Replace-objects is a different thing and changes it at
the OID level (and I don't think this can happen during the process
without concurrent external changes to the filesystem).
Thanks,
-Stolee
