Re: [PATCH v2] commit,shallow: unparse commits if grafts changed
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jun 3, 2022, 13:29 UTC
- Message-ID
- <394c054e-e1d2-41a5-a655-2ad3cb7219e0@github.com>
- In-Reply-To
- <220603.86k09yxf4z.gmgdl@evledraar.gmail.com>
On 6/3/2022 5:30 AM, Ævar Arnfjörð Bjarmason wrote:
Show 21 quoted lines
>
> 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.
Show 28 quoted lines
>> 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