Hi Junio,
On Mon, 1 Dec 2014, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> >
> >> +static const char *update_worktree(unsigned char *sha1)
> >> +{
> >> +...
> >> + const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
> >
> > I overlooked this one, but is there a reason why this has to look at
> > an internal implementatino detail which is git_work_tree_cfg,
> > instead of asking get_git_work_tree()?
> >
> > I am wondering if that reason is a valid rationale to fix whatever
> > makes get_git_work_tree() unsuitable for this purpose.
> >
> > Cc'ing Duy who has been working on the part of the system to
> > determine and set-up work-tree and git-dir.
>
> Answering myself...
>
> This is because you know receive-pack runs inside the $GIT_DIR,
> whether it is a bare or non-bare repository, so either core.worktree
> points at a directory that is otherwise unrelated to the $GIT_DIR
> (but is the correct $GIT_WORK_TREE), or the top of the working tree
> must be ".." for a non-bare repository. I haven't checked the code,
> but we probably are not even doing the repository/working-tree
> discovery to set up the data necessary for get_git_work_tree() to
> give us a correct answer.
Excellent analysis. Indeed, we do not enter the
setup_git_directory_gently() path that would interpret core.worktree and
call set_git_work_tree() explicitly. Instead, we are using the
enter_repo() code path in cmd_receive_pack() because only minimal setup is
required for the default operation of git receive-pack.
> Doing an optional set-up to make get_git_work_tree() work would
> involve more damage to the codebase than necessary, I would suspect,
> so let's keep this part of the code as-is.
Indeed, that is the exact reason why I did not want to go down that rabbit
hole. There are so many things that are skipped when using enter_repo()
instead of setup_git_directory() that the performance impact of switching
alone would probably pose a major regression for big hosters like GitHub
or Atlassian.
I have to admit also that I never used the work tree feature myself,
therefore the support for it is pretty much untested (I *think* that I
briefly tested it years ago, when I came up with the original
updateInstead patch, but that is *long* ago). We could of course simply go
the safe route and error out if we detect that git_work_tree_cfg is set,
leaving the work to support it and to add a proper unit test to somebody
who actually needs this. Hmm?
Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html