Junio C Hamano <gitster@xxxxxxxxx> writes:
> The following patch seems to fix the issue for me, but this is
> primarily meant for discussion, as I do not quite understand why
> the same issue does not manifest itself when NO_MMAP is not
> used.
> ...
BTW, the lookup of the object that dies is in update_branch().
The call to write_ref_sha1() at the last tries to verify b->sha1
is available and is a commit.
static int update_branch(struct branch *b)
{
static const char *msg = "fast-import";
struct ref_lock *lock;
unsigned char old_sha1[20];
if (read_ref(b->name, old_sha1))
hashclr(old_sha1);
lock = lock_any_ref_for_update(b->name, old_sha1, 0);
if (!lock)
return error("Unable to lock %s", b->name);
if (!force_update && !is_null_sha1(old_sha1)) {
struct commit *old_cmit, *new_cmit;
old_cmit = lookup_commit_reference_gently(old_sha1, 0);
new_cmit = lookup_commit_reference_gently(b->sha1, 0);
if (!old_cmit || !new_cmit) {
unlock_ref(lock);
return error("Branch %s is missing commits.", b->name);
}
if (!in_merge_bases(old_cmit, &new_cmit, 1)) {
unlock_ref(lock);
warning("Not updating %s"
" (new tip %s does not contain %s)",
b->name, sha1_to_hex(b->sha1), sha1_to_hex(old_sha1));
return -1;
}
}
if (write_ref_sha1(lock, b->sha1, msg) < 0)
return error("Unable to update %s", b->name);
return 0;
}
The write_ref_sha1() function previously did not even check if
the object pointed at by b->sha1 actually existed, but now it
parses the object and makes sure it is a commit, if the updated
ref is under ref/heads/ hierarchy.
What I do not quite understand is how this can be a new issue.
The codepath to allow updating an existing branch shown above
(i.e. "if it is not force and old is not NULL") uses the usual
lookup_commit_reference_gently() interface to access b->sha1,
and does not use gfi-aware gfi_unpack_entry() or anything
magical, which means it would have passed the same codepath down
to trigger the same issue. IOW, even before this tightening of
write_ref_sha1(), we already should have had the issue of not
being to able to grab the object b->sha1 refers to out of the
newly built packfile.
Is it just nobody seriously exercised the codepath yet, or is
there a difference between these two calls that is more subtle
than that?
Quite confused...
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
