Thread (30 messages) 30 messages, 5 authors, 2024-11-20

Re: [PATCH v2 3/4] t5300: move --window clamp test next to unclamped

From: Jonathan Tan <hidden>
Date: 2024-11-15 19:55:06

Jeff King [off-list ref] writes:
Where it gets weirder to me is with quarantine directories (and maybe
this is what you meant above). On receiving a push, we "index --stdin"
into a temporary quarantine directory. If that kicks off a pack-objects
run, where does that pack-objects put its new pack? Within the
quarantined index-pack we set GIT_OBJECT_DIRECTORY to the quarantine and
add the original repo as an alternate. So I _think_ both the pushed-up
pack and the repacked promisor pack would go into the quarantine dir,
and then we'd migrate both (or neither) when we commit to the push.

Which is OK, but I don't know that I thought that far ahead when writing
the quarantine stuff long ago.

It's probably somewhat academic right now, as I'm not sure if you can
even push reliably into a promisor repo (and it doesn't look like
receive-pack knows about passing --promisor anyway).
Thanks for this description. Such a push would be an "I am pushing a
pack with missing objects to you, and you can later get those missing
objects from me" situation. Not completely implausible, but doesn't seem
high-priority to me.
We don't quarantine
on fetch right now, though we have discussed it in the past (and I think
we should consider doing it).

So this may become more real in the future. I wonder if there is a way
to add a test to future-proof against changes to how the quarantine
system works. The theoretical problem case is if we did quarantine
fetches, but accidentally wrote the new promisor pack into the main
repo instead of the quarantine, and then a fetch rejected the incoming
pack (because of a hook, failed connectivity check, etc). Then we'd end
up with the new promisor pack when we shouldn't, which I guess could
move objects from that incoming pack that we rejected into the main
repo, despite the quarantine?

I can't think of a way to test that now, without the quarantine-on-fetch
feature existing.
Quarantine on fetch does seem like a good idea. I also can't think of
a way to test that now. Although, for the fetch case, my patch set is
not the first time that an extra packfile (that is, a packfile not in
the "packfile" section of the fetch response) could be written during
a fetch: packfile-uris and bundle-uris already exist. So I would hope
that the implementor of the fetch quarantine feature would be aware of
at least one of these extra features, and design the test to check that
absolutely no packfiles are written if the fetch is rejected. (So I
don't think the future needs to be "proofed" so much.)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help