threads / patch / 43336

patchRe: [PATCH] Install git-sh-setup.sh into $(prefix)/share/git-core. Call with explicit path.

Subject: Re: [PATCH] Install git-sh-setup.sh into $(prefix)/share/git-core. Call with explicit path.

## tl;dr

9 messages between Dec 8, 2006 and Dec 10, 2006. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Han-Wen Nienhuys· Dec 8, 2006, 17:13 UTC · lore

[PATCH] Install git-sh-setup.sh into $(prefix)/share/git-core. Call with explicit path.

git-sh-setup isn't a 'normal' binary, in that it should be called by bash only and not run in a subshell. Therefore, it should not be installed in a executable directory, but rather in $prefix/share/git-core/

From 4cefe86667d01ecc912c1f9d8b78c831c33792a1 Mon Sep 17 00:00:00 2001
From: Han-Wen Nienhuys <hanwen@xs4all.nl>
Date: Wed, 6 Dec 2006 13:27:16 +0100
Subject: 
Signed-off-by: Han-Wen Nienhuys <hanwen@xs4all.nl>
---
 Makefile            |   10 ++++++++--
 git-am.sh           |    2 +-
 git-applymbox.sh    |    2 +-
 git-applypatch.sh   |    2 +-
 git-bisect.sh       |    2 +-
 git-checkout.sh     |    2 +-
 git-clean.sh        |    2 +-
 git-commit.sh       |    2 +-
 git-fetch.sh        |    2 +-
 git-instaweb.sh     |    2 +-
 git-lost-found.sh   |    2 +-
 git-merge.sh        |    2 +-
 git-pull.sh         |    2 +-
 git-quiltimport.sh  |    2 +-
 git-rebase.sh       |    2 +-
 git-repack.sh       |    2 +-
 git-request-pull.sh |    2 +-
 git-reset.sh        |    2 +-
 git-resolve.sh      |    2 +-
 git-revert.sh       |    2 +-
 git-tag.sh          |    2 +-
 git-verify-tag.sh   |    2 +-
 22 files changed, 29 insertions(+), 23 deletions(-)
Show changes to 22 files +29 −23

Makefile, git-am.sh, git-applymbox.sh, git-applypatch.sh, git-bisect.sh, git-checkout.sh, git-clean.sh, git-commit.sh, git-fetch.sh, git-instaweb.sh, git-lost-found.sh, git-merge.sh, git-pull.sh, git-quiltimport.sh, git-rebase.sh, git-repack.sh, git-request-pull.sh, git-reset.sh, git-resolve.sh, git-revert.sh, git-tag.sh, git-verify-tag.sh

diff --git a/Makefile b/Makefile
index a1861de..cb9b745 100644
--- a/Makefile
+++ b/Makefile
@@ -116,8 +116,10 @@ STRIP ?= strip
 
 prefix = $(HOME)
 bindir = $(prefix)/bin
+datadir = $(prefix)/share
+GIT_datadir = $(datadir)/git-core
 gitexecdir = $(bindir)
-template_dir = $(prefix)/share/git-core/templates/
+template_dir = $(GIT_datadir)/templates/
 # DESTDIR=
 
 # default configuration for gitweb
@@ -166,7 +168,7 @@ SCRIPT_SH = \
 	git-merge-one-file.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
-	git-resolve.sh git-revert.sh git-sh-setup.sh \
+	git-resolve.sh git-revert.sh \
 	git-tag.sh git-verify-tag.sh \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
@@ -550,6 +552,7 @@ endif
 # Shell quote (do not use $(call) to accommodate ancient setups);
 
 SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
+GIT_datadir_SQ = $(subst ','\'',$(GIT_datadir))
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 bindir_SQ = $(subst ','\'',$(bindir))
@@ -604,6 +607,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's!@@GIT_datadir@@!$(GIT_datadir)!g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
 	    $@.sh >$@+
 	chmod +x $@+
