Re: [PATCH] git-sh-setup: Restore sourcability from outside scripts
From: Junio C Hamano <hidden>
Date: 2016-10-30 22:25:26
Anders Kaseorg [off-list ref] writes:
v2.10.0-rc0~45^2~2 “i18n: git-sh-setup.sh: mark strings for translation” broke outside scripts such as guilt that source git-sh-setup as described in the documentation: $ . "$(git --exec-path)/git-sh-setup" sh: 6: .: git-sh-i18n: not found This also affects contrib/convert-grafts-to-replace-refs.sh and contrib/rerere-train.sh in tree. Fix this by using git --exec-path to find git-sh-i18n. While we’re here, move the sourcing of git-sh-i18n below the shell portability fixes. Signed-off-by: Anders Kaseorg <redacted> ---
Looks good. Our in-tree scripts rely on the fact that $PATH is adjusted to have $GIT_EXEC_PATH early (either by getting invoked indirectly by "git" potty, or the requirement to do so for people and scripts that still run our in-tree scripts with dashed e.g. "git-rebase" form) by the time they run. But when sh-setup dot-sources git-sh-i18n for its own use, it should be explicit to name which one of the many copies that may appear in directories on user's $PATH (one among which is the one in $GIT_EXEC_PATH) it wants to use. And this patch does the right thing by not relying on the $PATH, but instead naming the exact path using $(git --exec-path)/ prefix, to the included file. In other words, I think this patch is a pure bugfix, even if there is no third-party script that includes it. We may want to have the above as the rationale to apply this patch in the proposed log message, though.
Is this a supported use of git-sh-setup? Although the documentation is clear that the end user should not invoke it directly, it seems to imply that scripts may do this, and in practice it has worked until v2.10.0.
It is correct for the documentation to say that this is not a "command" end users would want to run; they cannot invoke it as a standalone command as it is written as a dot-sourced shell library. Even though it is intended solely for internal use, so far we have not removed things from there, which would have signalled people that third-party scripts can also dot-source it. We may want to reserve the right to break them in the future, but because this is a pure bugfix, "can third-party rely on the interface not changing?" is not a question we need to answer in this thread---there is no reason to leave this broken. Thanks.
quoted hunk
git-sh-setup.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)diff --git a/git-sh-setup.sh b/git-sh-setup.sh index a8a4576..240c7eb 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh@@ -2,9 +2,6 @@ # to set up some variables pointing at the normal git directories and # a few helper shell functions. -# Source git-sh-i18n for gettext support. -. git-sh-i18n - # Having this variable in your environment would break scripts because # you would cause "cd" to be taken to unexpected places. If you # like CDPATH, define it for your interactive shell sessions without@@ -46,6 +43,9 @@ git_broken_path_fix () { # @@BROKEN_PATH_FIX@@ +# Source git-sh-i18n for gettext support. +. "$(git --exec-path)/git-sh-i18n" + die () { die_with_status 1 "$@" }