[PATCH 0/9] remove dead shell code

[PATCH 0/9] remove dead shell code

[Ævar Arnfjörð Bjarmason]

Remove dead shell code in git-sh-setup, inspired by parallel
discussion on another topic (but the two don't conflict):
https://lore.kernel.org/git/87lf4f9gre.fsf@evledraar.gmail.com/

The last two patches were picked from a dropped series of mine
submitted earlier this year, it was dropped because of other more
complex patches that I haven't included here:
https://lore.kernel.org/git/20210311001447.28254-1-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (9):
  git-sh-setup: remove unused set_reflog_action() function
  git-sh-setup: remove unused git_editor() function
  git-sh-setup: remove unused git_pager() function
  git-sh-setup: remove unused sane_egrep() function
  git-sh-setup: remove unused require_work_tree_exists() function
  git-sh-setup: move create_virtual_base() to mergetools/p4merge
  git-sh-setup: move peel_committish() function to git-subtree.sh
  git-bisect: remove unused SHA-1 $x40 shell variable
  test-lib: remove unused $_x40 and $_z40 variables

 Documentation/git-sh-setup.txt |  24 --------
 Documentation/git.txt          |   4 --
 contrib/subtree/git-subtree.sh |  12 ++++
 git-bisect.sh                  |   2 -
 git-sh-setup.sh                | 103 ---------------------------------
 git-submodule.sh               |   5 --
 mergetools/p4merge             |  12 ++++
 t/t7006-pager.sh               |  13 -----
 t/test-lib.sh                  |   6 +-
 9 files changed, 26 insertions(+), 155 deletions(-)

-- 
2.33.0.814.gb82868f05f3

[PATCH 1/9] git-sh-setup: remove unused set_reflog_action() function

[Ævar Arnfjörð Bjarmason]

Remove the set_reflog_action() function last used in the
git-legacy-rebase.sh script removed in d03ebd411c6 (rebase: remove the
rebase.useBuiltin setting, 2019-03-18).

When the documentation I'm removing from git.txt was added in
c3e2d18996e (setup_reflog_action: document the rules for using
GIT_REFLOG_ACTION, 2013-06-19) we had git-pull.sh, git-am.sh and
git-rebase.sh using this in-tree, an addition to various example
scripts later removed in 49eb8d39c78 (Remove contrib/examples/*,
2018-03-25).

Since this part of the documentation was aimed at those writing
scripts in git.git we're long past the point where we should remove
it, this still leaves the description of the GIT_REFLOG_ACTION
variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-sh-setup.txt |  7 ------
 Documentation/git.txt          |  4 ----
 git-sh-setup.sh                | 42 ----------------------------------
 3 files changed, 53 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 8632612c31d..1ae15905492 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -40,13 +40,6 @@ die::
 usage::
 	die with the usage message.
 
-set_reflog_action::
-	Set `GIT_REFLOG_ACTION` environment to a given string (typically
-	the name of the program) unless it is already set.  Whenever
-	the script runs a `git` command that updates refs, a reflog
-	entry is created using the value of this string to leave the
-	record of what command updated the ref.
-
 git_editor::
 	runs an editor of user's choice (GIT_EDITOR, core.editor, VISUAL or
 	EDITOR) on a given file, but error out if no editor is specified
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6dd241ef838..38bc1403313 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -857,10 +857,6 @@ for full details.
 	track of the reason why the ref was updated (which is
 	typically the name of the high-level command that updated
 	the ref), in addition to the old and new values of the ref.
-	A scripted Porcelain command can use set_reflog_action
-	helper function in `git-sh-setup` to set its name to this
-	variable when it is invoked as the top level command by the
-	end user, to be recorded in the body of the reflog.
 
 `GIT_REF_PARANOIA`::
 	If set to `1`, include broken or badly named refs when iterating
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 10d97641856..ee6935ca455 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -106,48 +106,6 @@ $LONG_USAGE")"
 	esac
 fi
 
-# Set the name of the end-user facing command in the reflog when the
-# script may update refs.  When GIT_REFLOG_ACTION is already set, this
-# will not overwrite it, so that a scripted Porcelain (e.g. "git
-# rebase") can set it to its own name (e.g. "rebase") and then call
-# another scripted Porcelain (e.g. "git am") and a call to this
-# function in the latter will keep the name of the end-user facing
-# program (e.g. "rebase") in GIT_REFLOG_ACTION, ensuring whatever it
-# does will be record as actions done as part of the end-user facing
-# operation (e.g. "rebase").
-#
-# NOTE NOTE NOTE: consequently, after assigning a specific message to
-# GIT_REFLOG_ACTION when calling a "git" command to record a custom
-# reflog message, do not leave that custom value in GIT_REFLOG_ACTION,
-# after you are done.  Other callers of "git" commands that rely on
-# writing the default "program name" in reflog expect the variable to
-# contain the value set by this function.
-#
-# To use a custom reflog message, do either one of these three:
-#
-# (a) use a single-shot export form:
-#     GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: preparing frotz" \
-#         git command-that-updates-a-ref
-#
-# (b) save the original away and restore:
-#     SAVED_ACTION=$GIT_REFLOG_ACTION
-#     GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: preparing frotz"
-#     git command-that-updates-a-ref
-#     GIT_REFLOG_ACITON=$SAVED_ACTION
-#
-# (c) assign the variable in a subshell:
-#     (
-#         GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: preparing frotz"
-#         git command-that-updates-a-ref
-#     )
-set_reflog_action() {
-	if [ -z "${GIT_REFLOG_ACTION:+set}" ]
-	then
-		GIT_REFLOG_ACTION="$*"
-		export GIT_REFLOG_ACTION
-	fi
-}
-
 git_editor() {
 	if test -z "${GIT_EDITOR:+set}"
 	then
-- 
2.33.0.814.gb82868f05f3

[PATCH 2/9] git-sh-setup: remove unused git_editor() function

[Ævar Arnfjörð Bjarmason]

Remove the git_editor() function last referenced in
49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-sh-setup.txt | 5 -----
 git-sh-setup.sh                | 9 ---------
 2 files changed, 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 1ae15905492..2a28361cf66 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -40,11 +40,6 @@ die::
 usage::
 	die with the usage message.
 
-git_editor::
-	runs an editor of user's choice (GIT_EDITOR, core.editor, VISUAL or
-	EDITOR) on a given file, but error out if no editor is specified
-	and the terminal is dumb.
-
 is_bare_repository::
 	outputs `true` or `false` to the standard output stream
 	to indicate if the repository is a bare repository
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ee6935ca455..cfedda79471 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -106,15 +106,6 @@ $LONG_USAGE")"
 	esac
 fi
 
-git_editor() {
-	if test -z "${GIT_EDITOR:+set}"
-	then
-		GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
-	fi
-
-	eval "$GIT_EDITOR" '"$@"'
-}
-
 git_pager() {
 	if test -t 1
 	then
-- 
2.33.0.814.gb82868f05f3

[PATCH 3/9] git-sh-setup: remove unused git_pager() function

[Ævar Arnfjörð Bjarmason]

Remove the git_editor() function last referenced by non-test code code
in 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).

We can also remove the test for this added in 995bc22d7f8 (pager: move
pager-specific setup into the build, 2016-08-04), the test that
actually matters is the one added in e54c1f2d253 (pager: set LV=-c
alongside LESS=FRSX, 2014-01-06) just above the removed test.

I.e. we don't care if the "LESS" and "LV" variables are set by
git-sh-setup anymore, no built-in uses them, we do care that pager.c
sets them, which we still test for.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh  | 16 ----------------
 t/t7006-pager.sh | 13 -------------
 2 files changed, 29 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index cfedda79471..d4e8225affa 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -106,22 +106,6 @@ $LONG_USAGE")"
 	esac
 fi
 
-git_pager() {
-	if test -t 1
-	then
-		GIT_PAGER=$(git var GIT_PAGER)
-	else
-		GIT_PAGER=cat
-	fi
-	for vardef in @@PAGER_ENV@@
-	do
-		var=${vardef%%=*}
-		eval ": \"\${$vardef}\" && export $var"
-	done
-
-	eval "$GIT_PAGER" '"$@"'
-}
-
 sane_grep () {
 	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
 }
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435e..08f712a4497 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -49,19 +49,6 @@ test_expect_success TTY 'LESS and LV envvars are set for pagination' '
 	grep ^LV= pager-env.out
 '
 
-test_expect_success !MINGW,TTY 'LESS and LV envvars set by git-sh-setup' '
-	(
-		sane_unset LESS LV &&
-		PAGER="env >pager-env.out; wc" &&
-		export PAGER &&
-		PATH="$(git --exec-path):$PATH" &&
-		export PATH &&
-		test_terminal sh -c ". git-sh-setup && git_pager"
-	) &&
-	grep ^LESS= pager-env.out &&
-	grep ^LV= pager-env.out
-'
-
 test_expect_success TTY 'some commands do not use a pager' '
 	rm -f paginated.out &&
 	test_terminal git rev-list HEAD &&
-- 
2.33.0.814.gb82868f05f3

[PATCH 4/9] git-sh-setup: remove unused sane_egrep() function

[Ævar Arnfjörð Bjarmason]

The is_zero_oid() function in git-submodule.sh has not been used since
e83e3333b57 (submodule: port submodule subcommand 'summary' from shell
to C, 2020-08-13), so we can remove it, and the sane_egrep() function,
dead is_zero_oid() was the only function which still referenced it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh  | 4 ----
 git-submodule.sh | 5 -----
 2 files changed, 9 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index d4e8225affa..a2a28982b6d 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -110,10 +110,6 @@ sane_grep () {
 	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
 }
 
-sane_egrep () {
-	GREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ "$@"
-}
-
 is_bare_repository () {
 	git rev-parse --is-bare-repository
 }
diff --git a/git-submodule.sh b/git-submodule.sh
index dbd2ec20503..aeb96c58243 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -63,11 +63,6 @@ isnumber()
 	n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 
-# Given a full hex object ID, is this the zero OID?
-is_zero_oid () {
-	echo "$1" | sane_egrep '^0+$' >/dev/null 2>&1
-}
-
 # Sanitize the local git environment for use within a submodule. We
 # can't simply use clear_local_git_env since we want to preserve some
 # of the settings from GIT_CONFIG_PARAMETERS.
-- 
2.33.0.814.gb82868f05f3

[PATCH 5/9] git-sh-setup: remove unused require_work_tree_exists() function

[Ævar Arnfjörð Bjarmason]

The last code that used the require_work_tree_exists() function went
away in d03ebd411c6 (rebase: remove the rebase.useBuiltin setting,
2019-03-18), let's remove it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-sh-setup.txt | 6 ------
 git-sh-setup.sh                | 8 --------
 2 files changed, 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 2a28361cf66..1d8c87e9b2f 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -52,12 +52,6 @@ require_work_tree::
 	checks if the current directory is within the working tree
 	of the repository, and otherwise dies.
 
-require_work_tree_exists::
-	checks if the working tree associated with the repository
-	exists, and otherwise dies.  Often done before calling
-	cd_to_toplevel, which is impossible to do if there is no
-	working tree.
-
 require_clean_work_tree <action> [<hint>]::
 	checks that the working tree and index associated with the
 	repository have no uncommitted changes to tracked files.
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index a2a28982b6d..363c0096842 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -122,14 +122,6 @@ cd_to_toplevel () {
 	}
 }
 
-require_work_tree_exists () {
-	if test "z$(git rev-parse --is-bare-repository)" != zfalse
-	then
-		program_name=$0
-		die "$(eval_gettext "fatal: \$program_name cannot be used without a working tree.")"
-	fi
-}
-
 require_work_tree () {
 	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true || {
 		program_name=$0
-- 
2.33.0.814.gb82868f05f3

[PATCH 6/9] git-sh-setup: move create_virtual_base() to mergetools/p4merge

[Ævar Arnfjörð Bjarmason]

Move the create_virtual_base() function out of "git-sh-setup" to its
only user, "mergetools/p4merge". Since 1a92e53ba36 (merge-one-file:
use empty blob for add/add base, 2016-02-23) when
git-merge-one-file.sh stopped using it, it has only been used in
"mergetools/p4merge".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-sh-setup.txt |  6 ------
 git-sh-setup.sh                | 12 ------------
 mergetools/p4merge             | 12 ++++++++++++
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 1d8c87e9b2f..2febe754206 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -66,12 +66,6 @@ get_author_ident_from_commit::
 	outputs code for use with eval to set the GIT_AUTHOR_NAME,
 	GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
 
-create_virtual_base::
-	modifies the first file so only lines in common with the
-	second file remain. If there is insufficient common material,
-	then the first file is left empty. The result is suitable
-	as a virtual base input for a 3-way merge.
-
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 363c0096842..6a21238dc0e 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -238,18 +238,6 @@ clear_local_git_env() {
 	unset $(git rev-parse --local-env-vars)
 }
 
-# Generate a virtual base file for a two-file merge. Uses git apply to
-# remove lines from $1 that are not in $2, leaving only common lines.
-create_virtual_base() {
-	sz0=$(wc -c <"$1")
-	@@DIFF@@ -u -La/"$1" -Lb/"$1" "$1" "$2" | git apply --no-add
-	sz1=$(wc -c <"$1")
-
-	# If we do not have enough common material, it is not
-	# worth trying two-file merge using common subsections.
-	expr $sz0 \< $sz1 \* 2 >/dev/null || : >"$1"
-}
-
 
 # Platform specific tweaks to work around some commands
 case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 7a5b291dd28..23f8735efa2 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -19,6 +19,18 @@ diff_cmd () {
 	fi
 }
 
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+	sz0=$(wc -c <"$1")
+	@@DIFF@@ -u -La/"$1" -Lb/"$1" "$1" "$2" | git apply --no-add
+	sz1=$(wc -c <"$1")
+
+	# If we do not have enough common material, it is not
+	# worth trying two-file merge using common subsections.
+	expr $sz0 \< $sz1 \* 2 >/dev/null || : >"$1"
+}
+
 merge_cmd () {
 	if ! $base_present
 	then
-- 
2.33.0.814.gb82868f05f3

[PATCH 7/9] git-sh-setup: move peel_committish() function to git-subtree.sh

[Ævar Arnfjörð Bjarmason]

Move the peel_committish() function out of git-sh-setup to its only
user, contrib/subtree/git-subtree.sh. Since d03ebd411c6 (rebase:
remove the rebase.useBuiltin setting, 2019-03-18) when
git-legacy-rebase.sh was removed, it has only been used in
git-subtree.sh.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/subtree/git-subtree.sh | 12 ++++++++++++
 git-sh-setup.sh                | 12 ------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7f767b5c38f..a6deb57bcae 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -818,6 +818,18 @@ cmd_add_repository () {
 	cmd_add_commit FETCH_HEAD
 }
 
+peel_committish () {
+	case "$1" in
+	:/*)
+		peeltmp=$(git rev-parse --verify "$1") &&
+		git rev-parse --verify "${peeltmp}^0"
+		;;
+	*)
+		git rev-parse --verify "${1}^0"
+		;;
+	esac
+}
+
 # Usage: cmd_add_commit REV
 cmd_add_commit () {
 	# The rev has already been validated by cmd_add(), we just
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 6a21238dc0e..9243353bc21 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -294,15 +294,3 @@ if test -z "$NONGIT_OK"
 then
 	git_dir_init
 fi
-
-peel_committish () {
-	case "$1" in
-	:/*)
-		peeltmp=$(git rev-parse --verify "$1") &&
-		git rev-parse --verify "${peeltmp}^0"
-		;;
-	*)
-		git rev-parse --verify "${1}^0"
-		;;
-	esac
-}
-- 
2.33.0.814.gb82868f05f3

[PATCH 8/9] git-bisect: remove unused SHA-1 $x40 shell variable

[Ævar Arnfjörð Bjarmason]

This variable was last used in code removed in
06f5608c14 (bisect--helper: `bisect_start` shell function partially in
C, 2019-01-02).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-bisect.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 6a7afaea8da..b59f3aaad43 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -34,8 +34,6 @@ Please use "git help bisect" to get the full man page.'
 OPTIONS_SPEC=
 . git-sh-setup
 
-_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"
 TERM_BAD=bad
 TERM_GOOD=good
 
-- 
2.33.0.814.gb82868f05f3

[PATCH 9/9] test-lib: remove unused $_x40 and $_z40 variables

[Ævar Arnfjörð Bjarmason]

These two have fallen out of use with the SHA-256 migration.

The last use of $_x40 was removed in fc7e73d7ef (t4013: improve
diff-post-processor logic, 2020-08-21) and

The last use of $_z40 was removed in 7a868c51c2 (t5562: use $ZERO_OID,
2019-12-21), but it was then needlessly refactored to be hash-agnostic
in 192b517589 (t: use hash-specific lookup tables to define test
constants, 2020-02-22). We can just remove it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index abcfbed6d61..044a9231ae6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -534,7 +534,7 @@ SQ=\'
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x35 _x40 _z40 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
+export _x05 _x35 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
 
 # Each test should start with something like this, after copyright notices:
 #
@@ -1422,10 +1422,9 @@ then
 fi
 
 # Convenience
-# A regexp to match 5, 35 and 40 hexdigits
+# A regexp to match 5 and 35 hexdigits
 _x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x35="$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
-_x40="$_x35$_x05"
 
 test_oid_init
 
@@ -1434,7 +1433,6 @@ OID_REGEX=$(echo $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 OIDPATH_REGEX=$(test_oid_to_path $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 EMPTY_TREE=$(test_oid empty_tree)
 EMPTY_BLOB=$(test_oid empty_blob)
-_z40=$ZERO_OID
 
 # Provide an implementation of the 'yes' utility; the upper bound
 # limit is there to help Windows that cannot stop this loop from
-- 
2.33.0.814.gb82868f05f3

Re: [PATCH 3/9] git-sh-setup: remove unused git_pager() function

[Philippe Blain]

Hi Ævar,

Le 2021-09-02 à 12:01, Ævar Arnfjörð Bjarmason a écrit :
> Remove the git_editor() function last referenced by non-test code code
> in 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
> 

s/code code/code/

Thanks,

Philippe.

Re: [PATCH 0/9] remove dead shell code

[Peter Baumann]

On Thu, Sep 2, 2021 at 6:04 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> Remove dead shell code in git-sh-setup, inspired by parallel
> discussion on another topic (but the two don't conflict):
> https://lore.kernel.org/git/87lf4f9gre.fsf@evledraar.gmail.com/
>
> The last two patches were picked from a dropped series of mine
> submitted earlier this year, it was dropped because of other more
> complex patches that I haven't included here:
> https://lore.kernel.org/git/20210311001447.28254-1-avarab@gmail.com/
>
[...]

Hm, I have scripts here, implementing some porcelain commands which
follow the same approach as
the git porcelain scripts, e.g.

  #!/bin/bash
  SUBDIRECTORY_OK=Yes
  . git-sh-setup
  require_work_tree
  require_clean_work_tree
  cd_to_toplevel || die "Can't find top level for the git repo"
  set_reflog_action my-special-script                          # this
will be broken by the patch series

I was under the impression that this is how it should be done when one
needs to write some custom git scripts.
If that is the case, then removing these functions might break some
other users scripts out there.
At least  I have a quite an old self written script here which uses
`set_reflog_action`.
Obviously I can manage and adapt the script, but the more generic
question comes to mind if sourcing git-sh-setup
by end users is the correct approach to write porcelain scripts!

-Peter

Re: [PATCH 0/9] remove dead shell code

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Remove dead shell code in git-sh-setup, inspired by parallel
> discussion on another topic (but the two don't conflict):
> https://lore.kernel.org/git/87lf4f9gre.fsf@evledraar.gmail.com/
>
> The last two patches were picked from a dropped series of mine
> submitted earlier this year, it was dropped because of other more
> complex patches that I haven't included here:
> https://lore.kernel.org/git/20210311001447.28254-1-avarab@gmail.com/
>
> Ævar Arnfjörð Bjarmason (9):
>   git-sh-setup: remove unused set_reflog_action() function
>   git-sh-setup: remove unused git_editor() function
>   git-sh-setup: remove unused git_pager() function
>   git-sh-setup: remove unused sane_egrep() function
>   git-sh-setup: remove unused require_work_tree_exists() function
>   git-sh-setup: move create_virtual_base() to mergetools/p4merge
>   git-sh-setup: move peel_committish() function to git-subtree.sh
>   git-bisect: remove unused SHA-1 $x40 shell variable
>   test-lib: remove unused $_x40 and $_z40 variables

Was "unused" above decided based solely on the presence of in-tree
users?  If that is the case, I do not think we want to take these
sh-setup changes.

The implementation details of the remaining part of git-bisect.sh
and test-lib.sh are OK, of course, as that is truly our local
issue.

Thanks.

Re: [PATCH 0/9] remove dead shell code

[Junio C Hamano]

Peter Baumann <peter.baumann@gmail.com> writes:

> Hm, I have scripts here, implementing some porcelain commands which
> follow the same approach as
> the git porcelain scripts, e.g.
>
>   #!/bin/bash
>   SUBDIRECTORY_OK=Yes
>   . git-sh-setup
>   require_work_tree
>   require_clean_work_tree
>   cd_to_toplevel || die "Can't find top level for the git repo"
>   set_reflog_action my-special-script                          # this
> will be broken by the patch series
>
> I was under the impression that this is how it should be done when one
> needs to write some custom git scripts.

The reason why output from "git log --stat -- git-sh-setup.sh" does
not have that much removal is exactly this.  These are part of our
published API.  It is very much appreciated that you raised this
point.

Thanks.

Re: [PATCH 3/9] git-sh-setup: remove unused git_pager() function

[Andrei Rybak]

On 02/09/2021 18:34, Philippe Blain wrote:
> Hi Ævar,
> 
> Le 2021-09-02 à 12:01, Ævar Arnfjörð Bjarmason a écrit :
>> Remove the git_editor() function last referenced by non-test code code

Also s/git_editor/git_pager/

>> in 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>>
> 
> s/code code/code/
> 
> Thanks,
> 
> Philippe.

Re: [PATCH 0/9] remove dead shell code

[Carlo Arenas]

On Thu, Sep 2, 2021 at 1:58 PM Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> > Ævar Arnfjörð Bjarmason (9):
> >   git-sh-setup: remove unused set_reflog_action() function
> >   git-sh-setup: remove unused git_editor() function
> >   git-sh-setup: remove unused git_pager() function
> >   git-sh-setup: remove unused sane_egrep() function
> >   git-sh-setup: remove unused require_work_tree_exists() function
> >   git-sh-setup: move create_virtual_base() to mergetools/p4merge
> >   git-sh-setup: move peel_committish() function to git-subtree.sh
> >   git-bisect: remove unused SHA-1 $x40 shell variable
> >   test-lib: remove unused $_x40 and $_z40 variables
>
> Was "unused" above decided based solely on the presence of in-tree
> users?  If that is the case, I do not think we want to take these
> sh-setup changes.
>
> The implementation details of the remaining part of git-bisect.sh
> and test-lib.sh are OK, of course, as that is truly our local
> issue.

The removal of sane_egrep() is also unlikely to cause issues, as it
wasn't documented as a public API, and it seems similar to the
git-bisect's implementation details you refer to, except for
git-submodule.

Dropping it now would avoid having to change it to `grep -E` as egrep
gets obsoleted.

In that line, git_pager() and peel_committish() aren't documented
either; document or drop?

Carlo

Re: [PATCH 0/9] remove dead shell code

[Ævar Arnfjörð Bjarmason]

On Thu, Sep 02 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Remove dead shell code in git-sh-setup, inspired by parallel
>> discussion on another topic (but the two don't conflict):
>> https://lore.kernel.org/git/87lf4f9gre.fsf@evledraar.gmail.com/
>>
>> The last two patches were picked from a dropped series of mine
>> submitted earlier this year, it was dropped because of other more
>> complex patches that I haven't included here:
>> https://lore.kernel.org/git/20210311001447.28254-1-avarab@gmail.com/
>>
>> Ævar Arnfjörð Bjarmason (9):
>>   git-sh-setup: remove unused set_reflog_action() function
>>   git-sh-setup: remove unused git_editor() function
>>   git-sh-setup: remove unused git_pager() function
>>   git-sh-setup: remove unused sane_egrep() function
>>   git-sh-setup: remove unused require_work_tree_exists() function
>>   git-sh-setup: move create_virtual_base() to mergetools/p4merge
>>   git-sh-setup: move peel_committish() function to git-subtree.sh
>>   git-bisect: remove unused SHA-1 $x40 shell variable
>>   test-lib: remove unused $_x40 and $_z40 variables
>
> Was "unused" above decided based solely on the presence of in-tree
> users?  If that is the case, I do not think we want to take these
> sh-setup changes.

I should have remembered to reference the earlier discussion, but I
think we had this exact discussion around a year ago when I submitted
patches to remove git-parse-remote.sh, and decided this direction was
OK.

See a89a2fbfccd (parse-remote: remove this now-unused library,
2020-11-14) and the thread starting at
<20201111173738.GB9902@coredump.intra.peff.net>:
https://lore.kernel.org/git/20201111173738.GB9902@coredump.intra.peff.net/

You'll know better what you meant, but I interpreted the docs you added
for git-sh-setup in 850844e28f7 (Documentation/git-sh-setup.txt:
programmer's docs, 2007-01-17) as a guide for in-tree porcelain scripts.

As noted in my recently sent <87lf4f9gre.fsf@evledraar.gmail.com>
(https://lore.kernel.org/git/87lf4f9gre.fsf@evledraar.gmail.com/) the
eventual goal I have in mind here is to get rid of git-sh-i18n.sh.

If we're set on maintaining these shell libraries indefinitely even
after in-tree users have gone away that pretty much means we can't do
that, which would be unfortunate. We continue paying for quite a bit of
technical debt to extend certain parts of core C git functionality to
*.sh and *.perl.

Re: [PATCH 0/9] remove dead shell code

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> You'll know better what you meant, but I interpreted the docs you added
> for git-sh-setup in 850844e28f7 (Documentation/git-sh-setup.txt:
> programmer's docs, 2007-01-17) as a guide for in-tree porcelain scripts.

No, it is not for "in-tree".  Especially back then, one of the Git's
goal was to be and to stay scriptable, and git-sh-setup was for those
who are scripting, either "in-tree" or custom Porcelain people wrote
around Git as part of a larger Git ecosystem.

Re: [PATCH 0/9] remove dead shell code

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> Dropping it now would avoid having to change it to `grep -E` as egrep
> gets obsoleted.

Keeping it would isolate the callers from "grep -E" vs "egrep"
because they do not need to know.  They can keep calling
sane_egrep().

Having said that, I do not think anybody minds losing sane_egrep().
Many helper functions in the shell library are about encapsulating
the knowledge we have on and around Git and making it easier for
third-party script writers to use the knowledge.  The functions like
is_bare_repository(), cd_to_toplevel(), and git_pager() all fall
into that category.  sane_egrep is not that kind of a helper (it
encapusulates our knowledge about GNU grep's quirk we found when we
tried to use it in our scripts---third-party script writers do not
need help from Git experts on their use of egrep).

Thanks.

[PATCH v2 0/7] remove dead & undocumented shell code

[Ævar Arnfjörð Bjarmason]

This re-roll should address all the comments on the v1, in particular
all of the changes that removed in-tree unused functions that are
documented in our manpages were ejected. This series is now only
removals of code that should always have been in-tree-only.

The minor change in 4/7 is new, and so is 5/7.

Ævar Arnfjörð Bjarmason (7):
  git-sh-setup: remove unused git_pager() function
  git-sh-setup: remove unused sane_egrep() function
  git-sh-setup: move peel_committish() function to git-subtree.sh
  git-sh-setup: clear_local_git_env() function to git-submodule.sh
  git-sh-setup: remove unused "pull with rebase" message
  git-bisect: remove unused SHA-1 $x40 shell variable
  test-lib: remove unused $_x40 and $_z40 variables

 contrib/subtree/git-subtree.sh | 12 +++++++++
 git-bisect.sh                  |  2 --
 git-sh-setup.sh                | 45 ----------------------------------
 git-submodule.sh               |  7 +-----
 t/t7006-pager.sh               | 13 ----------
 t/test-lib.sh                  |  6 ++---
 6 files changed, 15 insertions(+), 70 deletions(-)

Range-diff against v1:
 1:  2e3ed8061d5 <  -:  ----------- git-sh-setup: remove unused set_reflog_action() function
 2:  7d3ea928099 <  -:  ----------- git-sh-setup: remove unused git_editor() function
 3:  73e540896fc !  1:  8eb1dfbff5d git-sh-setup: remove unused git_pager() function
    @@ Metadata
      ## Commit message ##
         git-sh-setup: remove unused git_pager() function
     
    -    Remove the git_editor() function last referenced by non-test code code
    -    in 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
    +    Remove the git_pager() function last referenced by non-test code in
    +    49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
     
         We can also remove the test for this added in 995bc22d7f8 (pager: move
         pager-specific setup into the build, 2016-08-04), the test that
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## git-sh-setup.sh ##
    -@@ git-sh-setup.sh: $LONG_USAGE")"
    - 	esac
    - fi
    +@@ git-sh-setup.sh: git_editor() {
    + 	eval "$GIT_EDITOR" '"$@"'
    + }
      
     -git_pager() {
     -	if test -t 1
 4:  73f0676db7a =  2:  e7f3115797c git-sh-setup: remove unused sane_egrep() function
 5:  dc4dd7d1399 <  -:  ----------- git-sh-setup: remove unused require_work_tree_exists() function
 6:  d2d65f3d77f <  -:  ----------- git-sh-setup: move create_virtual_base() to mergetools/p4merge
 7:  a3047b93f7d =  3:  d92e880fcfa git-sh-setup: move peel_committish() function to git-subtree.sh
 -:  ----------- >  4:  46c018aa860 git-sh-setup: clear_local_git_env() function to git-submodule.sh
 -:  ----------- >  5:  45c1369e958 git-sh-setup: remove unused "pull with rebase" message
 8:  88dffac9088 =  6:  bcae7884bb0 git-bisect: remove unused SHA-1 $x40 shell variable
 9:  0fd1516af85 =  7:  479e94f22f4 test-lib: remove unused $_x40 and $_z40 variables
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Ævar Arnfjörð Bjarmason]

Remove the git_pager() function last referenced by non-test code in
49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).

We can also remove the test for this added in 995bc22d7f8 (pager: move
pager-specific setup into the build, 2016-08-04), the test that
actually matters is the one added in e54c1f2d253 (pager: set LV=-c
alongside LESS=FRSX, 2014-01-06) just above the removed test.

I.e. we don't care if the "LESS" and "LV" variables are set by
git-sh-setup anymore, no built-in uses them, we do care that pager.c
sets them, which we still test for.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh  | 16 ----------------
 t/t7006-pager.sh | 13 -------------
 2 files changed, 29 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 10d97641856..7ee4b0edff5 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -157,22 +157,6 @@ git_editor() {
 	eval "$GIT_EDITOR" '"$@"'
 }
 
-git_pager() {
-	if test -t 1
-	then
-		GIT_PAGER=$(git var GIT_PAGER)
-	else
-		GIT_PAGER=cat
-	fi
-	for vardef in @@PAGER_ENV@@
-	do
-		var=${vardef%%=*}
-		eval ": \"\${$vardef}\" && export $var"
-	done
-
-	eval "$GIT_PAGER" '"$@"'
-}
-
 sane_grep () {
 	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
 }
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435e..08f712a4497 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -49,19 +49,6 @@ test_expect_success TTY 'LESS and LV envvars are set for pagination' '
 	grep ^LV= pager-env.out
 '
 
-test_expect_success !MINGW,TTY 'LESS and LV envvars set by git-sh-setup' '
-	(
-		sane_unset LESS LV &&
-		PAGER="env >pager-env.out; wc" &&
-		export PAGER &&
-		PATH="$(git --exec-path):$PATH" &&
-		export PATH &&
-		test_terminal sh -c ". git-sh-setup && git_pager"
-	) &&
-	grep ^LESS= pager-env.out &&
-	grep ^LV= pager-env.out
-'
-
 test_expect_success TTY 'some commands do not use a pager' '
 	rm -f paginated.out &&
 	test_terminal git rev-list HEAD &&
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 2/7] git-sh-setup: remove unused sane_egrep() function

[Ævar Arnfjörð Bjarmason]

The is_zero_oid() function in git-submodule.sh has not been used since
e83e3333b57 (submodule: port submodule subcommand 'summary' from shell
to C, 2020-08-13), so we can remove it, and the sane_egrep() function,
dead is_zero_oid() was the only function which still referenced it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh  | 4 ----
 git-submodule.sh | 5 -----
 2 files changed, 9 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7ee4b0edff5..c170be07c7d 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -161,10 +161,6 @@ sane_grep () {
 	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
 }
 
-sane_egrep () {
-	GREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ "$@"
-}
-
 is_bare_repository () {
 	git rev-parse --is-bare-repository
 }
diff --git a/git-submodule.sh b/git-submodule.sh
index dbd2ec20503..aeb96c58243 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -63,11 +63,6 @@ isnumber()
 	n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 
-# Given a full hex object ID, is this the zero OID?
-is_zero_oid () {
-	echo "$1" | sane_egrep '^0+$' >/dev/null 2>&1
-}
-
 # Sanitize the local git environment for use within a submodule. We
 # can't simply use clear_local_git_env since we want to preserve some
 # of the settings from GIT_CONFIG_PARAMETERS.
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 3/7] git-sh-setup: move peel_committish() function to git-subtree.sh

[Ævar Arnfjörð Bjarmason]

Move the peel_committish() function out of git-sh-setup to its only
user, contrib/subtree/git-subtree.sh. Since d03ebd411c6 (rebase:
remove the rebase.useBuiltin setting, 2019-03-18) when
git-legacy-rebase.sh was removed, it has only been used in
git-subtree.sh.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/subtree/git-subtree.sh | 12 ++++++++++++
 git-sh-setup.sh                | 12 ------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7f767b5c38f..a6deb57bcae 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -818,6 +818,18 @@ cmd_add_repository () {
 	cmd_add_commit FETCH_HEAD
 }
 
+peel_committish () {
+	case "$1" in
+	:/*)
+		peeltmp=$(git rev-parse --verify "$1") &&
+		git rev-parse --verify "${peeltmp}^0"
+		;;
+	*)
+		git rev-parse --verify "${1}^0"
+		;;
+	esac
+}
+
 # Usage: cmd_add_commit REV
 cmd_add_commit () {
 	# The rev has already been validated by cmd_add(), we just
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c170be07c7d..3fc8830cb36 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -365,15 +365,3 @@ if test -z "$NONGIT_OK"
 then
 	git_dir_init
 fi
-
-peel_committish () {
-	case "$1" in
-	:/*)
-		peeltmp=$(git rev-parse --verify "$1") &&
-		git rev-parse --verify "${peeltmp}^0"
-		;;
-	*)
-		git rev-parse --verify "${1}^0"
-		;;
-	esac
-}
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 4/7] git-sh-setup: clear_local_git_env() function to git-submodule.sh

[Ævar Arnfjörð Bjarmason]

Since 14111fc4927 (git: submodule honor -c credential.* from command
line, 2016-02-29) the clear_local_git_env() function has only been
used in the sanitize_submodule_env() function, let's inline its
one-line functionality there.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh  | 7 -------
 git-submodule.sh | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 3fc8830cb36..b3a97d6455a 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -290,13 +290,6 @@ get_author_ident_from_commit () {
 	parse_ident_from_commit author AUTHOR
 }
 
-# Clear repo-local GIT_* environment variables. Useful when switching to
-# another repository (e.g. when entering a submodule). See also the env
-# list in git_connect()
-clear_local_git_env() {
-	unset $(git rev-parse --local-env-vars)
-}
-
 # Generate a virtual base file for a two-file merge. Uses git apply to
 # remove lines from $1 that are not in $2, leaving only common lines.
 create_virtual_base() {
diff --git a/git-submodule.sh b/git-submodule.sh
index aeb96c58243..c9dca58fdaa 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -69,7 +69,7 @@ isnumber()
 sanitize_submodule_env()
 {
 	save_config=$GIT_CONFIG_PARAMETERS
-	clear_local_git_env
+	unset $(git rev-parse --local-env-vars)
 	GIT_CONFIG_PARAMETERS=$save_config
 	export GIT_CONFIG_PARAMETERS
 }
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 5/7] git-sh-setup: remove unused "pull with rebase" message

[Ævar Arnfjörð Bjarmason]

Remove the "pull with rebase" message previously used by the
git-pull.sh script, which was removed in 49eb8d39c78 (Remove
contrib/examples/*, 2018-03-25).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index b3a97d6455a..acbd05fe25b 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -203,9 +203,6 @@ require_clean_work_tree () {
 		"rewrite branches")
 			gettextln "Cannot rewrite branches: You have unstaged changes." >&2
 			;;
-		"pull with rebase")
-			gettextln "Cannot pull with rebase: You have unstaged changes." >&2
-			;;
 		*)
 			eval_gettextln "Cannot \$action: You have unstaged changes." >&2
 			;;
@@ -222,9 +219,6 @@ require_clean_work_tree () {
 			rebase)
 				gettextln "Cannot rebase: Your index contains uncommitted changes." >&2
 				;;
-			"pull with rebase")
-				gettextln "Cannot pull with rebase: Your index contains uncommitted changes." >&2
-				;;
 			*)
 				eval_gettextln "Cannot \$action: Your index contains uncommitted changes." >&2
 				;;
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 6/7] git-bisect: remove unused SHA-1 $x40 shell variable

[Ævar Arnfjörð Bjarmason]

This variable was last used in code removed in
06f5608c14 (bisect--helper: `bisect_start` shell function partially in
C, 2019-01-02).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-bisect.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 6a7afaea8da..b59f3aaad43 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -34,8 +34,6 @@ Please use "git help bisect" to get the full man page.'
 OPTIONS_SPEC=
 . git-sh-setup
 
-_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"
 TERM_BAD=bad
 TERM_GOOD=good
 
-- 
2.33.0.821.gfd4106eadbd

[PATCH v2 7/7] test-lib: remove unused $_x40 and $_z40 variables

[Ævar Arnfjörð Bjarmason]

These two have fallen out of use with the SHA-256 migration.

The last use of $_x40 was removed in fc7e73d7ef (t4013: improve
diff-post-processor logic, 2020-08-21) and

The last use of $_z40 was removed in 7a868c51c2 (t5562: use $ZERO_OID,
2019-12-21), but it was then needlessly refactored to be hash-agnostic
in 192b517589 (t: use hash-specific lookup tables to define test
constants, 2020-02-22). We can just remove it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index abcfbed6d61..044a9231ae6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -534,7 +534,7 @@ SQ=\'
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x35 _x40 _z40 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
+export _x05 _x35 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
 
 # Each test should start with something like this, after copyright notices:
 #
@@ -1422,10 +1422,9 @@ then
 fi
 
 # Convenience
-# A regexp to match 5, 35 and 40 hexdigits
+# A regexp to match 5 and 35 hexdigits
 _x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x35="$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
-_x40="$_x35$_x05"
 
 test_oid_init
 
@@ -1434,7 +1433,6 @@ OID_REGEX=$(echo $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 OIDPATH_REGEX=$(test_oid_to_path $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 EMPTY_TREE=$(test_oid empty_tree)
 EMPTY_BLOB=$(test_oid empty_blob)
-_z40=$ZERO_OID
 
 # Provide an implementation of the 'yes' utility; the upper bound
 # limit is there to help Windows that cannot stop this loop from
-- 
2.33.0.821.gfd4106eadbd

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Phillip Wood]

Hi Ævar

On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
> Remove the git_pager() function last referenced by non-test code in
> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
> 
> We can also remove the test for this added in 995bc22d7f8 (pager: move
> pager-specific setup into the build, 2016-08-04), the test that
> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
> alongside LESS=FRSX, 2014-01-06) just above the removed test.
> 
> I.e. we don't care if the "LESS" and "LV" variables are set by
> git-sh-setup anymore, no built-in uses them, we do care that pager.c
> sets them, which we still test for.

git_pager() might not be documented but I think it is useful for script 
authors and I wouldn't be surprised if someone out there is using it. 
The same goes for peel_committish(). It does not seem like a huge 
maintenance burden to keep and maybe document these two functions.

Best Wishes

Phillip

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   git-sh-setup.sh  | 16 ----------------
>   t/t7006-pager.sh | 13 -------------
>   2 files changed, 29 deletions(-)
> 
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 10d97641856..7ee4b0edff5 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -157,22 +157,6 @@ git_editor() {
>   	eval "$GIT_EDITOR" '"$@"'
>   }
>   
> -git_pager() {
> -	if test -t 1
> -	then
> -		GIT_PAGER=$(git var GIT_PAGER)
> -	else
> -		GIT_PAGER=cat
> -	fi
> -	for vardef in @@PAGER_ENV@@
> -	do
> -		var=${vardef%%=*}
> -		eval ": \"\${$vardef}\" && export $var"
> -	done
> -
> -	eval "$GIT_PAGER" '"$@"'
> -}
> -
>   sane_grep () {
>   	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
>   }
> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index 0e7cf75435e..08f712a4497 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -49,19 +49,6 @@ test_expect_success TTY 'LESS and LV envvars are set for pagination' '
>   	grep ^LV= pager-env.out
>   '
>   
> -test_expect_success !MINGW,TTY 'LESS and LV envvars set by git-sh-setup' '
> -	(
> -		sane_unset LESS LV &&
> -		PAGER="env >pager-env.out; wc" &&
> -		export PAGER &&
> -		PATH="$(git --exec-path):$PATH" &&
> -		export PATH &&
> -		test_terminal sh -c ". git-sh-setup && git_pager"
> -	) &&
> -	grep ^LESS= pager-env.out &&
> -	grep ^LV= pager-env.out
> -'
> -
>   test_expect_success TTY 'some commands do not use a pager' '
>   	rm -f paginated.out &&
>   	test_terminal git rev-list HEAD &&
> 

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 06 2021, Phillip Wood wrote:

> Hi Ævar
>
> On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
>> Remove the git_pager() function last referenced by non-test code in
>> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>> We can also remove the test for this added in 995bc22d7f8 (pager:
>> move
>> pager-specific setup into the build, 2016-08-04), the test that
>> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
>> alongside LESS=FRSX, 2014-01-06) just above the removed test.
>> I.e. we don't care if the "LESS" and "LV" variables are set by
>> git-sh-setup anymore, no built-in uses them, we do care that pager.c
>> sets them, which we still test for.
>
> git_pager() might not be documented but I think it is useful for
> script authors and I wouldn't be surprised if someone out there is
> using it. The same goes for peel_committish(). It does not seem like a
> huge maintenance burden to keep and maybe document these two
> functions.

The git_pager() and peel_committish() seem to thoroughly be in the same
camp as the now-removed git-parse-remote.sh (see a89a2fbfccd
(parse-remote: remove this now-unused library, 2020-11-14)) and say its
get_remote_merge_branch(). I.e. we carried it for a while, but the
function was never publicly documented.

I think rather than document these it makes sense to just kick that
maintenance burden over to whoever decided they'd rely on undocumented
shellscript functions git was shipping.

In these cases they can rather easily use the documented GIT_PAGER
environment variable directly, and their own invocation of "git
rev-parse" for peel_committish().

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Phillip Wood]

On 06/09/2021 23:27, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Sep 06 2021, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
>>> Remove the git_pager() function last referenced by non-test code in
>>> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>>> We can also remove the test for this added in 995bc22d7f8 (pager:
>>> move
>>> pager-specific setup into the build, 2016-08-04), the test that
>>> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
>>> alongside LESS=FRSX, 2014-01-06) just above the removed test.
>>> I.e. we don't care if the "LESS" and "LV" variables are set by
>>> git-sh-setup anymore, no built-in uses them, we do care that pager.c
>>> sets them, which we still test for.
>>
>> git_pager() might not be documented but I think it is useful for
>> script authors and I wouldn't be surprised if someone out there is
>> using it. The same goes for peel_committish(). It does not seem like a
>> huge maintenance burden to keep and maybe document these two
>> functions.
> 
> The git_pager() and peel_committish() seem to thoroughly be in the same
> camp as the now-removed git-parse-remote.sh (see a89a2fbfccd
> (parse-remote: remove this now-unused library, 2020-11-14)) and say its
> get_remote_merge_branch(). I.e. we carried it for a while, but the
> function was never publicly documented.
> 
> I think rather than document these it makes sense to just kick that
> maintenance burden over to whoever decided they'd rely on undocumented
> shellscript functions git was shipping.
> 
> In these cases they can rather easily use the documented GIT_PAGER
> environment variable directly, 

No, they need to know to call 'git var GIT_PAGER' rather than using the 
environment variable directly to pick up core.pager and they should be 
checking whether stdout is a tty. That is why this function existed and 
we didn't just check the value of GIT_PAGER in our scripts

> and their own invocation of "git rev-parse" for peel_committish().

The reason the function exists is that you cannot just call 'git 
rev-parse $OID^{commit}' if $OID starts with :/

I'm not sure what the maintenance burden of keeping these functions is 
that makes it worth removing them

Best Wishes

Phillip

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Ævar Arnfjörð Bjarmason]

On Tue, Sep 07 2021, Phillip Wood wrote:

> On 06/09/2021 23:27, Ævar Arnfjörð Bjarmason wrote:
>> On Mon, Sep 06 2021, Phillip Wood wrote:
>> 
>>> Hi Ævar
>>>
>>> On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
>>>> Remove the git_pager() function last referenced by non-test code in
>>>> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>>>> We can also remove the test for this added in 995bc22d7f8 (pager:
>>>> move
>>>> pager-specific setup into the build, 2016-08-04), the test that
>>>> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
>>>> alongside LESS=FRSX, 2014-01-06) just above the removed test.
>>>> I.e. we don't care if the "LESS" and "LV" variables are set by
>>>> git-sh-setup anymore, no built-in uses them, we do care that pager.c
>>>> sets them, which we still test for.
>>>
>>> git_pager() might not be documented but I think it is useful for
>>> script authors and I wouldn't be surprised if someone out there is
>>> using it. The same goes for peel_committish(). It does not seem like a
>>> huge maintenance burden to keep and maybe document these two
>>> functions.
>> The git_pager() and peel_committish() seem to thoroughly be in the
>> same
>> camp as the now-removed git-parse-remote.sh (see a89a2fbfccd
>> (parse-remote: remove this now-unused library, 2020-11-14)) and say its
>> get_remote_merge_branch(). I.e. we carried it for a while, but the
>> function was never publicly documented.
>> I think rather than document these it makes sense to just kick that
>> maintenance burden over to whoever decided they'd rely on undocumented
>> shellscript functions git was shipping.
>> In these cases they can rather easily use the documented GIT_PAGER
>> environment variable directly, 
>
> No, they need to know to call 'git var GIT_PAGER' rather than using
> the environment variable directly to pick up core.pager[...]

Sorry, I should have said "...directly via git var GIT_PAGER". I also
see that we could improve some of the doc cross-referencing here,
i.e. "git help git") doesn't make this explicit or point to "git var",
but we cover this in "git help var" itself.

> [...]should be checking whether stdout is a tty. That is why this function
> existed and we didn't just check the value of GIT_PAGER in our scripts

For a hypothetical out-of-tree user is this really something anyone
strictly needs? It's just an optimization. If you don't do it you'll
just use your pager to pipe output to a non-tty.

>> and their own invocation of "git rev-parse" for peel_committish().
>
> The reason the function exists is that you cannot just call 'git
> rev-parse $OID^{commit}' if $OID starts with :/

Sure, but is the answer to that & the pager case above that we need to
support an always-undocumented function that someone could only have
found via source spelunking, as opposed to them maintaining that case,
or submitting a patch to "git rev-parse" to make their use-case easier?

> I'm not sure what the maintenance burden of keeping these functions is
> that makes it worth removing them

I was hoping that we could head towards just entirely removing
git-sh-setup in the near-ish future, but per the evolution of this
series it seems that we've got out-of-tree users for its *documented*
functions, so we can't simply do that.

I would like to be able to rip out the shellscript support for i18n
sooner than later, I think a way to get there would be to only emit
strings in the C locale from these remaining git-sh-setup functions, and
eventually move that script to live in contrib/ or something (and be
able to install it). I.e. make it a backwards-compatability-only
interface.

So yes, the maintenance burden of any two functions being removed here
is trivial and we could just keep them around.

I'm pursuing this because I'd really like to get clarity on where
exactly we're drawing the line with this git-sh-setup interface, since
we can anticipate a near-future where our own remaining user won't use
it anymore (or the 1-2 things they do can be moved to
git-filter-branch.sh or whatever).

The burden of that *is* non-trivial. I.e. us having to project gettext
to shellscript land, which in turn is something that's kept me from
improving git's i18n interface, i.e. provide some light alternative to
it that won't require libintl, as long as I've got to support that sort
of thing in shellscript land...

Do you have out-of-tree uses of git_pager(), or do we know about anyone
who does? I understand the argument that we've documented certain things
in git-sh-setup for years, but needing to support what's amounted to an
undocumented internal implementation detail seems like it's going too
far.

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> No, they need to know to call 'git var GIT_PAGER' rather than using
>> the environment variable directly to pick up core.pager[...]
>
> Sorry, I should have said "...directly via git var GIT_PAGER". I also
> see that we could improve some of the doc cross-referencing here,
> i.e. "git help git") doesn't make this explicit or point to "git var",
> but we cover this in "git help var" itself.
>
>> [...]should be checking whether stdout is a tty. That is why this function
>> existed and we didn't just check the value of GIT_PAGER in our scripts
>
> For a hypothetical out-of-tree user is this really something anyone
> strictly needs? It's just an optimization. If you don't do it you'll
> just use your pager to pipe output to a non-tty.

The question we should be asking when we advocate to remove things
is "is this really something we absolutely cannot live with?"

But answering your question, if an out-of-tree user wants to behave
just like Git, pretending that it would have been part of Git and
the only reason why it is not is because it weren't invented here,
yes, not forcing the end-user to pipe the tool's output to pager is
something they would want to have a handy way to mimic, I would
think.

Thanks.

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

[Ævar Arnfjörð Bjarmason]

On Tue, Sep 07 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> No, they need to know to call 'git var GIT_PAGER' rather than using
>>> the environment variable directly to pick up core.pager[...]
>>
>> Sorry, I should have said "...directly via git var GIT_PAGER". I also
>> see that we could improve some of the doc cross-referencing here,
>> i.e. "git help git") doesn't make this explicit or point to "git var",
>> but we cover this in "git help var" itself.
>>
>>> [...]should be checking whether stdout is a tty. That is why this function
>>> existed and we didn't just check the value of GIT_PAGER in our scripts
>>
>> For a hypothetical out-of-tree user is this really something anyone
>> strictly needs? It's just an optimization. If you don't do it you'll
>> just use your pager to pipe output to a non-tty.
>
> The question we should be asking when we advocate to remove things
> is "is this really something we absolutely cannot live with?"
>
> But answering your question, if an out-of-tree user wants to behave
> just like Git, pretending that it would have been part of Git and
> the only reason why it is not is because it weren't invented here,
> yes, not forcing the end-user to pipe the tool's output to pager is
> something they would want to have a handy way to mimic, I would
> think.

I've made my preferences clear, but can live with whatever criteria we
come up with.

I am having trouble squaring the desire to keep git_pager() with the
view you're describing, unless it's also an implicit endorsement of
reverting a89a2fbfccd (parse-remote: remove this now-unused library,
2020-11-14).

I'd obviously prefer to see git-parse-remote stay gone. But if we're
worried about removing once-documented "git-sh-*" libraries from under
users who peeked under the hood at some point to see & use functions
within them, I'd think bringing back "git-parse-remote" would be more
likely to help those users than having a git_pager().

And once we're rid of all our own use of these libraries but still want
to ship them forever for such users, I'd think we'd want to bring some
version of a revert of 49eb8d39c78 (Remove contrib/examples/*,
2018-03-25) back, i.e. just to make sure we don't break these going
forward, as once our own use of them is removed they'll be completely
untested in-tree.

Anyway, as noted in <87eea0n04u.fsf@evledraar.gmail.com> I was hoping to
take a small step towards finishing up removing the libintl dependency.
But after this discussion I think I'm back to mentally classifying that
as too tedious of a task to even try, so I wouldn't mind dropping this
series of cleanups if we've landed on a consensus of keeping
git-sh-setup bug-for-bug compatible going forward, and by extension
git-sh-i18n.

[PATCH v3 0/4] remove dead & internal-only shell code

[Ævar Arnfjörð Bjarmason]

Per the discussion on v1 and v2 I've reduced the size of this series
to only those changes that should be uncontroversial to remove, even
by the harshest standards of maintaining compatibility with
out-of-tree users of undocumented functions in git's shell libraries.

Now we remove is_zero_oid() from git-submodule.sh, but not the
sane_egrep() from git-sh-setup.sh, git_pager() also stays, so does
peel_committish() and clear_local_git_env().

I think we can keep 2/4 ad remove the "pull with rebase" message,
since as noted there the worst case is that someone will lose only
that part of their translation.

And finally, the 3/4 and 4/4 are the same cleanups of internal-only or
test-only OID-matching variables as before.

Ævar Arnfjörð Bjarmason (4):
  git-submodule: remove unused is_zero_oid() function
  git-sh-setup: remove unused "pull with rebase" message
  git-bisect: remove unused SHA-1 $x40 shell variable
  test-lib: remove unused $_x40 and $_z40 variables

 git-bisect.sh    | 2 --
 git-sh-setup.sh  | 6 ------
 git-submodule.sh | 5 -----
 t/test-lib.sh    | 6 ++----
 4 files changed, 2 insertions(+), 17 deletions(-)

Range-diff against v2:
1:  8eb1dfbff5d < -:  ----------- git-sh-setup: remove unused git_pager() function
2:  e7f3115797c ! 1:  62b3a5881c9 git-sh-setup: remove unused sane_egrep() function
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    git-sh-setup: remove unused sane_egrep() function
    +    git-submodule: remove unused is_zero_oid() function
     
         The is_zero_oid() function in git-submodule.sh has not been used since
         e83e3333b57 (submodule: port submodule subcommand 'summary' from shell
    -    to C, 2020-08-13), so we can remove it, and the sane_egrep() function,
    -    dead is_zero_oid() was the only function which still referenced it.
    +    to C, 2020-08-13), so we can remove it.
     
    -    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    +    This was the last user of the sane_egrep() function in
    +    git-sh-setup.sh. I'm not removing it in case some out-of-tree user
    +    relied on it. Per the discussion that can be found upthread of [1].
     
    - ## git-sh-setup.sh ##
    -@@ git-sh-setup.sh: sane_grep () {
    - 	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
    - }
    - 
    --sane_egrep () {
    --	GREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ "$@"
    --}
    --
    - is_bare_repository () {
    - 	git rev-parse --is-bare-repository
    - }
    +    1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/
    +
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## git-submodule.sh ##
     @@ git-submodule.sh: isnumber()
3:  d92e880fcfa < -:  ----------- git-sh-setup: move peel_committish() function to git-subtree.sh
4:  46c018aa860 < -:  ----------- git-sh-setup: clear_local_git_env() function to git-submodule.sh
5:  45c1369e958 ! 2:  db7223741ec git-sh-setup: remove unused "pull with rebase" message
    @@ Commit message
         git-pull.sh script, which was removed in 49eb8d39c78 (Remove
         contrib/examples/*, 2018-03-25).
     
    +    Even if some out-of-tree user copy/pasted the old git-pull.sh code,
    +    and relied on passing it a "pull with rebase" argument, we'll fall
    +    back on the "*" case here, they just won't get the "pull with rebase"
    +    part of their message translated.
    +
    +    I don't think it's likely that anyone out-of-tree relied on that, but
    +    I'm being conservative here per the discussion that can be found
    +    upthread of [1].
    +
    +    1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## git-sh-setup.sh ##
6:  bcae7884bb0 = 3:  cc2059f09f1 git-bisect: remove unused SHA-1 $x40 shell variable
7:  479e94f22f4 = 4:  206519c2d34 test-lib: remove unused $_x40 and $_z40 variables
-- 
2.33.0.984.gea2c3555113

[PATCH v3 1/4] git-submodule: remove unused is_zero_oid() function

[Ævar Arnfjörð Bjarmason]

The is_zero_oid() function in git-submodule.sh has not been used since
e83e3333b57 (submodule: port submodule subcommand 'summary' from shell
to C, 2020-08-13), so we can remove it.

This was the last user of the sane_egrep() function in
git-sh-setup.sh. I'm not removing it in case some out-of-tree user
relied on it. Per the discussion that can be found upthread of [1].

1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-submodule.sh | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index dbd2ec20503..aeb96c58243 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -63,11 +63,6 @@ isnumber()
 	n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 
-# Given a full hex object ID, is this the zero OID?
-is_zero_oid () {
-	echo "$1" | sane_egrep '^0+$' >/dev/null 2>&1
-}
-
 # Sanitize the local git environment for use within a submodule. We
 # can't simply use clear_local_git_env since we want to preserve some
 # of the settings from GIT_CONFIG_PARAMETERS.
-- 
2.33.0.984.gea2c3555113

[PATCH v3 2/4] git-sh-setup: remove unused "pull with rebase" message

[Ævar Arnfjörð Bjarmason]

Remove the "pull with rebase" message previously used by the
git-pull.sh script, which was removed in 49eb8d39c78 (Remove
contrib/examples/*, 2018-03-25).

Even if some out-of-tree user copy/pasted the old git-pull.sh code,
and relied on passing it a "pull with rebase" argument, we'll fall
back on the "*" case here, they just won't get the "pull with rebase"
part of their message translated.

I don't think it's likely that anyone out-of-tree relied on that, but
I'm being conservative here per the discussion that can be found
upthread of [1].

1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-sh-setup.sh | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 10d97641856..cee053cdc38 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -223,9 +223,6 @@ require_clean_work_tree () {
 		"rewrite branches")
 			gettextln "Cannot rewrite branches: You have unstaged changes." >&2
 			;;
-		"pull with rebase")
-			gettextln "Cannot pull with rebase: You have unstaged changes." >&2
-			;;
 		*)
 			eval_gettextln "Cannot \$action: You have unstaged changes." >&2
 			;;
@@ -242,9 +239,6 @@ require_clean_work_tree () {
 			rebase)
 				gettextln "Cannot rebase: Your index contains uncommitted changes." >&2
 				;;
-			"pull with rebase")
-				gettextln "Cannot pull with rebase: Your index contains uncommitted changes." >&2
-				;;
 			*)
 				eval_gettextln "Cannot \$action: Your index contains uncommitted changes." >&2
 				;;
-- 
2.33.0.984.gea2c3555113

[PATCH v3 3/4] git-bisect: remove unused SHA-1 $x40 shell variable

[Ævar Arnfjörð Bjarmason]

This variable was last used in code removed in
06f5608c14 (bisect--helper: `bisect_start` shell function partially in
C, 2019-01-02).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-bisect.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 6a7afaea8da..b59f3aaad43 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -34,8 +34,6 @@ Please use "git help bisect" to get the full man page.'
 OPTIONS_SPEC=
 . git-sh-setup
 
-_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"
 TERM_BAD=bad
 TERM_GOOD=good
 
-- 
2.33.0.984.gea2c3555113

[PATCH v3 4/4] test-lib: remove unused $_x40 and $_z40 variables

[Ævar Arnfjörð Bjarmason]

These two have fallen out of use with the SHA-256 migration.

The last use of $_x40 was removed in fc7e73d7ef (t4013: improve
diff-post-processor logic, 2020-08-21) and

The last use of $_z40 was removed in 7a868c51c2 (t5562: use $ZERO_OID,
2019-12-21), but it was then needlessly refactored to be hash-agnostic
in 192b517589 (t: use hash-specific lookup tables to define test
constants, 2020-02-22). We can just remove it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index fc1e5215198..a0b944e8feb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -534,7 +534,7 @@ SQ=\'
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x35 _x40 _z40 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
+export _x05 _x35 LF u200c EMPTY_TREE EMPTY_BLOB ZERO_OID OID_REGEX
 
 # Each test should start with something like this, after copyright notices:
 #
@@ -1423,10 +1423,9 @@ then
 fi
 
 # Convenience
-# A regexp to match 5, 35 and 40 hexdigits
+# A regexp to match 5 and 35 hexdigits
 _x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x35="$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
-_x40="$_x35$_x05"
 
 test_oid_init
 
@@ -1435,7 +1434,6 @@ OID_REGEX=$(echo $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 OIDPATH_REGEX=$(test_oid_to_path $ZERO_OID | sed -e 's/0/[0-9a-f]/g')
 EMPTY_TREE=$(test_oid empty_tree)
 EMPTY_BLOB=$(test_oid empty_blob)
-_z40=$ZERO_OID
 
 # Provide an implementation of the 'yes' utility; the upper bound
 # limit is there to help Windows that cannot stop this loop from
-- 
2.33.0.984.gea2c3555113

Re: [PATCH v3 1/4] git-submodule: remove unused is_zero_oid() function

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> The is_zero_oid() function in git-submodule.sh has not been used since
> e83e3333b57 (submodule: port submodule subcommand 'summary' from shell
> to C, 2020-08-13), so we can remove it.
>
> This was the last user of the sane_egrep() function in
> git-sh-setup.sh. I'm not removing it in case some out-of-tree user
> relied on it. Per the discussion that can be found upthread of [1].

I am OK with losing sane_egrep because it is not about a usefulness
we can give to our users based on our deep knowledge on how Git works;
it was rather based on our experience having to deal with silly choice
GNU grep made about coloring that made it unpleasant to use in scripts.
The users shouldn't have to depend on us for such a thing.

But I am OK either way---the whole topic is more or less "Meh" to
me.  It is hard to draw a line between a collection of pointless
churn and a generally useful clean-up, and I am having a hard time
deciding which side of the boundary this falls.