| Message ID | 20240228091011.3652532-1-christian.couder@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a4324babe679352a801310f8e30f3cbcd9c1f16b |
| Headers | show |
| Series | revision: fix --missing=[print|allow*] for annotated tags | expand |
Christian Couder <christian.couder@gmail.com> writes: > diff --git a/revision.c b/revision.c > index 0c7266b1eb..8f0d638af1 100644 > --- a/revision.c > +++ b/revision.c > @@ -419,15 +419,21 @@ static struct commit *handle_commit(struct rev_info *revs, > */ > while (object->type == OBJ_TAG) { > struct tag *tag = (struct tag *) object; > + struct object_id *oid; > if (revs->tag_objects && !(flags & UNINTERESTING)) > add_pending_object(revs, object, tag->tag); > - object = parse_object(revs->repo, get_tagged_oid(tag)); > + oid = get_tagged_oid(tag); > + object = parse_object(revs->repo, oid); This is locally a no-op but we need it because we will use oid later, OK. > if (!object) { > if (revs->ignore_missing_links || (flags & UNINTERESTING)) > return NULL; > if (revs->exclude_promisor_objects && > is_promisor_object(&tag->tagged->oid)) > return NULL; > + if (revs->do_not_die_on_missing_objects && oid) { > + oidset_insert(&revs->missing_commits, oid); > + return NULL; > + } And we recover from the "oh, that is not an object" by doing the usual "add to missing-objects list". OK. At this point we do not know the type of the tagged object (the tag itself may hint what the tagged object is, though). We might want to rename .missing_commits to .missing_objects later after the dust settles. revision.c:get_reference() already adds anything that is pointed at by a ref to this oidset already, so it is not a new problem with this patch, though. > diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh > index 78387eebb3..127180e1c9 100755 > --- a/t/t6022-rev-list-missing.sh > +++ b/t/t6022-rev-list-missing.sh > @@ -10,7 +10,10 @@ TEST_PASSES_SANITIZE_LEAK=true > test_expect_success 'create repository and alternate directory' ' > test_commit 1 && > test_commit 2 && > - test_commit 3 > + test_commit 3 && > + git tag -m "tag message" annot_tag HEAD~1 && > + git tag regul_tag HEAD~1 && > + git branch a_branch HEAD~1 > ' > > # We manually corrupt the repository, which means that the commit-graph may > @@ -78,7 +81,7 @@ do > done > done > > -for missing_tip in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t" > +for missing_tip in "annot_tag" "regul_tag" "a_branch" "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t" > do > # We want to check that things work when both > # - all the tips passed are missing (case existing_tip = ""), and > @@ -88,9 +91,6 @@ do > for action in "allow-any" "print" > do > test_expect_success "--missing=$action with tip '$missing_tip' missing and tip '$existing_tip'" ' > - oid="$(git rev-parse $missing_tip)" && > - path=".git/objects/$(test_oid_to_path $oid)" && > - > # Before the object is made missing, we use rev-list to > # get the expected oids. > if test "$existing_tip" = "HEAD" > @@ -109,11 +109,23 @@ do > echo $(git rev-parse HEAD:2.t) >>expect.raw > fi && > > + missing_oid="$(git rev-parse $missing_tip)" && > + > + if test "$missing_tip" = "annot_tag" > + then > + oid="$(git rev-parse $missing_tip^{commit})" && > + echo "$missing_oid" >>expect.raw > + else > + oid="$missing_oid" > + fi && > + > + path=".git/objects/$(test_oid_to_path $oid)" && > + > mv "$path" "$path.hidden" && > test_when_finished "mv $path.hidden $path" && Hmph, this might be OK for now, but recently I saw Dscho used a nice trick to prepare a packfile that excludes certain objects in a separate directory and use that directory as GIT_OBJECT_DIRECTORY to simulate a situation where some objects are missing without touching this level of implementation details. We may want to clean things up. Perhaps somebody will write a shell helper function that creates such an object directory that contains all objects in the current repository, except for ones that are specified. And then we add it to t/test-lib-functions.sh, so that it can be used to update various tests (#leftoverbits). > git rev-list --missing=$action --objects --no-object-names \ > - $oid $existing_tip >actual.raw && > + $missing_oid $existing_tip >actual.raw && > > # When the action is to print, we should also add the missing > # oid to the expect list.
diff --git a/revision.c b/revision.c index 0c7266b1eb..8f0d638af1 100644 --- a/revision.c +++ b/revision.c @@ -419,15 +419,21 @@ static struct commit *handle_commit(struct rev_info *revs, */ while (object->type == OBJ_TAG) { struct tag *tag = (struct tag *) object; + struct object_id *oid; if (revs->tag_objects && !(flags & UNINTERESTING)) add_pending_object(revs, object, tag->tag); - object = parse_object(revs->repo, get_tagged_oid(tag)); + oid = get_tagged_oid(tag); + object = parse_object(revs->repo, oid); if (!object) { if (revs->ignore_missing_links || (flags & UNINTERESTING)) return NULL; if (revs->exclude_promisor_objects && is_promisor_object(&tag->tagged->oid)) return NULL; + if (revs->do_not_die_on_missing_objects && oid) { + oidset_insert(&revs->missing_commits, oid); + return NULL; + } die("bad object %s", oid_to_hex(&tag->tagged->oid)); } object->flags |= flags; diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh index 78387eebb3..127180e1c9 100755 --- a/t/t6022-rev-list-missing.sh +++ b/t/t6022-rev-list-missing.sh @@ -10,7 +10,10 @@ TEST_PASSES_SANITIZE_LEAK=true test_expect_success 'create repository and alternate directory' ' test_commit 1 && test_commit 2 && - test_commit 3 + test_commit 3 && + git tag -m "tag message" annot_tag HEAD~1 && + git tag regul_tag HEAD~1 && + git branch a_branch HEAD~1 ' # We manually corrupt the repository, which means that the commit-graph may @@ -78,7 +81,7 @@ do done done -for missing_tip in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t" +for missing_tip in "annot_tag" "regul_tag" "a_branch" "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t" do # We want to check that things work when both # - all the tips passed are missing (case existing_tip = ""), and @@ -88,9 +91,6 @@ do for action in "allow-any" "print" do test_expect_success "--missing=$action with tip '$missing_tip' missing and tip '$existing_tip'" ' - oid="$(git rev-parse $missing_tip)" && - path=".git/objects/$(test_oid_to_path $oid)" && - # Before the object is made missing, we use rev-list to # get the expected oids. if test "$existing_tip" = "HEAD" @@ -109,11 +109,23 @@ do echo $(git rev-parse HEAD:2.t) >>expect.raw fi && + missing_oid="$(git rev-parse $missing_tip)" && + + if test "$missing_tip" = "annot_tag" + then + oid="$(git rev-parse $missing_tip^{commit})" && + echo "$missing_oid" >>expect.raw + else + oid="$missing_oid" + fi && + + path=".git/objects/$(test_oid_to_path $oid)" && + mv "$path" "$path.hidden" && test_when_finished "mv $path.hidden $path" && git rev-list --missing=$action --objects --no-object-names \ - $oid $existing_tip >actual.raw && + $missing_oid $existing_tip >actual.raw && # When the action is to print, we should also add the missing # oid to the expect list.
In 9830926c7d (rev-list: add commit object support in `--missing` option, 2023-10-27) we fixed the `--missing` option in `git rev-list` so that it works with missing commits, not just blobs/trees. Unfortunately, such a command was still failing with a "fatal: bad object <oid>" if it was passed a missing commit, blob or tree as an argument (before the rev walking even begins). This was fixed in a recent commit. That fix still doesn't work when an argument passed to the command is an annotated tag pointing to a missing commit though. In that case `git rev-list --missing=...` still errors out with a "fatal: bad object <oid>" error where <oid> is the object ID of the missing commit. Let's fix this issue, and also, while at it, let's add tests not just for annotated tags but also for regular tags and branches. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> --- This is a follow up to cc/rev-list-allow-missing-tips which is in 'next' and scheduled to be merged to 'master'. So it depends on that branch. revision.c | 8 +++++++- t/t6022-rev-list-missing.sh | 24 ++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-)