Re: [PATCH] unpack-trees: allow missing CE_SKIP_WORKTREE objs

From: Ben Peart <peartben@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] unpack-trees: allow missing CE_SKIP_WORKTREE objs
Date: Tue, 9 Oct 2018 10:49:24 -0400	[thread overview]
Message-ID: <56cc6e57-d1e3-1182-4290-c8b6fa796534@gmail.com> (raw)
In-Reply-To: <xmqqpnwjpivb.fsf@gitster-ct.c.googlers.com>

On 10/9/2018 5:30 AM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
> 
>> @@ -1635,6 +1635,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>>   				o->result.cache_tree = cache_tree();
>>   			if (!cache_tree_fully_valid(o->result.cache_tree))
>>   				cache_tree_update(&o->result,
>> +						  WRITE_TREE_SKIP_WORKTREE_MISSING_OK |
>>   						  WRITE_TREE_SILENT |
>>   						  WRITE_TREE_REPAIR);
>>   		}
> 
> Hmmmm.  Should this be passing the bit unconditionally?  Shouldn't
> it be set only when we are doing lazy clone?  A non-lazy clone that
> merely uses sparse checkout has nowhere else to turn to if it loses
> a blob object that currently is not necessary to complete a checkout,
> unlike a repository with promisor.
> 

I agree.  I believe this logic should only be triggered when running in 
a partial clone repo. Otherwise, we're potentially changing the behavior 
of 'normal' repos unnecessarily.