All of lore.kernel.org
 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 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.