Neeraj Singh <nksingh85@xxxxxxxxx> 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@xxxxxxxxx/ > > 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.
