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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2019, 20:11 UTC
Message-ID: <xmqqy30kmu8r.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190726155258.28561-8-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 9 quoted lines

> 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().