All of lore.kernel.org
 help / color / mirror / 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

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.