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. > ... > Yeah, I guess the fundamental thing here is that anybody who isn't > passing "--promisor" is not going to be affected, so that at least > limits the opportunity for surprise. > > The quarantine discussion above is an example of how there could be > unexpected consequences. I _think_ it's OK based on what I wrote, but > hopefully that explains my general feeling of surprise. I dunno. It > still may be the least bad thing. 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. As to future potential mis-interaction between quarantined fetch and the effect of this "repack local objects that can be reached by objects in a promisor pack" feature, I do not offhand think of a good way to future-proof it with tests. Thanks for the discussion, both of you.
