[PATCH 0/6] Makefile: be less eager to re-build, *.sh code cleanups

[PATCH 0/6] Makefile: be less eager to re-build, *.sh code cleanups

[Ævar Arnfjörð Bjarmason]

This removes the dependency of our built *.sh on $(GIT_VERSION), which
as noted in 2/6 looks to have been the intention back in 2012, but
didn't happen then.

The rest of the series removes more dead code related to our
$(SCRIPT_DEFINES), some of it it not used or needed for 5-15ish years.

Ævar Arnfjörð Bjarmason (6):
  Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)
  Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)
  Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)
  git-instaweb: unconditionally assume that gitweb is mod_perl capable
  git-sh-setup: remove unused sane_egrep() function
  git-sh-setup: remove "sane_grep", it's not needed anymore

 Makefile                            | 26 ++++++++++----------------
 config.mak.uname                    |  1 -
 configure.ac                        |  8 --------
 contrib/buildsystems/CMakeLists.txt |  1 -
 git-filter-branch.sh                |  2 +-
 git-instaweb.sh                     |  9 +++------
 git-sh-setup.sh                     |  8 --------
 7 files changed, 14 insertions(+), 41 deletions(-)

-- 
2.33.1.1494.g88b39a443e1

[PATCH 1/6] Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)

[Ævar Arnfjörð Bjarmason]

When "GIT-SCRIPT-DEFINES" was added in e4dd89ab984 (Makefile: update
scripts when build-time parameters change, 2012-06-20) the rules for
generating the scripts themselves were moved further away from the
"cmd_munge_script" added in 46bac904581 (Do not install shell
libraries executable, 2010-01-31).

Let's move these around so that the variables and defines needed by
given targets immediately precede them. This is not needed for any
subsequent changes to work, but makes the code consistent with how
GIT-PERL-DEFINES is structured.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 381bed2c1d2..f52402b24d8 100644
--- a/Makefile
+++ b/Makefile
@@ -2256,6 +2256,13 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
 	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
 	$(perllibdir_SQ)
+GIT-SCRIPT-DEFINES: FORCE
+	@FLAGS='$(SCRIPT_DEFINES)'; \
+	    if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
+		echo >&2 "    * new script parameters"; \
+		echo "$$FLAGS" >$@; \
+            fi
+
 define cmd_munge_script
 sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
@@ -2271,14 +2278,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     $@.sh >$@+
 endef
 
-GIT-SCRIPT-DEFINES: FORCE
-	@FLAGS='$(SCRIPT_DEFINES)'; \
-	    if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
-		echo >&2 "    * new script parameters"; \
-		echo "$$FLAGS" >$@; \
-            fi
-
-
 $(SCRIPT_SH_GEN) : % : %.sh GIT-SCRIPT-DEFINES
 	$(QUIET_GEN)$(cmd_munge_script) && \
 	chmod +x $@+ && \
-- 
2.33.1.1494.g88b39a443e1

[PATCH 2/6] Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)

[Ævar Arnfjörð Bjarmason]

Remove the $(GIT_VERSION) from $(SCRIPT_DEFINES). Now every time HEAD
changes in a development copy we don't need to re-build the scripts
and script libraries.

This has not been needed since 2b9391bc675 (Makefile: do not replace
@@GIT_VERSION@@ in shell scripts, 2012-06-20). On my setup this
changes the re-making of 44 targets in a development copy where moved
HEAD to 27.

The $(GIT_VERSION) was seemingly left here by mistake or omission. We
didn't need it since 2b9391bc675, but in the later
e4dd89ab984 (Makefile: update scripts when build-time parameters
change, 2012-06-20) it was added to SCRIPT_DEFINES.

The two were part of the same series of patches, and given the summary
in [1] and [2] it looks like this was probably a case of some earlier
version of a later patch being combined with an updated earlier patch.

1. https://lore.kernel.org/git/20120619232231.GA6328@sigill.intra.peff.net/
2. https://lore.kernel.org/git/20120619232453.GB6496@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f52402b24d8..fdce392929b 100644
--- a/Makefile
+++ b/Makefile
@@ -2252,7 +2252,7 @@ command-list.h: $(wildcard Documentation/git*.txt)
 hook-list.h: generate-hooklist.sh Documentation/githooks.txt
 	$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@
 
-SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
+SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
 	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
 	$(perllibdir_SQ)
-- 
2.33.1.1494.g88b39a443e1

[PATCH 3/6] Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)

[Ævar Arnfjörð Bjarmason]

