From: Dridi Boukelmoune <dridi.boukelmoune@gmail.com>
To: git@vger.kernel.org
Subject: [PATCH] git-sh: Avoid sourcing scripts with git --exec-path
Date: Fri, 29 Sep 2017 00:31:34 +0200 [thread overview]
Message-ID: <20170928223134.GA30744@varnish> (raw)
For end users making use of a custom exec path many commands will simply
fail. Adding git's exec path to the PATH also allows overriding git-sh-*
scripts, not just adding commands. One can then patch a script without
tainting their system installation of git for example.
Signed-off-by: Dridi Boukelmoune <dridi.boukelmoune@gmail.com>
---
Hi,
I've been bothered by this problem ever since I upgraded my system to
Fedora 26, and I grew tired of locally patching git-sh-setup after every
git-core update. So I decided to look at the instructions on how to send
patches to the Git project, and here is my first patch.
I hope you will find it appropriate, I didn't study the test suite to
have it enforce that files shouldn't be sourced from the "system"
installation. I couldn't reproduce it after a quick look, and I'm not
familiar enough to tinker with it yet, so I reached my trial&error quota
before I could figure things out.
Best Regards,
Dridi
Documentation/CodingGuidelines | 22 ++++++++++++++++++++++
contrib/convert-grafts-to-replace-refs.sh | 3 ++-
contrib/rerere-train.sh | 3 ++-
contrib/subtree/git-subtree.sh | 1 -
git-sh-setup.sh | 2 +-
5 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index c4cb5ff0d..fdc0d3318 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -151,6 +151,28 @@ For shell scripts specifically (not exhaustive):
interface translatable. See "Marking strings for translation" in
po/README.
+ - When sourcing a "git-sh-*" file using "git --exec-path" make sure
+ to only use its base name.
+
+ (incorrect)
+ . "$(git --exec-path)/git-sh-setup"
+
+ (correct)
+ . git-sh-setup
+
+ Otherwise for a user with a custom "GIT_EXEC_PATH=/foo" the shell
+ expects "/foo:/usr/libexec/git-core/git-sh-setup" or something
+ similar depending on the installation. The git command already
+ adds the git exec path to the regular path before executing a
+ command.
+
+ For scripts that aren't run via the git command, add the git exec
+ path to the path instead.
+
+ (correct)
+ PATH="$(git --exec-path):$PATH"
+ . git-sh-setup
+
- We do not write our "test" command with "-a" and "-o" and use "&&"
or "||" to concatenate multiple "test" commands instead, because
the use of "-a/-o" is often error-prone. E.g.
diff --git a/contrib/convert-grafts-to-replace-refs.sh b/contrib/convert-grafts-to-replace-refs.sh
index 0cbc917b8..f7d2fab03 100755
--- a/contrib/convert-grafts-to-replace-refs.sh
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -5,7 +5,8 @@
GRAFTS_FILE="${GIT_DIR:-.git}/info/grafts"
-. $(git --exec-path)/git-sh-setup
+PATH="$(git --exec-path):$PATH"
+. git-sh-setup
test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'"
diff --git a/contrib/rerere-train.sh b/contrib/rerere-train.sh
index 52ad9e41f..07bad67e6 100755
--- a/contrib/rerere-train.sh
+++ b/contrib/rerere-train.sh
@@ -7,7 +7,8 @@ USAGE="$me rev-list-args"
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
-. "$(git --exec-path)/git-sh-setup"
+PATH="$(git --exec-path):$PATH"
+. git-sh-setup
require_work_tree
cd_to_toplevel
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index dec085a23..1d61f75d0 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -32,7 +32,6 @@ squash merge subtree changes as a single commit
"
eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
-PATH=$PATH:$(git --exec-path)
. git-sh-setup
require_work_tree
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 378928518..12e1220f8 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -44,7 +44,7 @@ git_broken_path_fix () {
# @@BROKEN_PATH_FIX@@
# Source git-sh-i18n for gettext support.
-. "$(git --exec-path)/git-sh-i18n"
+. git-sh-i18n
die () {
die_with_status 1 "$@"
--
2.13.5
next reply other threads:[~2017-09-28 22:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 22:31 Dridi Boukelmoune [this message]
2017-09-29 3:48 ` [PATCH] git-sh: Avoid sourcing scripts with git --exec-path Jonathan Nieder
2017-09-29 3:58 ` Junio C Hamano
2017-09-29 4:21 ` Jonathan Nieder
2017-09-29 5:00 ` Junio C Hamano
2017-09-29 11:58 ` Dridi Boukelmoune
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170928223134.GA30744@varnish \
--to=dridi.boukelmoune@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).