git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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


             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).