On Tue, Jan 19, 2021 at 05:53:56PM -0500, Jeff King wrote: > Thanks for posting this. I have a vague recollection that we considered > this either when we did the for-each-ref prefixes, or when we added > ls-refs prefixes, but I can't seem to find either. At any rate, at > GitHub we haven't generally found it to be a problem because our > horrifically-large repos tend to be aggregated alternates repos, not the > ones people serve upload-pack out of (though I did just time it, and > some of our largest repos should save a few hundred milliseconds per > advertisement, which is certainly not nothing). Great on all counts! > > This commit also fixes a bug in ls-refs.c that was not triggered > > before: we were using a strvec set to zero, which is not how you are > > supposed to initialize a strvec. We now call strvec_init after zeroing. > > Good catch. It didn't matter until now because nobody relied on having a > NULL entry when no prefix had been added (instead, they always iterated > over prefixes->nr). IMHO that is worth fixing as a separate commit. Yeah. Even after calling it out as such myself, I promptly forgot it when preparing the first patch I sent back to Jacob! I didn't pull it out into its own patch, and rather folded it in to my 2/2. It has a small call-out of its own, but if you'd prefer it by itself, I'm happy to resubmit it with that change included. > -Peff Thanks, Taylor
