On Thu, Nov 14, 2024 at 03:41:08PM +0900, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > >> As far as I know, index-pack, when run as part of fetch, indexes a pack > >> that's not in the repository's object store; it indexes a packfile in a > >> temp directory. (So I don't think this is a strange thing to do.) > > > > When fetching (or receiving a push), we use "index-pack --stdin" and do > > write the resulting pack into the repository (and the command will > > complain if there is no repository). > > ... > >> We definitely should prevent the segfault, but I think that's better > >> done by making --promisor only work if we run index-pack from within a > >> repo. I don't think we can restrict the repacking to run only if we're > >> indexing a pack within the repo, because in our fetch case, we're > >> indexing a new pack - not one within the repo. > > > > I think the "--stdin" thing above neatly solves this. > [...] > > Tying this extra processing to the use of "--stdin" is not exactly > intuitive, in that a "--stdin" user is not necessarily doing a fetch > (even though a fetch may always use "--stdin"), but I guess it is a > good enough approximation (and the best one easily available to us) > if we want to safeguard the use of this "--promisor" logic only to > fetch client. I think the "--stdin" thing is a bit more general than that. Even though we expect to use it with --promisor only under fetch, the real rule is more like: only do the extra --promisor repacking when indexing a pack that will be made available in the repository. With --stdin, we know the result will be available because index-pack itself will write the pack into the repository. And that is true whether it is fetch, push, or some other script driving it. When being fed a path to a pack on the command line, then "is it available in the repo" would involve some path comparisons to see if it's in the object database directory. Probably not that hard, but not entirely trivial (due to normalization, etc). But since we care only about fetch and --stdin, it seemed like an easy cheat to simply disallow the other case for now, erring on the conservative side. -Peff
