Re: [PATCH v3 13/39] i18n: git-sh-setup.sh: mark strings for translation
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 1, 2016, 20:30 UTC
- Message-ID
- <xmqqd1o0y7m0.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqqvb1sydmu.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> That is correct, but doesn't that hint that the current users of > git-sh-i18n no longer have to (and they probably do not want to) > dot-source it, as they are already dot-sourcing git-sh-setup? > > Otherwise they will waste cycles reading the same thing twice > (assuming that git-sh-i18n is safely read twice, that is).
This part still stands.
If we assume that it is a good idea to i18n shell functions that are supplied by git-sh-setup, it might be a better direction to retire git-sh-i18n and include its text directly in git-sh-setup, so that any scripted Porcelain has to open just one file to dot-source it, instead of two.
Show 16 quoted lines
>> Source "$(git --exec-path)"/git-sh-i18n instead of simply git-sh-i18n, > > That is somewhat strange. "git-rebase.sh" dot-sources both > git-sh-setup and git-sh-i18n with just ". $name". What is so > special about sh-setup dot-sourcing sh-i18n your patch adds? > >> because latter case would fail test t2300-cd-to-toplevel.sh. > > What is the real cause of this? > > ". git-sh-setup" is supposed to work because we prepend the > $GIT_EXEC_PATH in "git" itself and git-sh-setup is installed there. > ". git-sh-i18n" in "git-rebase.sh" works the same way. If it is > because is t2300 doing something strange, then we should fix that, > not adding $(git --exec-path) on the dot-source line to paper things > over.
This because what t2300 does is strange; I wouldn't call it "wrong" per se, but it is borderline wrong.
Would it allow you to lose the $(git --exec-path) prefix in the new dot-source to have this patch before applying your patch?
-- >8 -- Subject: t2300: run git-sh-setup in an environment that better mimics the real life
When we run scripted Porcelains, "git" potty has set up the $PATH by prepending $GIT_EXEC_PATH, the path given by "git --exec-path=$there $cmd", etc. already. Because of this, scripted Porcelains can dot-source shell script library like git-sh-setup with simple dot without specifying any path.
t2300 however dot-sources git-sh-setup without adjusting $PATH like the real "git" potty does. This has not been a problem so far, but once git-sh-setup wants to rely on the $PATH adjustment, just like any scripted Porcelains already do, it would become one. It cannot for example dot-source another shell library without specifying the full path to it by prefixing $(git --exec-path).
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t2300-cd-to-toplevel.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh index 9965bc5..cccd7d9 100755 --- a/t/t2300-cd-to-toplevel.sh +++ b/t/t2300-cd-to-toplevel.sh @@ -8,7 +8,8 @@ test_cd_to_toplevel () { test_expect_success $3 "$2" ' ( cd '"'$1'"' && - . "$(git --exec-path)"/git-sh-setup && + PATH="$(git --exec-path):$PATH" && + . git-sh-setup && cd_to_toplevel && [ "$(pwd -P)" = "$TOPLEVEL" ] )