Junio C Hamano <gitster@xxxxxxxxx> wrote: > 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. > > ... ... > What I do not quite understand is how this can be a new issue. It isn't. It is indeed a pretty old issue, as far as git issues go. Its probably about as old as fast-import being accepted into your tree is. > 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? I think the problem is nobody has tested fast-import updating an existing ref while using NO_MMAP. Or if they did, they didn't report the problem as they didn't figure they needed fast-import that badly. Updating an existing ref is not a common operation, but the test suite does test for it. So it must be the NO_MMAP configuration is simply not being tested well enough. -- Shawn. - 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