git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Paul Mackerras <paulus@samba.org>,
	Pat Thoyts <patthoyts@users.sourceforge.net>,
	Clemens Buchacher <drizzd@aon.at>,
	Marc Branchaud <marcnarc@xiplink.com>
Subject: [PATCH 9/9] remove #!interpreter line from shell libraries
Date: Mon, 25 Nov 2013 13:03:52 -0800	[thread overview]
Message-ID: <20131125210352.GZ4212@google.com> (raw)
In-Reply-To: <20131125205119.GQ4212@google.com>

In a shell snippet meant to be sourced by other shell scripts, an
opening #! line does more harm than good.

The harm:

 - When the shell library is sourced, the interpreter and options from
   the #! line are not used.  Specifying a particular shell can
   confuse the reader into thinking it is safe for the shell library
   to rely on idiosyncrasies of that shell.

 - Using #! instead of a plain comment drops a helpful visual clue
   that this is a shell library and not a self-contained script.

 - Tools such as lintian can use a #! line to tell when an
   installation script has failed by forgetting to set a script
   executable.  This check does not work if shell libraries also start
   with a #! line.

The good:

 - Text editors notice the #! line and use it for syntax highlighting
   if you try to edit the installed scripts (without ".sh" suffix) in
   place.

The use of the #! for file type detection is not needed because Git's
shell libraries are meant to be edited in source form (with ".sh"
suffix).  Replace the opening #! lines with comments.

This involves tweaking the test harness's valgrind support to find
shell libraries by looking for "# " in the first line instead of "#!"
(see v1.7.6-rc3~7, 2011-06-17).

Suggested by Russ Allbery through lintian.  Thanks to Jeff King and
Clemens Buchacher for further analysis.

Tested by searching for non-executable scripts with #! line:

	find . -name .git -prune -o -type f -not -executable |
	while read file
	do
		read line <"$file"
		case $line in
		'#!'*)
			echo "$file"
			;;
		esac
	done

The only remaining scripts found are templates for shell scripts
(unimplemented.sh, wrap-for-bin.sh) and sample input used in tests
(t/t4034/perl/{pre,post}).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.

 contrib/completion/git-completion.bash | 2 --
 contrib/completion/git-completion.tcsh | 2 --
 git-mergetool--lib.sh                  | 3 +--
 git-parse-remote.sh                    | 4 +++-
 git-rebase--am.sh                      | 3 ++-
 git-rebase--interactive.sh             | 9 +++------
 git-rebase--merge.sh                   | 4 +++-
 git-sh-i18n.sh                         | 5 ++---
 git-sh-setup.sh                        | 9 +++------
 t/test-lib.sh                          | 6 ++----
 10 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dba3c15..be61931 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,5 +1,3 @@
-#!bash
-#
 # bash/zsh completion support for core Git.
 #
 # Copyright (C) 2006,2007 Shawn O. Pearce <spearce@spearce.org>
diff --git a/contrib/completion/git-completion.tcsh b/contrib/completion/git-completion.tcsh
index eaacaf0..6104a42 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -1,5 +1,3 @@
-#!tcsh
-#
 # tcsh completion support for core Git.
 #
 # Copyright (C) 2012 Marc Khouzam <marc.khouzam@gmail.com>
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index a280f49..c45a020 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,4 @@
-#!/bin/sh
-# git-mergetool--lib is a library for common merge tool functions
+# git-mergetool--lib is a shell library for common merge tool functions
 
 : ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
 
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 0e87e09..55fe8d5 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -1,4 +1,6 @@
-#!/bin/sh
+# This is a shell library to calculate the remote repository and
+# upstream branch that should be pulled by "git pull" from the current
+# branch.
 
 # git-ls-remote could be called from outside a git managed repository;
 # this would fail in that case and would issue an error message.
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 34e3102..a4f683a 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -1,4 +1,5 @@
-#!/bin/sh
+# This shell script fragment is sourced by git-rebase to implement
+# its default, fast, patch-based, non-interactive mode.
 #
 # Copyright (c) 2010 Junio C Hamano.
 #
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3c6bed9..43c19e0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1,11 +1,8 @@
-#!/bin/sh
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
-
-# SHORT DESCRIPTION
-#
-# This script makes it easy to fix up commits in the middle of a series,
-# and rearrange commits.
 #
 # The original idea comes from Eric W. Biederman, in
 # http://article.gmane.org/gmane.comp.version-control.git/22407
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 16d1817..e7d96de 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -1,4 +1,6 @@
-#!/bin/sh
+# This shell script fragment is sourced by git-rebase to implement
+# its merge-based non-interactive mode that copes well with renamed
+# files.
 #
 # Copyright (c) 2010 Junio C Hamano.
 #
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 6a27f68..e6c3116 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -1,9 +1,8 @@
-#!/bin/sh
+# This shell library is Git's interface to gettext.sh. See po/README
+# for usage instructions.
 #
 # Copyright (c) 2010 Ævar Arnfjörð Bjarmason
 #
