On 27/09/2022 14:27, Phillip Wood wrote:
+ /**
+ * Determine the object id for the latest content commit for each
change.
+ * Fetch the commit at the head of each change ref. If it's a
normal commit,
+ * that's the commit we want. If it's a metacommit, locate its
content parent
+ * and use that.
+ */
+
+ for (i = 0; i < matching_refs.nr; i++) {
+ struct ref_array_item *item = matching_refs.items[i];
+ struct commit *commit = item->commit;
+
+ commit = lookup_commit_reference_gently(repo,
&item->objectname, 1);
We're assigning commit twice - why do we need to look it up if
filter_refs returns it?
I think we want to look it up to check that item->objectname is a
commit. item->commit is not filled unless we specify the verbose flag
and I'm not sure what the implications of setting that are. If we get an
objectname that does not name a commit we should call BUG() as suggested
below.
There are a number of places where we call
lookup_commit_reference_gently(..., 1) to silence the warning if the
objectname does not dereference to a commit. It is not clear to me that
we want to hide those errors. Indeed I think we should be doing
commit = lookup_commit_reference(repo, oid)
if (!commit)
BUG("commit missing ...")
unless there is a good reason that the lookup can fail.
