Jeff King <peff@xxxxxxxx> writes: > On Sun, Oct 27, 2024 at 05:23:24PM -0400, karthik nayak wrote: > >> While thinking about this over the last few days and also getting some >> advice from Patrick, I realized that we don't need to be this disruptive >> by simply adding the 'repository' variable to the already existing >> 'packed_git' struct. This allows us to leverage this information more >> easily, since most of the functions already have access to the >> 'packed_git' struct. >> >> This, plus the series by Jeff 'jk/dumb-http-finalize' which also removes >> some existing functions. We reduce the impact to only 3 functions being >> modified. > > Yeah, I noticed while working on that topic that we were dropping some > uses of the_repository. And FWIW I had the same notion, that packed_git > should perhaps refer to the repository struct in which it resides. > > As Taylor noted this is a tiny bit weird with respect to alternates, > which could exist in another repository (but don't have to! It could be > a bare objects/ directory). But I think from the perspective of a > particular process, we only have one repository struct that covers all > of its alternates for the duration of this process. So it would be OK in > practice. You might be able to get away with just storing a hash_algo > pointer in packed_git, which would be less weird (and is enough for the > bits I looked at, but perhaps not in the more general case). > This was my thought as well regarding alternates. Also it should be noted that currently we're using the_repository anyways, so we will be in the same state as before. In a general case it seems more necessary to add the repo and not just the hash_algo. Mostly because there are parts which require access to the repository and also because some of my patches add config changes which also require access to the repository. > Looking at odb_pack_name(), it will still need to take a repository > struct, since we sometimes form it before having a packed_git. But for > most calls, I suspect you could have an alternate function that takes a > packed_git and uses both its "hash" member and the algorithm. > I have four functions which would still need to take in a repository: 1. for_each_packed_object 2. has_object_pack 3. has_object_kept_pack 4. obd_pack_name > Anyway, just my two cents having worked in the area recently. > > -Peff Thanks for your input!
Attachment:
signature.asc
Description: PGP signature
