Subject: Re: [PATCH v2 07/20] Use write_index_as_tree() in lieu of write_tree_from_memory()

From: Junio C Hamano <[email-masked]>
Date: Fri, 26 Jul 2019 13:11:16 -0700

Elijah Newren <[email-masked]> writes:

> diff --git a/cache-tree.c b/cache-tree.c
> index 706ffcf188..99144b1704 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -613,14 +613,19 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
>  	int entries, was_valid;
>  	struct lock_file lock_file = LOCK_INIT;
>  	int ret = 0;
> +	int access_disk = !(flags & WRITE_TREE_FROM_MEMORY);

Shouldn't we go one step futher and make the bulk of in-core index
processing into a new helper function, while making
write_index_as_tree() a thin-wrapper around it, i.e.

	write_index_as_tree() 
	{
		lock the index for update;
		read the on-disk index;
		call that new helper function to write a tree;
		update the on-disk index;
	}

and reuse the helper from
merge-recursive.c::write_tree_from_memory() while keeping the call
to the latter in merge_trees_internal()?  Wouldn't that approach
let you do this without adding an extra flag bit?

Also, there used to be a check to ensure that the in-core index fed
to write_tree_from_memory() is fully merged and otherwise dump the
unmerged entries with BUG().  Can we simply lose it?  I know you
return with "error building trees" from merge_trees_internal() but
it does not BUG().