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
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 <redacted>
---
Documentation/git-sh-setup.txt | 7 ------
Documentation/git.txt | 4 ----
git-sh-setup.sh | 42 ----------------------------------
3 files changed, 53 deletions(-)
@@ -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
@@ -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
@@ -106,48 +106,6 @@ $LONG_USAGE")"esacfi-# 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="$*"-exportGIT_REFLOG_ACTION-fi-}- git_editor(){iftest-z"${GIT_EDITOR:+set}"then
@@ -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
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 <redacted>
---
git-sh-setup.sh | 16 ----------------
t/t7006-pager.sh | 13 -------------
2 files changed, 29 deletions(-)
@@ -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_unsetLESSLV&&-PAGER="env >pager-env.out; wc"&&-exportPAGER&&-PATH="$(git--exec-path):$PATH"&&-exportPATH&&-test_terminalsh-c". git-sh-setup && git_pager"-)&&-grep^LESS=pager-env.out&&-grep^LV=pager-env.out-'- test_expect_successTTY'some commands do not use a pager''rm-fpaginated.out&&test_terminalgitrev-listHEAD&&
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 <redacted>
---
git-sh-setup.sh | 4 ----
git-submodule.sh | 5 -----
2 files changed, 9 deletions(-)
@@ -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/null2>&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.
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 <redacted>
---
Documentation/git-sh-setup.txt | 6 ------
git-sh-setup.sh | 8 --------
2 files changed, 14 deletions(-)
@@ -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.
@@ -122,14 +122,6 @@ cd_to_toplevel () {}}-require_work_tree_exists(){-iftest"z$(gitrev-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"$(gitrev-parse--is-inside-work-tree2>/dev/null)"=true||{program_name=$0
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 <redacted>
---
Documentation/git-sh-setup.txt | 6 ------
git-sh-setup.sh | 12 ------------
mergetools/p4merge | 12 ++++++++++++
3 files changed, 12 insertions(+), 18 deletions(-)
@@ -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
@@ -238,18 +238,6 @@ clear_local_git_env() {unset$(gitrev-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"|gitapply--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 commandscase$(uname-s)in
@@ -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
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 <redacted>
---
contrib/subtree/git-subtree.sh | 12 ++++++++++++
git-sh-setup.sh | 12 ------------
2 files changed, 12 insertions(+), 12 deletions(-)
@@ -818,6 +818,18 @@ cmd_add_repository () {cmd_add_commitFETCH_HEAD}+peel_committish(){+case"$1"in+:/*)+peeltmp=$(gitrev-parse--verify"$1")&&+gitrev-parse--verify"${peeltmp}^0"+;;+*)+gitrev-parse--verify"${1}^0"+;;+esac+}+# Usage: cmd_add_commit REV cmd_add_commit(){# The rev has already been validated by cmd_add(), we just
@@ -294,15 +294,3 @@ if test -z "$NONGIT_OK"thengit_dir_initfi--peel_committish(){-case"$1"in-:/*)-peeltmp=$(gitrev-parse--verify"$1")&&-gitrev-parse--verify"${peeltmp}^0"-;;-*)-gitrev-parse--verify"${1}^0"-;;-esac-}
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 <redacted>
---
git-bisect.sh | 2 --
1 file changed, 2 deletions(-)
@@ -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=badTERM_GOOD=good
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 <redacted>
---
t/test-lib.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
@@ -534,7 +534,7 @@ SQ=\'# when case-folding filenamesu200c=$(printf'\342\200\214')-export_x05_x35_x40_z40LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX+export_x05_x35LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX# Each test should start with something like this, after copyright notices:#
@@ -1422,10 +1422,9 @@ thenfi# 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_oidempty_tree)EMPTY_BLOB=$(test_oidempty_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
[...]
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
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 <redacted>
---
git-sh-setup.sh | 4 ----
git-submodule.sh | 5 -----
2 files changed, 9 deletions(-)
@@ -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/null2>&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.
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 <redacted>
---
contrib/subtree/git-subtree.sh | 12 ++++++++++++
git-sh-setup.sh | 12 ------------
2 files changed, 12 insertions(+), 12 deletions(-)
@@ -818,6 +818,18 @@ cmd_add_repository () {cmd_add_commitFETCH_HEAD}+peel_committish(){+case"$1"in+:/*)+peeltmp=$(gitrev-parse--verify"$1")&&+gitrev-parse--verify"${peeltmp}^0"+;;+*)+gitrev-parse--verify"${1}^0"+;;+esac+}+# Usage: cmd_add_commit REV cmd_add_commit(){# The rev has already been validated by cmd_add(), we just
@@ -365,15 +365,3 @@ if test -z "$NONGIT_OK"thengit_dir_initfi--peel_committish(){-case"$1"in-:/*)-peeltmp=$(gitrev-parse--verify"$1")&&-gitrev-parse--verify"${peeltmp}^0"-;;-*)-gitrev-parse--verify"${1}^0"-;;-esac-}
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 [off-list ref]
## 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
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 <redacted>
---
git-sh-setup.sh | 16 ----------------
t/t7006-pager.sh | 13 -------------
2 files changed, 29 deletions(-)
@@ -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_unsetLESSLV&&-PAGER="env >pager-env.out; wc"&&-exportPAGER&&-PATH="$(git--exec-path):$PATH"&&-exportPATH&&-test_terminalsh-c". git-sh-setup && git_pager"-)&&-grep^LESS=pager-env.out&&-grep^LV=pager-env.out-'- test_expect_successTTY'some commands do not use a pager''rm-fpaginated.out&&test_terminalgitrev-listHEAD&&
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 <redacted>
---
git-sh-setup.sh | 6 ------
1 file changed, 6 deletions(-)
@@ -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;;
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 <redacted>
---
git-sh-setup.sh | 7 -------
git-submodule.sh | 2 +-
2 files changed, 1 insertion(+), 8 deletions(-)
@@ -290,13 +290,6 @@ get_author_ident_from_commit () {parse_ident_from_commitauthorAUTHOR}-# 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$(gitrev-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(){
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 <redacted>
---
git-bisect.sh | 2 --
1 file changed, 2 deletions(-)
@@ -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=badTERM_GOOD=good
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 <redacted>
---
t/test-lib.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
@@ -534,7 +534,7 @@ SQ=\'# when case-folding filenamesu200c=$(printf'\342\200\214')-export_x05_x35_x40_z40LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX+export_x05_x35LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX# Each test should start with something like this, after copyright notices:#
@@ -1422,10 +1422,9 @@ thenfi# 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_oidempty_tree)EMPTY_BLOB=$(test_oidempty_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
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
@@ -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_unsetLESSLV&&-PAGER="env >pager-env.out; wc"&&-exportPAGER&&-PATH="$(git--exec-path):$PATH"&&-exportPATH&&-test_terminalsh-c". git-sh-setup && git_pager"-)&&-grep^LESS=pager-env.out&&-grep^LV=pager-env.out-'-test_expect_successTTY'some commands do not use a pager''rm-fpaginated.out&&test_terminalgitrev-listHEAD&&
Hi Ævar
On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
quoted
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().
On 06/09/2021 23:27, Ævar Arnfjörð Bjarmason wrote:
On Mon, Sep 06 2021, Phillip Wood wrote:
quoted
Hi Ævar
On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
quoted
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
On 06/09/2021 23:27, Ævar Arnfjörð Bjarmason wrote:
quoted
On Mon, Sep 06 2021, Phillip Wood wrote:
quoted
Hi Ævar
On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
quoted
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.
quoted
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.
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 <redacted>
---
git-sh-setup.sh | 6 ------
1 file changed, 6 deletions(-)
@@ -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;;
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 [off-list ref]
## 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 [off-list ref]
+ 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 [off-list ref]
## 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 [off-list ref]
## 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
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 <redacted>
---
git-submodule.sh | 5 -----
1 file changed, 5 deletions(-)
@@ -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/null2>&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.
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 <redacted>
---
git-bisect.sh | 2 --
1 file changed, 2 deletions(-)
@@ -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=badTERM_GOOD=good
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 <redacted>
---
t/test-lib.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
@@ -534,7 +534,7 @@ SQ=\'# when case-folding filenamesu200c=$(printf'\342\200\214')-export_x05_x35_x40_z40LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX+export_x05_x35LFu200cEMPTY_TREEEMPTY_BLOBZERO_OIDOID_REGEX# Each test should start with something like this, after copyright notices:#
@@ -1423,10 +1423,9 @@ thenfi# 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_oidempty_tree)EMPTY_BLOB=$(test_oidempty_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
From: Junio C Hamano <hidden> Date: 2021-09-13 03:28:18
Ævar Arnfjörð Bjarmason [off-list ref] 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.