Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH

Jeff King [off-list ref] writes:

On Fri, Jun 05, 2009 at 06:36:15PM -0500, Brandon Casey wrote:
...

quoted

So provide a mechanism to prepend elements to the users PATH at runtime so
the modern binaries will be found.

So this bit me already, and it's only been in next for a day. :) I
_already_ have /usr/xpg4/bin in my PATH before /usr/bin, but with this
patch, I get it stuck at the _beginning_ of my PATH automagically. Which
overrides, against my wishes, the "even more sane than /usr/xpg4/bin"
part of my PATH that comes at the beginning.

Specifically, I have "~peff/local/bin" at the beginning of my PATH which
contains a 'vi' that points to vim. Running "git rebase -i" now puts
/usr/xpg4/bin at the beginning of the PATH (before ~peff/local/bin),

In git-sh-setup, we do "unset CDPATH" ourselves to help and protect
clueless people, even though "people should have a sane environment".
Even though I suspect that anybody who is using Solaris for anything real
would not be using /usr/bin tools themselves (i.e. it should not be
necessary for us fixing their PATH), there may be people who do not know.
I think helping them with path munging falls into the same category, but
at the same time, the remedy looks worse than the disease.

We could further uglify the patch like this.

 Makefile        |    5 +++--
 git-sh-setup.sh |   28 +++++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 3890a0e..c678cc0 100644
--- a/Makefile
+++ b/Makefile

@@ -881,7 +881,8 @@ endif
 -include config.mak
 
 ifdef SANE_TOOL_PATH
-BROKEN_PATH_FIX = s|^. @@PATH@@|PATH=$(SANE_TOOL_PATH)|
+SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
 PATH := $(SANE_TOOL_PATH):${PATH}
 else
 BROKEN_PATH_FIX = d

@@ -1288,7 +1289,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	    -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-	    -e '/^# @@PATH@@/$(BROKEN_PATH_FIX)' \
+	    -e $(BROKEN_PATH_FIX) \
 	    $@.sh >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7802581..80acb7d 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -11,7 +11,33 @@
 # exporting it.
 unset CDPATH
 
-# @@PATH@@:$PATH
+git_broken_path_fix () {
+	case ":$PATH:" in
+	*:$1:*) : ok ;;
+	*)
+		PATH=$(
+			SANE_TOOL_PATH="$1"
+			IFS=: path= sep=
+			set x $PATH
+			shift
+			for elem
+			do
+				case "$SANE_TOOL_PATH:$elem" in
+				(?*:/bin | ?*:/usr/bin)
+					path="$path$sep$SANE_TOOL_PATH"
+					sep=:
+					SANE_TOOL_PATH=
+				esac
+				path="$path$sep$elem"
+				sep=:
+			done
+			echo "$path"
+		)
+		;;
+	esac
+}
+
+# @@BROKEN_PATH_FIX@@
 
 die() {
 	echo >&2 "$@"

On Mon, Jun 08, 2009 at 09:41:49AM -0700, Junio C Hamano wrote:

We could further uglify the patch like this.
[...]
+git_broken_path_fix () {
+	case ":$PATH:" in
+	*:$1:*) : ok ;;
+	*)
+		PATH=$(
+			SANE_TOOL_PATH="$1"
+			IFS=: path= sep=
+			set x $PATH
+			shift
+			for elem
+			do
+				case "$SANE_TOOL_PATH:$elem" in
+				(?*:/bin | ?*:/usr/bin)
+					path="$path$sep$SANE_TOOL_PATH"
+					sep=:
+					SANE_TOOL_PATH=
+				esac
+				path="$path$sep$elem"
+				sep=:
+			done
+			echo "$path"
+		)
+		;;
+	esac
+}

Wow. That _is_ ugly, but it actually addresses exactly both my concern
and Brandon's. I kind of like it.

-Peff

I never received the referenced email.  I'll try to extract from gmane
and test.

-brandon


Jeff King wrote:

On Mon, Jun 08, 2009 at 09:41:49AM -0700, Junio C Hamano wrote:

quoted

We could further uglify the patch like this.
[...]
+git_broken_path_fix () {
+	case ":$PATH:" in
+	*:$1:*) : ok ;;
+	*)
+		PATH=$(
+			SANE_TOOL_PATH="$1"
+			IFS=: path= sep=
+			set x $PATH
+			shift
+			for elem
+			do
+				case "$SANE_TOOL_PATH:$elem" in
+				(?*:/bin | ?*:/usr/bin)
+					path="$path$sep$SANE_TOOL_PATH"
+					sep=:
+					SANE_TOOL_PATH=
+				esac
+				path="$path$sep$elem"
+				sep=:
+			done
+			echo "$path"
+		)
+		;;
+	esac
+}

Wow. That _is_ ugly, but it actually addresses exactly both my concern
and Brandon's. I kind of like it.

-Peff

Brandon Casey wrote:

I never received the referenced email.  I'll try to extract from gmane
and test.

This patch works for me.

-brandon

Jeff King wrote:

quoted

On Mon, Jun 08, 2009 at 09:41:49AM -0700, Junio C Hamano wrote:

quoted

We could further uglify the patch like this.
[...]
+git_broken_path_fix () {
+	case ":$PATH:" in
+	*:$1:*) : ok ;;
+	*)
+		PATH=$(
+			SANE_TOOL_PATH="$1"
+			IFS=: path= sep=
+			set x $PATH
+			shift
+			for elem
+			do
+				case "$SANE_TOOL_PATH:$elem" in
+				(?*:/bin | ?*:/usr/bin)
+					path="$path$sep$SANE_TOOL_PATH"
+					sep=:
+					SANE_TOOL_PATH=
+				esac
+				path="$path$sep$elem"
+				sep=:
+			done
+			echo "$path"
+		)
+		;;
+	esac
+}

Wow. That _is_ ugly, but it actually addresses exactly both my concern
and Brandon's. I kind of like it.

-Peff

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html