Thread (1 message) 1 message, 1 author, 2016-10-30

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 "$@"
 }
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help