git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Brandon Casey <casey@nrlssc.navy.mil>,
	git@vger.kernel.org, Brandon Casey <drafnel@gmail.com>
Subject: Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Date: Mon, 08 Jun 2009 09:41:49 -0700	[thread overview]
Message-ID: <7v4ouq1xv6.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20090608114351.GA13775@coredump.intra.peff.net> (Jeff King's message of "Mon\, 8 Jun 2009 07\:43\:51 -0400")

Jeff King <peff@peff.net> writes:

> On Fri, Jun 05, 2009 at 06:36:15PM -0500, Brandon Casey wrote:
> ...
>> 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 "$@"

  parent reply	other threads:[~2009-06-08 16:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <7vk53vlxhz.fsf@alter.siamese.dyndns.org>
2009-06-05 23:36 ` [PATCH 0/8] enhancing builds on Solaris Brandon Casey
2009-06-05 23:36   ` [PATCH 1/8] Makefile: use /usr/ucb/install on SunOS platforms rather than ginstall Brandon Casey
2009-06-05 23:36     ` [PATCH 2/8] Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile arguments Brandon Casey
2009-06-05 23:36       ` [PATCH 3/8] diff-delta.c: "diff.h" is not a required include Brandon Casey
2009-06-05 23:36         ` [PATCH 4/8] On Solaris choose the OLD_ICONV iconv() declaration based on the UNIX spec Brandon Casey
2009-06-05 23:36           ` [PATCH 5/8] git-compat-util.h: tweak the way _XOPEN_SOURCE is set on Solaris Brandon Casey
2009-06-05 23:36             ` [PATCH 6/8] Makefile: define __sun__ on SunOS Brandon Casey
2009-06-05 23:36               ` [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH Brandon Casey
2009-06-05 23:36                 ` [PATCH 8/8] Makefile: add section for SunOS 5.7 Brandon Casey
2009-06-08 11:43                 ` [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH Jeff King
2009-06-08 13:39                   ` Brandon Casey
2009-06-08 13:50                     ` Jeff King
2009-06-08 15:59                       ` Brandon Casey
2009-06-08 16:41                   ` Junio C Hamano [this message]
2009-06-08 22:11                     ` Jeff King
2009-06-08 23:39                       ` Brandon Casey
2009-06-09 16:31                         ` Brandon Casey
2009-06-06  0:47         ` [PATCH v2 3/8] diff-delta.c: "delta.h" is not a required include Brandon Casey
2009-06-06  1:21           ` Nicolas Pitre
2009-06-06  2:49             ` Brandon Casey
2009-06-06  3:10               ` Nicolas Pitre
2009-06-06  3:56                 ` Brandon Casey
2009-06-08 23:53                   ` [PATCH] git-compat-util.h: avoid using c99 flex array feature with Sun compiler 5.8 Brandon Casey
2009-06-06  7:29       ` [PATCH 2/8] Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile arguments Jakub Narebski
2009-06-07  1:02         ` Brandon Casey
2009-06-07  5:40         ` [PATCH] configure: test whether -lresolv is needed Ralf Wildenhues
2009-06-05 23:46   ` [PATCH 0/8] enhancing builds on Solaris Brandon Casey
2009-06-06  0:13   ` Junio C Hamano
2009-06-06  0:41     ` Brandon Casey
2009-06-08 11:50   ` Jeff King

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=7v4ouq1xv6.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=casey@nrlssc.navy.mil \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).