On Thu, Jan 4, 2018 at 9:54 PM, Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> 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 <chriscool@xxxxxxxxxxxxx>
>> ---
>> 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.
