On Thu, Jul 29, 2021 at 03:44:34PM -0400, Taylor Blau wrote: > > Side note: IMHO the "check all packs to see if there are any other > > bitmaps to warn about" behavior is kind of pointless, and we should > > consider just returning as soon as we have one. This is already > > somewhat the case after your midx-bitmap patches, as we will not even > > bother to look for a pack bitmap after finding a midx bitmap. That is > > a good thing, because it means you can keep pack bitmaps around for > > flexibility. But let's leave any changes to the pack-only behavior out > > of this series for simplicity. > > I agree. I'd be in favor of something like > [...patch...] Yep, that looks good. I'd be quite happy if you sent that once the dust is settled. > > But we do allow an "--object-dir" option to "multi-pack-index write", > > and I don't see any other code explicitly requiring that it be part of > > the current repository. What I'm wondering is whether this would be > > breaking: > > > > cd $REPO/.. > > git multi-pack-index --object-dir $REPO/.git/objects write > > > > or: > > > > cd /some/other/repo > > git multi-pack-index --object-dir $REPO/.git/objects write > > > > The latter does seem to work, but the former segfaults (usually -- if > > there's already a midx it is OK). > > The former should work, but doesn't, because (as you pointed out to me > in our regular weekly discussion off-list) that the "multi-pack-index" > entry in git.c's commands array has the RUN_SETUP_GENTLY option, and > probably should have RUN_SETUP so that we complain with die() instead of > BUG. > > And the latter will continue to work, but only if in your scenario that > $REPO is an alternate of /some/other/repo. > > I wrote a little bit more in [1] about this behavior, but the upshot is > that we used to technically support passing *any* directory to > `--object-dir`, including directories that didn't belong to an > alternated repository. > > And that will cease to work after the patch that [1] is in response to > is applied. But for the reasons that I explain there, I think that is a > sufficient outcome, because the behavior is kind of bizarre to begin > with. Yeah, I think I am comfortable with the change at this point. The only case that will be broken is one that is quite ridiculous, and I am surprised worked in the first place. Thanks for talking it through. -Peff