@@ -816,6 +820,8 @@ install: all
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
 	fi
 	$(foreach p,$(BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
+	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(GIT_datadir_SQ)'
+	$(INSTALL) -m755 git-sh-setup.sh '$(DESTDIR_SQ)$(GIT_datadir_SQ)'
 
 install-doc:
 	$(MAKE) -C Documentation install
diff --git a/git-am.sh b/git-am.sh
index afe322b..b9dfcf0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -5,7 +5,7 @@
 USAGE='[--signoff] [--dotest=<dir>] [--utf8] [--binary] [--3way]
   [--interactive] [--whitespace=<option>] <mbox>...
   or, when resuming [--skip | --resolved]'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 git var GIT_COMMITTER_IDENT >/dev/null || exit
 
diff --git a/git-applymbox.sh b/git-applymbox.sh
index 5569fdc..eaab50d 100755
--- a/git-applymbox.sh
+++ b/git-applymbox.sh
@@ -19,7 +19,7 @@
 ## git-am is supposed to be the newer and better tool for this job.
 
 USAGE='[-u] [-k] [-q] [-m] (-c .dotest/<num> | mbox) [signoff]'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 git var GIT_COMMITTER_IDENT >/dev/null || exit
 
diff --git a/git-applypatch.sh b/git-applypatch.sh
index 8df2aee..7ddf5d3 100755
--- a/git-applypatch.sh
+++ b/git-applypatch.sh
@@ -12,7 +12,7 @@
 ##
 
 USAGE='<msg> <patch> <info> [<signoff>]'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 case "$#" in 3|4) ;; *) usage ;; esac
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 6da31e8..6fa92e2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -10,7 +10,7 @@ git bisect visualize            show bisect status in gitk.
 git bisect replay <logfile>	replay bisection log
 git bisect log			show bisect log.'
 
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 sq() {
 	@@PERL@@ -e '
diff --git a/git-checkout.sh b/git-checkout.sh
index 737abd0..beea113 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -2,7 +2,7 @@
 
 USAGE='[-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
 SUBDIRECTORY_OK=Sometimes
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 old_name=HEAD
 old=$(git-rev-parse --verify $old_name 2>/dev/null)
diff --git a/git-clean.sh b/git-clean.sh
index 3834323..7a7ce6e 100755
--- a/git-clean.sh
+++ b/git-clean.sh
@@ -13,7 +13,7 @@ LONG_USAGE='Clean untracked files from the working directory
 When optional <paths>... arguments are given, the paths
 affected are further limited to those that match them.'
 SUBDIRECTORY_OK=Yes
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 ignored=
 ignoredonly=
diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..5a388f5 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -5,7 +5,7 @@
 
 USAGE='[-a] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit>] [-u] [--amend] [-e] [--author <author>] [[-i | -o] <path>...]'
 SUBDIRECTORY_OK=Yes
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 git-rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
 branch=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD)
