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. > However, if the commits are present in > the object store, then there could still be issues if those commits are > not reachable from the repository's references. The repository only has > guarantees that its object store is closed under reachability for the > objects that are reachable from references. Correct. > Thus, the code in verify_bundle() has previously had the additional > check that all prerequisite commits are reachable from repository > references. This is done via a revision walk from all references, > stopping only if all prerequisite commits are discovered or all commits > are walked. This uses a custom walk to verify_bundle(). Correct. > 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? > In the fetch case, Git guarantees that the new references > are closed under reachability by walking from the new references until > walking commits that are reachable from repository refs. This is done > through the well-used check_connected() method. Correct and is a good point to make. > 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.