Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases

[Junio C Hamano <gitster@pobox.com>]

Neeraj Singh <nksingh85@gmail.com> writes:

>> > +	if (tmp_objdir)
>> > +		tmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);
>> >  	free(path);
>> >  }
>> 
>> This is called during set_git_dir(), which happens fairly early in
>> the set-up sequence.  I wonder if there is a real use case that
>> creates a tmp-objdir that early in the process to require this
>> unapply-reapply sequence.
>> 
>
> The lack of this code was causing a failure, I believe in
> t2107-update-index-basic.sh: "--refresh triggers late setup_work_tree".
>
> This problem came up after applying: https://lore.kernel.org/git/4a40fd4a29a468b9ce320bc7b22f19e5a526fad6.1637020263.git.gitgitgadget@gmail.com/
>
> I thought it would be best to fix this in the tmp-objdir code so that
> callers could plug/unplug bulk checkin without any subtle surprises.

OK, I think that is fine.

As a slightly-related tangent that is outside the topic, I think we
should revisit "update-index", which is one of the oldest plumbing
commands with its own quirks.  I do not offhand see why it needs to
sprinkle this many setup_work_tree() calls everywhere.  Having an
index to work on means we must have a working tree to update and/or
refresh from.  We should be able to get away with the NEED_WORK_TREE
bit in the git.c::commands[] table for this command.  If this were a
more recent command, I may suspect that there were valid reasons
like "in this particular mode, update-index must work inside a bare
repository" to force us to take this unusual program structure, but
because this is probably a lot older than NEED_WORK_TREE bit, I
would not be surprised if the answer were "nobody noticed the
ugliness so far".

> Given that this patch series introduces functions with no users, are you
> going to hold off on putting this into 'next' until another next-worthy
> patch series is ready?

Even without any existing callers, as long as we see Reviewed-by: by
Elijah, who we know will have to build on top of this series, I
think this can and should go to 'next'.

Thanks.