diff --git a/git-fetch.sh b/git-fetch.sh
index 4eecf14..efe44ea 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -2,7 +2,7 @@
 #
 
 USAGE='<fetch-options> <repository> <refspec>...'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 . git-parse-remote
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 16cd351..ae9fbc1 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -6,7 +6,7 @@ USAGE='[--start] [--stop] [--restart]
   [--local] [--httpd=<httpd>] [--port=<port>] [--browser=<browser>]
   [--module-path=<path> (for Apache2 only)]'
 
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 case "$GIT_DIR" in
 /*)
diff --git a/git-lost-found.sh b/git-lost-found.sh
index b928f2c..5f612f2 100755
--- a/git-lost-found.sh
+++ b/git-lost-found.sh
@@ -2,7 +2,7 @@
 
 USAGE=''
 SUBDIRECTORY_OK='Yes'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 if [ "$#" != "0" ]
 then
diff --git a/git-merge.sh b/git-merge.sh
index a948878..b83b4b9 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -5,7 +5,7 @@
 
 USAGE='[-n] [--no-commit] [--squash] [-s <strategy>] [--reflog-action=<action>] [-m=<merge-message>] <commit>+'
 
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 LF='
 '
diff --git a/git-pull.sh b/git-pull.sh
index e23beb6..8c0f292 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -6,7 +6,7 @@
 
 USAGE='[-n | --no-summary] [--no-commit] [-s strategy]... [<fetch-options>] <repo> <head>...'
 LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 strategy_args= no_summary= no_commit= squash=
 while case "$#,$1" in 0) break ;; *,-*) ;; *) break ;; esac
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 10135da..2f1e7b7 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 USAGE='--dry-run --author <author> --patches </path/to/quilt/patch/directory>'
 SUBDIRECTORY_ON=Yes
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 dry_run=""
 quilt_author=""
diff --git a/git-rebase.sh b/git-rebase.sh
index 25530df..3220291 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -27,7 +27,7 @@ Example:       git-rebase master~1 topic
        /                   -->           /
   D---E---F---G master          D---E---F---G master
 '
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 RESOLVEMSG="
 When you have resolved this problem run \"git rebase --continue\".
diff --git a/git-repack.sh b/git-repack.sh
index f150a55..e85f0b6 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -5,7 +5,7 @@
 
 USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--window=N] [--depth=N]'
 SUBDIRECTORY_OK='Yes'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 no_update_info= all_into_one= remove_redundant=
 local= quiet= no_reuse_delta= extra=
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 4eacc3a..fd02d55 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -8,7 +8,7 @@ USAGE='<commit> <url> [<head>]'
 LONG_USAGE='Summarizes the changes since <commit> to the standard output,
 and includes <url> in the message generated.'
 SUBDIRECTORY_OK='Yes'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 revision=$1
 url=$2
diff --git a/git-reset.sh b/git-reset.sh
index c0feb44..4c4f551 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 USAGE='[--mixed | --soft | --hard]  [<commit-ish>]'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 update=
 reset_type=--mixed
diff --git a/git-resolve.sh b/git-resolve.sh
index 36b90e3..4c2d7da 100755
--- a/git-resolve.sh
+++ b/git-resolve.sh
@@ -10,7 +10,7 @@ echo 'WARNING: Please use git-merge or git-pull instead.' >&2
 sleep 2
 
 USAGE='<head> <remote> <merge-message>'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 dropheads() {
 	rm -f -- "$GIT_DIR/MERGE_HEAD" \
diff --git a/git-revert.sh b/git-revert.sh
index 6eab3c7..8a30297 100755
--- a/git-revert.sh
+++ b/git-revert.sh
@@ -18,7 +18,7 @@ case "$0" in
 * )
 	die "What are you talking about?" ;;
 esac
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 no_commit=
 while case "$#" in 0) break ;; esac
diff --git a/git-tag.sh b/git-tag.sh
index d53f94c..a7d6223 100755
--- a/git-tag.sh
+++ b/git-tag.sh
@@ -3,7 +3,7 @@
 
 USAGE='-l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d] [-m <msg>] <tagname> [<head>]'
 SUBDIRECTORY_OK='Yes'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 message_given=
 annotate=
diff --git a/git-verify-tag.sh b/git-verify-tag.sh
index 36f171b..d265150 100755
--- a/git-verify-tag.sh
+++ b/git-verify-tag.sh
@@ -2,7 +2,7 @@
 
 USAGE='<tag>'
 SUBDIRECTORY_OK='Yes'
-. git-sh-setup
+. @@GIT_datadir@@/git-sh-setup.sh
 
 verbose=
 while case $# in 0) break;; esac

-- 
1.4.4.1.gc9922-dirty


-- 
 Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Nguyen Thai Ngoc Duy· Dec 8, 2006, 19:11 UTC · re: Han-Wen Nienhuys · lore
On 12/9/06, Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
>
> git-sh-setup isn't a 'normal' binary, in that it should be called by
> bash only and not run in a subshell. Therefore, it should not be installed
> in a executable directory, but rather in $prefix/share/git-core/

Executables should not go to $prefix/share I think. Maybe $prefix/libexec? BTW, I think you touched a larger issue: plumbling/porcelain separation.

Han-Wen Nienhuys· Dec 8, 2006, 20:50 UTC · re: Nguyen Thai Ngoc Duy · lore
Nguyen Thai Ngoc Duy escreveu:
Show 8 quoted lines
> On 12/9/06, Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
>>
>> git-sh-setup isn't a 'normal' binary, in that it should be called by
>> bash only and not run in a subshell. Therefore, it should not be
>> installed
>> in a executable directory, but rather in $prefix/share/git-core/
> 
> Executables should not go to $prefix/share I think. Maybe $prefix/libexec?
Yes, correct. Will change.
-- 
 Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Eric Wong· Dec 8, 2006, 19:37 UTC · re: Han-Wen Nienhuys · lore
Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
> 
> git-sh-setup isn't a 'normal' binary, in that it should be called by
> bash only and not run in a subshell. Therefore, it should not be installed
> in a executable directory, but rather in $prefix/share/git-core/

This seems to break existing hooks, or at least old repositories can be stuck using an old version of git-sh-setup in those hooks.

You could to add GIT_datadir to PATH before invoking hooks for existing ones to work, or write some sort of conversion script...

Show 24 quoted lines
> ---
>  Makefile            |   10 ++++++++--
>  git-am.sh           |    2 +-
>  git-applymbox.sh    |    2 +-
>  git-applypatch.sh   |    2 +-
>  git-bisect.sh       |    2 +-
>  git-checkout.sh     |    2 +-
>  git-clean.sh        |    2 +-
>  git-commit.sh       |    2 +-
>  git-fetch.sh        |    2 +-
>  git-instaweb.sh     |    2 +-
>  git-lost-found.sh   |    2 +-
>  git-merge.sh        |    2 +-
>  git-pull.sh         |    2 +-
>  git-quiltimport.sh  |    2 +-
>  git-rebase.sh       |    2 +-
>  git-repack.sh       |    2 +-
>  git-request-pull.sh |    2 +-
>  git-reset.sh        |    2 +-
>  git-resolve.sh      |    2 +-
>  git-revert.sh       |    2 +-
>  git-tag.sh          |    2 +-
>  git-verify-tag.sh   |    2 +-
>  22 files changed, 29 insertions(+), 23 deletions(-)
You shoudl probably change these, too:

templates/hooks--pre-applypatch:. git-sh-setup templates/hooks--applypatch-msg:. git-sh-setup

Junio C Hamano· Dec 8, 2006, 20:45 UTC · re: Eric Wong · lore
Eric Wong <normalperson@yhbt.net> writes:
Show 8 quoted lines
> Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
>> 
>> git-sh-setup isn't a 'normal' binary, in that it should be called by
>> bash only and not run in a subshell. Therefore, it should not be installed
>> in a executable directory, but rather in $prefix/share/git-core/
>
> This seems to break existing hooks, or at least old repositories can
> be stuck using an old version of git-sh-setup in those hooks.

Honestly speaking, I do not think this patch is worth it. People who care deeply enough should just use gitexecdir that is separate from bindir.

Han-Wen Nienhuys· Dec 9, 2006, 13:08 UTC · re: Junio C Hamano · lore
Junio C Hamano escreveu:
Show 12 quoted lines
> Eric Wong <normalperson@yhbt.net> writes:
> 
>> Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
>>> git-sh-setup isn't a 'normal' binary, in that it should be called by
>>> bash only and not run in a subshell. Therefore, it should not be installed
>>> in a executable directory, but rather in $prefix/share/git-core/
>> This seems to break existing hooks, or at least old repositories can
>> be stuck using an old version of git-sh-setup in those hooks.
> 
> Honestly speaking, I do not think this patch is worth it.
> People who care deeply enough should just use gitexecdir that is
> separate from bindir.

And how about coding the path of git-sh-setup (in <prefix>/bin/git-sh-setup) in the shell scripts? This will ensure that the sh-setup is the one from the right package, but still retain backward compatibility for scripts.

Junio C Hamano· Dec 9, 2006, 21:35 UTC · re: Han-Wen Nienhuys · lore
Han-Wen Nienhuys <hanwen@xs4all.nl> writes:
> And how about coding the path of git-sh-setup (in <prefix>/bin/git-sh-setup) 
> in the shell scripts? This will ensure that the sh-setup is the one
> from the right package, but still retain backward compatibility for 
> scripts.

Hardcoding $(bin)/git-sh-setup is a non-starter; it breaks GIT_EXEC_PATH convention to allow users to override it.

You would need to make sure the following:
 - If you are moving git-sh-setup out of the way, I think you
   would need to do the same for git-parse-remote while you are
   at it.  There might be other scriptlets whose sole purpose is
   to be sourced via "." (I haven't checked recently).
 - It should be arranged so that "make test" would test the
   version from the source tree, even when you have an installed
   version of git-sh-setup and friends elsewhere on your PATH.
 - People who build but do not install should be able to
   continue using git from the build location with appropriate
   environment settings (look for place that talks about PATH
   and GIT_EXEC_PATH in INSTALL -- if you need something else,
   you should document new requirements there).

I haven't looked closely, but I suspect your patch might have broken the latter two.

Having said that, I still do not see the point of doing this.

Was there ever a case that you typed git-sh-setup on the command line by mistake (in which case it might be a valid desire to prevent the mistake from happening again)? Command complettion might offer git-sh-setup when you say "git-<TAB>", which might irritate you --- but these days you can use Shawn's completion and say "git <TAB>" or "git somecommand<TAB>", which is way nicer, and makes it not really an issue either.

In other words, I do not know what real problem you are trying to solve.

Han-Wen Nienhuys· Dec 9, 2006, 23:40 UTC · re: Junio C Hamano · lore
Junio C Hamano escreveu:
Show 9 quoted lines
> Han-Wen Nienhuys <hanwen@xs4all.nl> writes:
> 
>> And how about coding the path of git-sh-setup (in <prefix>/bin/git-sh-setup) 
>> in the shell scripts? This will ensure that the sh-setup is the one
>> from the right package, but still retain backward compatibility for 
>> scripts.
> 
> Hardcoding $(bin)/git-sh-setup is a non-starter; it breaks
> GIT_EXEC_PATH convention to allow users to override it.
> In other words, I do not know what real problem you are trying
> to solve.

The real problem is that git is nonstandard to wrap around: scripts like git-sh-setup need special attention, because they don't work with

  exec /some/where/else/git-sh-setup "$@"
I admit is a small problem, but IMO that's not an argument not to solve it.
Junio C Hamano· Dec 10, 2006, 00:50 UTC · re: Han-Wen Nienhuys · lore
Han-Wen Nienhuys <hanwen@xs4all.nl> writes:
Show 5 quoted lines
> The real problem is that git is nonstandard to wrap around:
> scripts like git-sh-setup need special attention, because they don't work 
> with
>
>   exec /some/where/else/git-sh-setup "$@"
Of course.  Who would want to wrap git-sh-setup that way?

The only thing you would need to wrap around is "git" itself, isn't it?

← back to recent threads