Hi Junio,
On Sun, 27 Jan 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > When building Git with RUNTIME_PREFIX and starting a test helper from
> > t/helper/, it fails to detect the system prefix correctly.
> >
> > This is the reason that the warning
> >
> > RUNTIME_PREFIX requested, but prefix computation failed. [...]
> >
> > to be printed.
> >
> > In t0061, we did not expect that to happen, and it actually did not
> > happen in the normal case, because bin-wrappers/test-tool specifically
> > sets GIT_TEXTDOMAINDIR (and as a consequence, nothing in test-tool wants
> > to know about the runtime prefix).
> >
> > However, with --with-dashes, bin-wrappers/test-tool is no longer called,
> > but t/helper/test-tool is called directly.
> >
> > So let's just ignore the RUNTIME_PREFIX warning.
>
> Two questions that would come to the readers' minds are
>
> - Why "it fails to detect the system prefix correctly"? Is that a
> bug waiting to hurt end users?
I recall explaining that already in a different thread (one that actually
*was* about the RUNTIME_PREFIX feature, where you asked why we cannot test
for it in the test suite): to verify that we are in a valid Git
installation location, we verify that the `git` executable is either in
`<PREFIX>/bin` or in `<PREFIX>/libexec/git-core` (we need to check that,
as the prefix is the parent directory in one case, and the grand parent
directory in the other).
And for Git's test suite, we simply do not control the name of the
directory in which the Git executable lives (think: `git clone
<git.git-url> junios-git`).
And even if we could control that name: it simply is not a valid
installation location. For example, it is unlikely that the translation
live in `../share/locale/de/LC_MESSAGES/git.mo` relative to the Git
executable.
> - Why is it better not to bother fixing that failure? Is it
> because this happens only in the test helper and won't hurt end
> users?
>
> I do not mind this particular "sweeping it under the rug" if the
> rationale is "it only is the strange set-up of test-tool that causes
> it, and we shouldn't burden the code shared with the actual runtime
> to compute runtime prefix just to fix this bug". I do think it is
> not productive to bend backwards if it is only done in order to work
> around unusual setup in the test helper binary and I do agree that
> ignoring the warning is the right solution.
>
> I just do not like to see that our commits do not explain why we
> chose to ignore, instead of "fix", the issue.
But that was clarified already to your satisfaction in the RUNTIME_PREFIX
patches. Or do you want to reopen that case now?
Ciao,
Dscho
>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > t/t0061-run-command.sh | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
> > index 99a614bc7c..5a2d087bf0 100755
> > --- a/t/t0061-run-command.sh
> > +++ b/t/t0061-run-command.sh
> > @@ -166,7 +166,8 @@ test_trace () {
> > expect="$1"
> > shift
> > GIT_TRACE=1 test-tool run-command "$@" run-command true 2>&1 >/dev/null | \
> > - sed -e 's/.* run_command: //' -e '/trace: .*/d' >actual &&
> > + sed -e 's/.* run_command: //' -e '/trace: .*/d' \
> > + -e '/RUNTIME_PREFIX requested/d' >actual &&
> > echo "$expect true" >expect &&
> > test_cmp expect actual
> > }
>