Stop including $(NO_CURL) in $(SCRIPT_DEFINES). The "@NO_CURL@"
replacement added in 6c5c62f3401 (Print an error if cloning a http
repo and NO_CURL is set, 2006-02-15) has not been referenced by
anything in-tree since 49eb8d39c78 (Remove contrib/examples/*,
2018-03-25).

That commit removed the reference from contrib/examples/*, but this
@@NO_CURL@@ hasn't been used since git-pull.sh was the primary entry
point for "git pull".

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

diff --git a/Makefile b/Makefile
index fdce392929b..50eba522f4e 100644
--- a/Makefile
+++ b/Makefile
@@ -2253,7 +2253,7 @@ hook-list.h: generate-hooklist.sh Documentation/githooks.txt
 	$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
-	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
+	$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
 	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
 	$(perllibdir_SQ)
 GIT-SCRIPT-DEFINES: FORCE
@@ -2268,7 +2268,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
     -e 's|@@DIFF@@|$(DIFF_SQ)|' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
-    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
     -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
     -e $(BROKEN_PATH_FIX) \
     -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
-- 
2.33.1.1494.g88b39a443e1

[PATCH 4/6] git-instaweb: unconditionally assume that gitweb is mod_perl capable

[Ævar Arnfjörð Bjarmason]

Remove a check for whether mod_perl is a supported mode of gitweb.cgi
added in a51d37c1df6 (Add git-instaweb, instantly browse the working
repo with gitweb, 2006-07-01).

The reason for the check was to support users who had a newer version
of git and an older version of gitweb, it was then subsequently
adjusted for changes in the script in f0e588dffc1 (git-instaweb: fix
mod_perl detection for apache2, 2009-08-08).

It's a fair bet that nobody's running a git from 2021 and gitweb from
pre-2007 anymore, so we can unconditionally assume that this will be
supported by gitweb.cgi.

This allows a subsequent commit to remove the sane_grep() wrapper,
this change is split up from that since this is the only case where
the "grep" invocation could be removed entirely.

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

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 7c55229773e..54094fbe8de 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -380,10 +380,7 @@ TypesConfig "$fqgitdir/mime.types"
 DirectoryIndex gitweb.cgi
 EOF
 
-	# check to see if Dennis Stosberg's mod_perl compatibility patch
-	# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied
-	if test -f "$module_path/mod_perl.so" &&
-	   sane_grep 'MOD_PERL' "$root/gitweb.cgi" >/dev/null
+	if test -f "$module_path/mod_perl.so"
 	then
 		# favor mod_perl if available
 		cat >> "$conf" <<EOF
-- 
2.33.1.1494.g88b39a443e1

[PATCH 5/6] 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.

Unlike some other functions in git-sh-setup.sh, this function has not
been documented in git-sh-setup(1), so per [1] it should be OK to
remove it. I'm still unclear about the future of some of the other
functions[2], but any questions in that area should not apply here.

1. https://lore.kernel.org/git/xmqqr1dtgnn8.fsf@gitster.g/
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 | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index cee053cdc38..758520f5a61 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -177,10 +177,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
 }
-- 
2.33.1.1494.g88b39a443e1

[PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Ævar Arnfjörð Bjarmason]

Remove the sane_grep() shell function in git-sh-setup. The two reasons
for why it existed don't apply anymore:

1. It was added due to GNU grep supporting GREP_OPTIONS. See
   e1622bfcbad (Protect scripted Porcelains from GREP_OPTIONS insanity,
   2009-11-23).

   Newer versions of GNU grep ignore that, but even on older versions
   its existence won't matter, none of these sane_grep() uses care
   about grep's output, they're merely using it to check if a string
   exists in a file or stream. We also don't care about the "LC_ALL=C"
   that "sane_grep" was using, these greps for fixed or ASCII strings
   will behave the same under any locale.

2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass "-a" if
   grep accepts it, 2016-03-08) isn't needed either, none of these grep
   uses deal with binary data.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                            | 6 +-----
 config.mak.uname                    | 1 -
 configure.ac                        | 8 --------
 contrib/buildsystems/CMakeLists.txt | 1 -
 git-filter-branch.sh                | 2 +-
 git-instaweb.sh                     | 4 ++--
 git-sh-setup.sh                     | 4 ----
 7 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/Makefile b/Makefile
index 50eba522f4e..01585bfebdc 100644
--- a/Makefile
+++ b/Makefile
@@ -305,9 +305,6 @@ all::
 #
 # Define NO_TCLTK if you do not want Tcl/Tk GUI.
 #
-# Define SANE_TEXT_GREP to "-a" if you use recent versions of GNU grep
-# and egrep that are pickier when their input contains non-ASCII data.
-#
 # The TCL_PATH variable governs the location of the Tcl interpreter
 # used to optimize git-gui for your system.  Only used if NO_TCLTK
 # is not set.  Defaults to the bare 'tclsh'.
@@ -2254,7 +2251,7 @@ hook-list.h: generate-hooklist.sh Documentation/githooks.txt
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
 	$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
-	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
+	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
 	$(perllibdir_SQ)
 GIT-SCRIPT-DEFINES: FORCE
 	@FLAGS='$(SCRIPT_DEFINES)'; \
@@ -2272,7 +2269,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e $(BROKEN_PATH_FIX) \
     -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
     -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
-    -e 's|@@SANE_TEXT_GREP@@|$(SANE_TEXT_GREP)|g' \
     -e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
     $@.sh >$@+
 endef
diff --git a/config.mak.uname b/config.mak.uname
index 3236a4918a3..d0701f9beb0 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -58,7 +58,6 @@ ifeq ($(uname_S),Linux)
 	# -lrt is needed for clock_gettime on glibc <= 2.16
 	NEEDS_LIBRT = YesPlease
 	HAVE_GETDELIM = YesPlease
-	SANE_TEXT_GREP=-a
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	BASIC_CFLAGS += -DHAVE_SYSINFO
 	PROCFS_EXECUTABLE_PATH = /proc/self/exe
diff --git a/configure.ac b/configure.ac
index 031e8d3fee8..5ee25ec95c8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -507,14 +507,6 @@ if test -n "$ASCIIDOC"; then
 	esac
 fi
 
-if grep -a ascii configure.ac >/dev/null; then
-  AC_MSG_RESULT([Using 'grep -a' for sane_grep])
-  SANE_TEXT_GREP=-a
-else
-  SANE_TEXT_GREP=
-fi
-GIT_CONF_SUBST([SANE_TEXT_GREP])
-
 ## Checks for libraries.
 AC_MSG_NOTICE([CHECKS for libraries])
 #
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index fd1399c440f..86b46114464 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -781,7 +781,6 @@ foreach(script ${git_shell_scripts})
 	string(REPLACE "@@USE_GETTEXT_SCHEME@@" "" content "${content}")
 	string(REPLACE "# @@BROKEN_PATH_FIX@@" "" content "${content}")
 	string(REPLACE "@@PERL@@" "${PERL_PATH}" content "${content}")
-	string(REPLACE "@@SANE_TEXT_GREP@@" "-a" content "${content}")
 	string(REPLACE "@@PAGER_ENV@@" "LESS=FRX LV=-c" content "${content}")
 	file(WRITE ${CMAKE_BINARY_DIR}/${script} ${content})
 endforeach()
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index cb893728136..3a51d4507c7 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -579,7 +579,7 @@ if [ "$filter_tag_name" ]; then
 				git hash-object -t tag -w --stdin) ||
 				die "Could not create new tag object for $ref"
 			if git cat-file tag "$ref" | \
-			   sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
+			   grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
 			then
 				warn "gpg signature stripped from tag object $sha1t"
 			fi
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 54094fbe8de..4349566c891 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -49,7 +49,7 @@ resolve_full_httpd () {
 	*apache2*|*lighttpd*|*httpd*)
 		# yes, *httpd* covers *lighttpd* above, but it is there for clarity
 		# ensure that the apache2/lighttpd command ends with "-f"
-		if ! echo "$httpd" | sane_grep -- '-f *$' >/dev/null 2>&1
+		if ! echo "$httpd" | grep -- '-f *$' >/dev/null 2>&1
 		then
 			httpd="$httpd -f"
 		fi
@@ -399,7 +399,7 @@ EOF
 		# plain-old CGI
 		resolve_full_httpd
 		list_mods=$(echo "$full_httpd" | sed 's/-f$/-l/')
-		$list_mods | sane_grep 'mod_cgi\.c' >/dev/null 2>&1 || \
+		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
 		if test -f "$module_path/mod_cgi.so"
 		then
 			echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 758520f5a61..1eb79458b48 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -173,10 +173,6 @@ git_pager() {
 	eval "$GIT_PAGER" '"$@"'
 }
 
-sane_grep () {
-	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
-}
-
 is_bare_repository () {
 	git rev-parse --is-bare-repository
 }
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Junio C Hamano]

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

> 2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass "-a" if
>    grep accepts it, 2016-03-08) isn't needed either, none of these grep
>    uses deal with binary data.

For all other users, I think the above applies well, but the one in
filter-branch deals with end-user contents, so we cannot say
"contents of the tag cannot be binary".

Not that I care about filter-branch too deeply ;-)

Re: [PATCH 5/6] git-sh-setup: remove unused sane_egrep() 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, and the sane_egrep() function,
> dead is_zero_oid() was the only function which still referenced it.
>
> Unlike some other functions in git-sh-setup.sh, this function has not
> been documented in git-sh-setup(1), so per [1] it should be OK to
> remove it. I'm still unclear about the future of some of the other
> functions[2], but any questions in that area should not apply here.
>
> 1. https://lore.kernel.org/git/xmqqr1dtgnn8.fsf@gitster.g/
> 1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/

Don't force readers to go outside of "git log" stream they are
reading.  Instead, summarize the argument that you base your
decision on for readers of your proposed commit log message.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  git-sh-setup.sh | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index cee053cdc38..758520f5a61 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -177,10 +177,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
>  }

Re: [PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

Hi Junio,

On Thu, 21 Oct 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
> > 2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass "-a" if
> >    grep accepts it, 2016-03-08) isn't needed either, none of these grep
> >    uses deal with binary data.
>
> For all other users, I think the above applies well, but the one in
> filter-branch deals with end-user contents, so we cannot say
> "contents of the tag cannot be binary".
>
> Not that I care about filter-branch too deeply ;-)

You probably meant that you do not care about filter-branch too deeply?
;-)

FWIW neither do I. Recently, I had the need for some large-scale
rewriting, and I used this as an excuse to learn me some
`git-repo-filter`. I was pleasantly surprised how easy it is to use, and
how fast it works.

So yeah, I don't care about `git filter-branch` too deeply, either ;-)

Ciao,
Dscho

Re: [PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> For all other users, I think the above applies well, but the one in
>> filter-branch deals with end-user contents, so we cannot say
>> "contents of the tag cannot be binary".
>>
>> Not that I care about filter-branch too deeply ;-)
>
> You probably meant that you do not care about filter-branch too deeply?
> ;-)

I don't get it.  Not only I meant, I said so, didn't I?

Re: [PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Johannes Schindelin]

Hi Junio,

On Sun, 24 Oct 2021, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> For all other users, I think the above applies well, but the one in
> >> filter-branch deals with end-user contents, so we cannot say
> >> "contents of the tag cannot be binary".
> >>
> >> Not that I care about filter-branch too deeply ;-)
> >
> > You probably meant that you do not care about filter-branch too deeply?
> > ;-)
>
> I don't get it.  Not only I meant, I said so, didn't I?

Hah, I autocorrected your "Not" to "Note" in my mind ;-)

Ciao,
Dscho

Re: [PATCH 6/6] git-sh-setup: remove "sane_grep", it's not needed anymore

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Sun, 24 Oct 2021, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> >> For all other users, I think the above applies well, but the one in
>> >> filter-branch deals with end-user contents, so we cannot say
>> >> "contents of the tag cannot be binary".
>> >>
>> >> Not that I care about filter-branch too deeply ;-)
>> >
>> > You probably meant that you do not care about filter-branch too deeply?
>> > ;-)
>>
>> I don't get it.  Not only I meant, I said so, didn't I?
>
> Hah, I autocorrected your "Not" to "Note" in my mind ;-)

Ahh, a single byte makes quite a big difference ;-)

Re: [PATCH 2/6] Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)

[Jeff King]

On Thu, Oct 21, 2021 at 09:57:56PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Remove the $(GIT_VERSION) from $(SCRIPT_DEFINES). Now every time HEAD
> changes in a development copy we don't need to re-build the scripts
> and script libraries.
> 
> This has not been needed since 2b9391bc675 (Makefile: do not replace
> @@GIT_VERSION@@ in shell scripts, 2012-06-20). On my setup this
> changes the re-making of 44 targets in a development copy where moved
> HEAD to 27.
> 
> The $(GIT_VERSION) was seemingly left here by mistake or omission. We
> didn't need it since 2b9391bc675, but in the later
> e4dd89ab984 (Makefile: update scripts when build-time parameters
> change, 2012-06-20) it was added to SCRIPT_DEFINES.
> 
> The two were part of the same series of patches, and given the summary
> in [1] and [2] it looks like this was probably a case of some earlier
> version of a later patch being combined with an updated earlier patch.
>
Yeah, that seems likely. No such version hit the list, but I probably
wrote the SCRIPT_DEFINES one first, and then realized GIT_VERSION could
be dropped, but didn't update the other.

At any rate, your patch looks like the correct thing to do.

-Peff

Re: [PATCH 0/6] Makefile: be less eager to re-build, *.sh code cleanups

[Jeff King]

On Thu, Oct 21, 2021 at 09:57:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This removes the dependency of our built *.sh on $(GIT_VERSION), which
> as noted in 2/6 looks to have been the intention back in 2012, but
> didn't happen then.
> 
> The rest of the series removes more dead code related to our
> $(SCRIPT_DEFINES), some of it it not used or needed for 5-15ish years.
> 
> Ævar Arnfjörð Bjarmason (6):
>   Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)
>   Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)
>   Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)
>   git-instaweb: unconditionally assume that gitweb is mod_perl capable
>   git-sh-setup: remove unused sane_egrep() function
>   git-sh-setup: remove "sane_grep", it's not needed anymore

The first five all look obviously correct to me. The 6th also looks
fine, but I don't have a strong opinion on the filter-branch thing
either way.

-Peff