Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 5, 2016, 18:37 UTC
- Message-ID
- <xmqqy3zu43yk.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <alpine.DEB.2.20.1612051126320.117539@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Seriously, do you really think it is a good idea to have > git_config_get_value() *ignore* any value in .git/config?
When we do not know ".git/" directory we see is the repository we were told to work on, then we should ignore ".git/config" file. So allowing git_config_get_value() to _ignore_ ".git/config" before the program calls setup_git_directory() does have its uses.
But I agree that "difftool.useBuiltin" that flips between two implementations is a different use case than the above. Both implementations eventually want to work on ".git/" and would want to read from ".git/config".
> We need to fix this.
I have a feeling that "environment modifications that cannot be undone" we used as the rationale in 73c2779f42 ("builtin-am: implement skeletal builtin am", 2015-08-04) might be overly pessimistic and in order to implement undo_setup_git_directory(), all we need to do may just be matter of doing a chdir(2) back to prefix and unsetting GIT_PREFIX environment, but I haven't looked into details of the setup sequence recently.