Re: [PATCH v2] write-tree: integrate with sparse index

Victoria Dye [off-list ref] writes:

Shuqi Liang wrote:

quoted

Update 'git write-tree' to allow using the sparse-index in memory
without expanding to a full one.

The recursive algorithm for update_one() was already updated in 2de37c5
(cache-tree: integrate with sparse directory entries, 2021-03-03) to
handle sparse directory entries in the index. Hence we can just set the
requires-full-index to false for "write-tree".

The `p2000` tests demonstrate a ~96% execution time reduction for 'git
write-tree' using a sparse index:

Test                                           before  after
-----------------------------------------------------------------
2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%
2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%
2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%
2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%

Signed-off-by: Shuqi Liang <redacted>
---

* change the position of "settings.command_requires_full_index = 0"

Could you describe why you made this change? You don't need to re-roll, but
in the future please make sure to describe the reasoning for changes like
this in these version notes if the context can't be gathered from other
discussions in the thread. 

The reason, I think, is because previous iteration hit a BUG() when
the command "git write-tree -h" is run outside a repository.  That
form of the help request is handled in the parse_options() machinery
without any need to have a repository or a working tree.

But prepare_repo_settings() does need to be run inside a repository,
so calling it without first checking if we are even in a repository
is asking for trouble.

I guess an alternative fix could have been to see if we are indeed
in a repository, by doing something like

	if (the_repository->gitdir) {
		prepare_repo_settings(the_repository);
		the_repository->settings.command_requires_full_index = 0;
	}

like implementations of some subcommands do.  And being explicit
that way, instead of relying on an implicit safety given by ordering
of calls, would be more maintainable in the longer haul.