On 1/24/2023 12:33 PM, Junio C Hamano wrote: > Derrick Stolee <derrickstolee@xxxxxxxxxx> writes: > >> When Git verifies a bundle to see if it is safe for unbundling, it first >> looks to see if the prerequisite commits are in the object store. This >> is usually a sufficient filter, and those missing commits are indicated >> clearly in the error messages. > > I am not sure if our early check is because "does the prerequisite > even exist?" is sufficient. It is a short-cut that is cheap and can > be done without preparing the commit traversal. I suppose I should say "Usually, existence in the object store is correlated with having all reachable objects, but this is not guaranteed." I'll also mention that it is a short-cut that can fail faster than the reachability check. >> This check is more strict than what Git applies even to fetched >> pack-files. > > I do not see the need to say "even" here. In what other situation > do we make connectivity checks, and is there a need to be more > strict than others when checking fetched packfiles? I suppose that I was implying that fetches are the more common operation, and the scrutiny applied to an arbitrary pack-file from a remote is probably higher there. However, who knows where a bundle came from, so the scrutiny should be the same. >> To better align with the restrictions required by 'git fetch', >> reimplement this check in verify_bundle() to use check_connected(). This >> also simplifies the code significantly. > > Wonderful. I never liked the custom check done in unbundle code, > which I am reasonably sure came from scripted hack to unbundle I > wrote eons ago. Excellent. Thanks for your feedback. -Stolee