-# This is Git's interface to gettext.sh. See po/README for usage
-# instructions.
 
 # Export the TEXTDOMAIN* data that we need for Git
 TEXTDOMAIN=git
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ebfe8f7..190a539 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -1,9 +1,6 @@
-#!/bin/sh
-#
-# This is included in commands that either have to be run from the toplevel
-# of the repository, or with GIT_DIR environment variable properly.
-# If the GIT_DIR does not look like the right correct git-repository,
-# it dies.
+# This shell scriplet is meant to be included by other shell scripts
+# to set up some variables pointing at the normal git directories and
+# a few helper shell functions.
 
 # Having this variable in your environment would break scripts because
 # you would cause "cd" to be taken to unexpected places.  If you
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c306bd0..c3e07b9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -573,11 +573,9 @@ then
 
 	make_valgrind_symlink () {
 		# handle only executables, unless they are shell libraries that
-		# need to be in the exec-path.  We will just use "#!" as a
-		# guess for a shell-script, since we have no idea what the user
-		# may have configured as the shell path.
+		# need to be in the exec-path.
 		test -x "$1" ||
-		test "#!" = "$(head -c 2 <"$1")" ||
+		test "# " = "$(head -c 2 <"$1")" ||
 		return;
 
 		base=$(basename "$1")
-- 
1.8.4.1

      parent reply	other threads:[~2013-11-25 21:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-25 20:51 [PATCH 0/9] War on #! lines in shell libraries Jonathan Nieder
2013-11-25 20:52 ` [PATCH 1/9] mark Windows build scripts executable Jonathan Nieder
2013-11-25 20:53 ` [PATCH 2/9] mark perl test " Jonathan Nieder
2013-11-25 20:55 ` [PATCH 3/9] mark contributed hooks executable Jonathan Nieder
2013-11-25 20:58 ` [PATCH 4/9] contrib: remove git-p4import Jonathan Nieder
2013-11-26 12:31   ` Pete Wyckoff
2013-11-25 21:00 ` [PATCH 5/9 gitk] gitk: chmod +x po2msg Jonathan Nieder
2013-11-25 21:01 ` [PATCH 6/9 git-gui] git-gui: chmod +x po2msg, windows/git-gui.sh Jonathan Nieder
2013-11-25 21:02 ` [PATCH 7/9] test: make FILEMODE a lazy prereq Jonathan Nieder
2013-11-25 21:03 ` [PATCH 8/9] test: replace shebangs with descriptions in shell libraries Jonathan Nieder
2013-11-26  5:18   ` Eric Sunshine
2013-11-26 21:39     ` [PATCH 8/9 v2] " Jonathan Nieder
2013-11-25 21:03 ` Jonathan Nieder [this message]

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=20131125210352.GZ4212@google.com \
    --to=jrnieder@gmail.com \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=marcnarc@xiplink.com \
    --cc=patthoyts@users.sourceforge.net \
    --cc=paulus@samba.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 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 NNTP newsgroup(s).