Re: [PATCH v2] commit,shallow: unparse commits if grafts changed
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 3, 2022, 09:30 UTC
- Message-ID
- <220603.86k09yxf4z.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20220602231145.310564-1-jonathantanmy@google.com>
On Thu, Jun 02 2022, Jonathan Tan wrote:
Show 42 quoted lines
> 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;
> +}
> +
> 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)
I have some semi-related (test) changes locally where we do have blind spots in tag v.s. commit parsing semi-related to this, i.e. in the whole "unparsed" stage.
So I wonder what happens with a tag that's pointing to a shallow object that's parsed, but its underlying commit becomes un-parsed.
Or maybe that's impossible, I'm not too familiar with "shallow"...
Show 16 quoted lines
> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh > index 92948de7a0..ba0a4c5d15 100755 > --- a/t/t5537-fetch-shallow.sh > +++ b/t/t5537-fetch-shallow.sh > @@ -170,6 +170,15 @@ test_expect_success 'fetch --update-shallow into a repo with submodules' ' > git -C repo-with-sub fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/* > ' > > +test_expect_success 'fetch --update-shallow a commit that is also a shallow point into a repo with submodules' ' > + git init repo-with-unreachable-upstream-shallow && > + git -C repo-with-unreachable-upstream-shallow submodule add ../a-submodule a-submodule && > + git -C repo-with-unreachable-upstream-shallow commit -m "added submodule" && > + > + SHALLOW=$(cat shallow/.git/shallow) && > + git -C repo-with-unreachable-upstream-shallow fetch --update-shallow ../shallow/.git "$SHALLOW":refs/heads/a-shallow > +'
Nit: Can this be e.g. s/repo-with-unreachable-upstream-shallow/repo/. The overly long repo name makes this much harder to follow. Compare this one which would clean up after itself too:
test_when_finished "rm -rf repo" && git init repo && git -C repo submodule add ../a-submodule a-submodule && git -C repo commit -m "added submodule" &&
SHALLOW=$(cat shallow/.git/shallow) && git -C repo fetch --update-shallow ../shallow/.git "$SHALLOW":refs/heads/a-shallow
(I didn't check if that test really works, i.e. do we have a "repo" already, but you get the idea...