On Thu, Jan 4, 2018 at 9:54 PM, Jeff Hostetler <g...@jeffhostetler.com> wrote: > > On 1/3/2018 11:33 AM, Christian Couder wrote: >> >> Objects managed by an external ODB should not be put into >> pack files. They should be transfered using other mechanism >> that can be specific to the external odb. >> >> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org> >> --- >> builtin/pack-objects.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c >> index 6c71552cdf..4ed66c7677 100644 >> --- a/builtin/pack-objects.c >> +++ b/builtin/pack-objects.c >> @@ -28,6 +28,7 @@ >> #include "argv-array.h" >> #include "mru.h" >> #include "packfile.h" >> +#include "external-odb.h" >> static const char *pack_usage[] = { >> N_("git pack-objects --stdout [<options>...] [< <ref-list> | < >> <object-list>]"), >> @@ -1026,6 +1027,9 @@ static int want_object_in_pack(const struct >> object_id *oid, >> return want; >> } >> + if (external_odb_has_object(oid->hash)) >> + return 0; > > I worry about the performance of this in light of my comments > earlier in the patch series about the expense of building the > "have" sets.
Building the have set is done only if the cap_have capability is used, (see my answer to your comments on patch 15/40). > Since we've already checked for a loose object and we are about > to walk thru the local packfiles, so if we don't find it in any > of them, then we don't have it locally. Only then do we need > to worry about external odbs. > > If we don't have it locally, does the caller of this function > have sufficient "promisor" infomation infer that the object > should exist on the promisor remote? Since you're going to > omit it from the packfile anyway, you don't really need to know > if the remote actually has it. Yeah, I think it works in the same way as how the promisor code works, but I haven't checked carefully, so I will take a look at that. Thanks, Christian.