Junio C Hamano [off-list ref] writes:
... I do not think you can safely add these two bits here until the
migration completes.
I accidentally removed a more useful bit I wrote after the above
sentence while editing.
The NEEDSWORK comment in 73c2779f42 ("builtin-am: implement skeletal
builtin am", 2015-08-04) mentions why it calls setup-git-directory
and setup-work-tree instead of letting run_builtin() do so; perhaps
you can do something similar here to fix this.
I doubt that setting core.usebuiltindifftool to false and running
the tool from a subdirectory and a pathspec work correctly with this
patch. If running difftool from a subdirectory with a pathspec is
not tested in t7800, perhaps we should.
It is nice that we can now lose PERL prerequisite from t7800 ;-)
Thanks.
Hi Junio,
On Wed, 23 Nov 2016, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
... I do not think you can safely add these two bits here until the
migration completes.
I accidentally removed a more useful bit I wrote after the above
sentence while editing.
The NEEDSWORK comment in 73c2779f42 ("builtin-am: implement skeletal
builtin am", 2015-08-04) mentions why it calls setup-git-directory
and setup-work-tree instead of letting run_builtin() do so; perhaps
you can do something similar here to fix this.
This is the Catch-22 I mentioned a couple times: if you insist on a config
setting, the config has to be read. For that to work,
setup_git_directory() has to be called.
So no matter what you do, if you want to have conditional code that
depends on the config, and that wants setup_git_directory() *not* to be
called before, you are simply out of luck.
Sadly, I now bought into your comment that using a file in exec-path as a
feature flag is a bad thing, and that we have to use a config setting. So
now I have to spend more time on fixing something that was not a problem
in my original patches.
However, this exchange has something else in it, apart from creating
unneeded work for me.
What you really accidentally did was to identify a fundamental problem
with the builtin difftool: when called from a subdirectory, the RUN_SETUP
flag would make it chdir() to the top-level directory, and the
subsequently spawned Git processes would get the wrong idea about relative
paths.
Thank you for that,
Dscho