{"thread":{"id":"56751","subject":"[PATCH 1/6] Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)","startedAt":"2021-10-21T19:58:05Z","lastAt":"2021-10-25T19:09:00Z","messageCount":15,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"439273","messageId":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":null,"subject":"[PATCH 0/6] Makefile: be less eager to re-build, *.sh code cleanups","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:57:54Z","receivedAt":"2021-10-21T19:58:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This removes the dependency of our built *.sh on $(GIT_VERSION), which\nas noted in 2/6 looks to have been the intention back in 2012, but\ndidn't happen then.\n\nThe rest of the series removes more dead code related to our\n$(SCRIPT_DEFINES), some of it it not used or needed for 5-15ish years.\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)\n Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)\n Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)\n git-instaweb: unconditionally assume that gitweb is mod_perl capable\n git-sh-setup: remove unused sane_egrep() function\n git-sh-setup: remove \"sane_grep\", it's not needed anymore\n\n Makefile | 26 ++++++++++----------------\n config.mak.uname | 1 -\n configure.ac | 8 --------\n contrib/buildsystems/CMakeLists.txt | 1 -\n git-filter-branch.sh | 2 +-\n git-instaweb.sh | 9 +++------\n git-sh-setup.sh | 8 --------\n 7 files changed, 14 insertions(+), 41 deletions(-)\n\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439271","messageId":"patch-1.6-2a7b5de44a8-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"[PATCH 1/6] Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:57:55Z","receivedAt":"2021-10-21T19:58:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When \"GIT-SCRIPT-DEFINES\" was added in e4dd89ab984 (Makefile: update\nscripts when build-time parameters change, 2012-06-20) the rules for\ngenerating the scripts themselves were moved further away from the\n\"cmd_munge_script\" added in 46bac904581 (Do not install shell\nlibraries executable, 2010-01-31).\n\nLet's move these around so that the variables and defines needed by\ngiven targets immediately precede them. This is not needed for any\nsubsequent changes to work, but makes the code consistent with how\nGIT-PERL-DEFINES is structured.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 15 +++++++--------\n 1 file changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 381bed2c1d2..f52402b24d8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2256,6 +2256,13 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\n \t$(perllibdir_SQ)\n+GIT-SCRIPT-DEFINES: FORCE\n+\t@FLAGS='$(SCRIPT_DEFINES)'; \\\n+\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n+\t\techo >&2 \" * new script parameters\"; \\\n+\t\techo \"$$FLAGS\" >$@; \\\n+ fi\n+\n define cmd_munge_script\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n@@ -2271,14 +2278,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n $@.sh >$@+\n endef\n \n-GIT-SCRIPT-DEFINES: FORCE\n-\t@FLAGS='$(SCRIPT_DEFINES)'; \\\n-\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new script parameters\"; \\\n-\t\techo \"$$FLAGS\" >$@; \\\n- fi\n-\n-\n $(SCRIPT_SH_GEN) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439272","messageId":"patch-2.6-7cc3a1d7205-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"[PATCH 2/6] Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:57:56Z","receivedAt":"2021-10-21T19:58:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the $(GIT_VERSION) from $(SCRIPT_DEFINES). Now every time HEAD\nchanges in a development copy we don't need to re-build the scripts\nand script libraries.\n\nThis has not been needed since 2b9391bc675 (Makefile: do not replace\n@@GIT_VERSION@@ in shell scripts, 2012-06-20). On my setup this\nchanges the re-making of 44 targets in a development copy where moved\nHEAD to 27.\n\nThe $(GIT_VERSION) was seemingly left here by mistake or omission. We\ndidn't need it since 2b9391bc675, but in the later\ne4dd89ab984 (Makefile: update scripts when build-time parameters\nchange, 2012-06-20) it was added to SCRIPT_DEFINES.\n\nThe two were part of the same series of patches, and given the summary\nin [1] and [2] it looks like this was probably a case of some earlier\nversion of a later patch being combined with an updated earlier patch.\n\n1. https://lore.kernel.org/git/20120619232231.GA6328@sigill.intra.peff.net/\n2. https://lore.kernel.org/git/20120619232453.GB6496@sigill.intra.peff.net/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex f52402b24d8..fdce392929b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2252,7 +2252,7 @@ command-list.h: $(wildcard Documentation/git*.txt)\n hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n \n-SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n+SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\n \t$(perllibdir_SQ)\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439274","messageId":"patch-3.6-04c45e191a9-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"[PATCH 3/6] Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:57:57Z","receivedAt":"2021-10-21T19:58:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Stop including $(NO_CURL) in $(SCRIPT_DEFINES). The \"@NO_CURL@\"\nreplacement added in 6c5c62f3401 (Print an error if cloning a http\nrepo and NO_CURL is set, 2006-02-15) has not been referenced by\nanything in-tree since 49eb8d39c78 (Remove contrib/examples/*,\n2018-03-25).\n\nThat commit removed the reference from contrib/examples/*, but this\n@@NO_CURL@@ hasn't been used since git-pull.sh was the primary entry\npoint for \"git pull\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex fdce392929b..50eba522f4e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2253,7 +2253,7 @@ hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\\\n-\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n+\t$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\n \t$(perllibdir_SQ)\n GIT-SCRIPT-DEFINES: FORCE\n@@ -2268,7 +2268,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n- -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n -e $(BROKEN_PATH_FIX) \\\n -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439275","messageId":"patch-4.6-4feeeb12aca-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"[PATCH 4/6] git-instaweb: unconditionally assume that gitweb is mod_perl capable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:57:58Z","receivedAt":"2021-10-21T19:58:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove a check for whether mod_perl is a supported mode of gitweb.cgi\nadded in a51d37c1df6 (Add git-instaweb, instantly browse the working\nrepo with gitweb, 2006-07-01).\n\nThe reason for the check was to support users who had a newer version\nof git and an older version of gitweb, it was then subsequently\nadjusted for changes in the script in f0e588dffc1 (git-instaweb: fix\nmod_perl detection for apache2, 2009-08-08).\n\nIt's a fair bet that nobody's running a git from 2021 and gitweb from\npre-2007 anymore, so we can unconditionally assume that this will be\nsupported by gitweb.cgi.\n\nThis allows a subsequent commit to remove the sane_grep() wrapper,\nthis change is split up from that since this is the only case where\nthe \"grep\" invocation could be removed entirely.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-instaweb.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 7c55229773e..54094fbe8de 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -380,10 +380,7 @@ TypesConfig \"$fqgitdir/mime.types\"\n DirectoryIndex gitweb.cgi\n EOF\n \n-\t# check to see if Dennis Stosberg's mod_perl compatibility patch\n-\t# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied\n-\tif test -f \"$module_path/mod_perl.so\" &&\n-\t sane_grep 'MOD_PERL' \"$root/gitweb.cgi\" >/dev/null\n+\tif test -f \"$module_path/mod_perl.so\"\n \tthen\n \t\t# favor mod_perl if available\n \t\tcat >> \"$conf\" <\n---\n git-sh-setup.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex cee053cdc38..758520f5a61 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -177,10 +177,6 @@ sane_grep () {\n \tGREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ \"$@\"\n }\n \n-sane_egrep () {\n-\tGREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ \"$@\"\n-}\n-\n is_bare_repository () {\n \tgit rev-parse --is-bare-repository\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439277","messageId":"patch-6.6-556fa96dde7-20211021T195538Z-avarab@gmail.com","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"[PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T19:58:00Z","receivedAt":"2021-10-21T19:58:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the sane_grep() shell function in git-sh-setup. The two reasons\nfor why it existed don't apply anymore:\n\n1. It was added due to GNU grep supporting GREP_OPTIONS. See\n e1622bfcbad (Protect scripted Porcelains from GREP_OPTIONS insanity,\n 2009-11-23).\n\n Newer versions of GNU grep ignore that, but even on older versions\n its existence won't matter, none of these sane_grep() uses care\n about grep's output, they're merely using it to check if a string\n exists in a file or stream. We also don't care about the \"LC_ALL=C\"\n that \"sane_grep\" was using, these greps for fixed or ASCII strings\n will behave the same under any locale.\n\n2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass \"-a\" if\n grep accepts it, 2016-03-08) isn't needed either, none of these grep\n uses deal with binary data.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 6 +-----\n config.mak.uname | 1 -\n configure.ac | 8 --------\n contrib/buildsystems/CMakeLists.txt | 1 -\n git-filter-branch.sh | 2 +-\n git-instaweb.sh | 4 ++--\n git-sh-setup.sh | 4 ----\n 7 files changed, 4 insertions(+), 22 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 50eba522f4e..01585bfebdc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -305,9 +305,6 @@ all::\n #\n # Define NO_TCLTK if you do not want Tcl/Tk GUI.\n #\n-# Define SANE_TEXT_GREP to \"-a\" if you use recent versions of GNU grep\n-# and egrep that are pickier when their input contains non-ASCII data.\n-#\n # The TCL_PATH variable governs the location of the Tcl interpreter\n # used to optimize git-gui for your system. Only used if NO_TCLTK\n # is not set. Defaults to the bare 'tclsh'.\n@@ -2254,7 +2251,7 @@ hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\\\n \t$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n-\t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\n+\t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\\\n \t$(perllibdir_SQ)\n GIT-SCRIPT-DEFINES: FORCE\n \t@FLAGS='$(SCRIPT_DEFINES)'; \\\n@@ -2272,7 +2269,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e $(BROKEN_PATH_FIX) \\\n -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n- -e 's|@@SANE_TEXT_GREP@@|$(SANE_TEXT_GREP)|g' \\\n -e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \\\n $@.sh >$@+\n endef\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 3236a4918a3..d0701f9beb0 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -58,7 +58,6 @@ ifeq ($(uname_S),Linux)\n \t# -lrt is needed for clock_gettime on glibc <= 2.16\n \tNEEDS_LIBRT = YesPlease\n \tHAVE_GETDELIM = YesPlease\n-\tSANE_TEXT_GREP=-a\n \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n \tBASIC_CFLAGS += -DHAVE_SYSINFO\n \tPROCFS_EXECUTABLE_PATH = /proc/self/exe\ndiff --git a/configure.ac b/configure.ac\nindex 031e8d3fee8..5ee25ec95c8 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -507,14 +507,6 @@ if test -n \"$ASCIIDOC\"; then\n \tesac\n fi\n \n-if grep -a ascii configure.ac >/dev/null; then\n- AC_MSG_RESULT([Using 'grep -a' for sane_grep])\n- SANE_TEXT_GREP=-a\n-else\n- SANE_TEXT_GREP=\n-fi\n-GIT_CONF_SUBST([SANE_TEXT_GREP])\n-\n ## Checks for libraries.\n AC_MSG_NOTICE([CHECKS for libraries])\n #\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex fd1399c440f..86b46114464 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -781,7 +781,6 @@ foreach(script ${git_shell_scripts})\n \tstring(REPLACE \"@@USE_GETTEXT_SCHEME@@\" \"\" content \"${content}\")\n \tstring(REPLACE \"# @@BROKEN_PATH_FIX@@\" \"\" content \"${content}\")\n \tstring(REPLACE \"@@PERL@@\" \"${PERL_PATH}\" content \"${content}\")\n-\tstring(REPLACE \"@@SANE_TEXT_GREP@@\" \"-a\" content \"${content}\")\n \tstring(REPLACE \"@@PAGER_ENV@@\" \"LESS=FRX LV=-c\" content \"${content}\")\n \tfile(WRITE ${CMAKE_BINARY_DIR}/${script} ${content})\n endforeach()\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex cb893728136..3a51d4507c7 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -579,7 +579,7 @@ if [ \"$filter_tag_name\" ]; then\n \t\t\t\tgit hash-object -t tag -w --stdin) ||\n \t\t\t\tdie \"Could not create new tag object for $ref\"\n \t\t\tif git cat-file tag \"$ref\" | \\\n-\t\t\t sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n+\t\t\t grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n \t\t\tthen\n \t\t\t\twarn \"gpg signature stripped from tag object $sha1t\"\n \t\t\tfi\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 54094fbe8de..4349566c891 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -49,7 +49,7 @@ resolve_full_httpd () {\n \t*apache2*|*lighttpd*|*httpd*)\n \t\t# yes, *httpd* covers *lighttpd* above, but it is there for clarity\n \t\t# ensure that the apache2/lighttpd command ends with \"-f\"\n-\t\tif ! echo \"$httpd\" | sane_grep -- '-f *$' >/dev/null 2>&1\n+\t\tif ! echo \"$httpd\" | grep -- '-f *$' >/dev/null 2>&1\n \t\tthen\n \t\t\thttpd=\"$httpd -f\"\n \t\tfi\n@@ -399,7 +399,7 @@ EOF\n \t\t# plain-old CGI\n \t\tresolve_full_httpd\n \t\tlist_mods=$(echo \"$full_httpd\" | sed 's/-f$/-l/')\n-\t\t$list_mods | sane_grep 'mod_cgi\\.c' >/dev/null 2>&1 || \\\n+\t\t$list_mods | grep 'mod_cgi\\.c' >/dev/null 2>&1 || \\\n \t\tif test -f \"$module_path/mod_cgi.so\"\n \t\tthen\n \t\t\techo \"LoadModule cgi_module $module_path/mod_cgi.so\" >> \"$conf\"\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 758520f5a61..1eb79458b48 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -173,10 +173,6 @@ git_pager() {\n \teval \"$GIT_PAGER\" '\"$@\"'\n }\n \n-sane_grep () {\n-\tGREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ \"$@\"\n-}\n-\n is_bare_repository () {\n \tgit rev-parse --is-bare-repository\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439303","messageId":"xmqqwnm6ge7w.fsf@gitster.g","threadId":"56751","inReplyTo":"patch-6.6-556fa96dde7-20211021T195538Z-avarab@gmail.com","subject":"Re: [PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T23:17:23Z","receivedAt":"2021-10-21T23:17:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> 2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass \"-a\" if\n> grep accepts it, 2016-03-08) isn't needed either, none of these grep\n> uses deal with binary data.\n\nFor all other users, I think the above applies well, but the one in\nfilter-branch deals with end-user contents, so we cannot say\n\"contents of the tag cannot be binary\".\n\nNot that I care about filter-branch too deeply ;-)\n"},{"id":"439319","messageId":"xmqq8rylgaz4.fsf@gitster.g","threadId":"56751","inReplyTo":"patch-5.6-eacbfd6be0e-20211021T195538Z-avarab@gmail.com","subject":"Re: [PATCH 5/6] git-sh-setup: remove unused sane_egrep() function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-22T00:27:27Z","receivedAt":"2021-10-22T00:27:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The is_zero_oid() function in git-submodule.sh has not been used since\n> e83e3333b57 (submodule: port submodule subcommand 'summary' from shell\n> to C, 2020-08-13), so we can remove it, and the sane_egrep() function,\n> dead is_zero_oid() was the only function which still referenced it.\n>\n> Unlike some other functions in git-sh-setup.sh, this function has not\n> been documented in git-sh-setup(1), so per [1] it should be OK to\n> remove it. I'm still unclear about the future of some of the other\n> functions[2], but any questions in that area should not apply here.\n>\n> 1. https://lore.kernel.org/git/xmqqr1dtgnn8.fsf@gitster.g/\n> 1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/\n\nDon't force readers to go outside of \"git log\" stream they are\nreading. Instead, summarize the argument that you base your\ndecision on for readers of your proposed commit log message.\n\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> git-sh-setup.sh | 4 ----\n> 1 file changed, 4 deletions(-)\n>\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index cee053cdc38..758520f5a61 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -177,10 +177,6 @@ sane_grep () {\n> \tGREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ \"$@\"\n> }\n> \n> -sane_egrep () {\n> -\tGREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ \"$@\"\n> -}\n> -\n> is_bare_repository () {\n> \tgit rev-parse --is-bare-repository\n> }\n"},{"id":"439376","messageId":"nycvar.QRO.7.76.6.2110221625570.62@tvgsbejvaqbjf.bet","threadId":"56751","inReplyTo":"xmqqwnm6ge7w.fsf@gitster.g","subject":"Re: [PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-22T14:27:28Z","receivedAt":"2021-10-22T14:27:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Oct 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n> > 2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass \"-a\" if\n> > grep accepts it, 2016-03-08) isn't needed either, none of these grep\n> > uses deal with binary data.\n>\n> For all other users, I think the above applies well, but the one in\n> filter-branch deals with end-user contents, so we cannot say\n> \"contents of the tag cannot be binary\".\n>\n> Not that I care about filter-branch too deeply ;-)\n\nYou probably meant that you do not care about filter-branch too deeply?\n;-)\n\nFWIW neither do I. Recently, I had the need for some large-scale\nrewriting, and I used this as an excuse to learn me some\n`git-repo-filter`. I was pleasantly surprised how easy it is to use, and\nhow fast it works.\n\nSo yeah, I don't care about `git filter-branch` too deeply, either ;-)\n\nCiao,\nDscho\n"},{"id":"439483","messageId":"xmqq1r4a6b9n.fsf@gitster.g","threadId":"56751","inReplyTo":"nycvar.QRO.7.76.6.2110221625570.62@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-24T09:08:04Z","receivedAt":"2021-10-24T09:08:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> For all other users, I think the above applies well, but the one in\n>> filter-branch deals with end-user contents, so we cannot say\n>> \"contents of the tag cannot be binary\".\n>>\n>> Not that I care about filter-branch too deeply ;-)\n>\n> You probably meant that you do not care about filter-branch too deeply?\n> ;-)\n\nI don't get it. Not only I meant, I said so, didn't I?\n"},{"id":"439545","messageId":"nycvar.QRO.7.76.6.2110251509340.62@tvgsbejvaqbjf.bet","threadId":"56751","inReplyTo":"xmqq1r4a6b9n.fsf@gitster.g","subject":"Re: [PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-25T13:09:57Z","receivedAt":"2021-10-25T13:10:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 24 Oct 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> >> For all other users, I think the above applies well, but the one in\n> >> filter-branch deals with end-user contents, so we cannot say\n> >> \"contents of the tag cannot be binary\".\n> >>\n> >> Not that I care about filter-branch too deeply ;-)\n> >\n> > You probably meant that you do not care about filter-branch too deeply?\n> > ;-)\n>\n> I don't get it. Not only I meant, I said so, didn't I?\n\nHah, I autocorrected your \"Not\" to \"Note\" in my mind ;-)\n\nCiao,\nDscho\n"},{"id":"439567","messageId":"xmqq1r492fiw.fsf@gitster.g","threadId":"56751","inReplyTo":"nycvar.QRO.7.76.6.2110251509340.62@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 6/6] git-sh-setup: remove \"sane_grep\", it's not needed anymore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-25T17:14:15Z","receivedAt":"2021-10-25T17:14:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Junio,\n>\n> On Sun, 24 Oct 2021, Junio C Hamano wrote:\n>\n>> Johannes Schindelin writes:\n>>\n>> >> For all other users, I think the above applies well, but the one in\n>> >> filter-branch deals with end-user contents, so we cannot say\n>> >> \"contents of the tag cannot be binary\".\n>> >>\n>> >> Not that I care about filter-branch too deeply ;-)\n>> >\n>> > You probably meant that you do not care about filter-branch too deeply?\n>> > ;-)\n>>\n>> I don't get it. Not only I meant, I said so, didn't I?\n>\n> Hah, I autocorrected your \"Not\" to \"Note\" in my mind ;-)\n\nAhh, a single byte makes quite a big difference ;-)\n"},{"id":"439578","messageId":"YXb9nYRWBxa7jDKB@coredump.intra.peff.net","threadId":"56751","inReplyTo":"patch-2.6-7cc3a1d7205-20211021T195538Z-avarab@gmail.com","subject":"Re: [PATCH 2/6] Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T18:57:11Z","receivedAt":"2021-10-25T18:57:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2021 at 09:57:56PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Remove the $(GIT_VERSION) from $(SCRIPT_DEFINES). Now every time HEAD\n> changes in a development copy we don't need to re-build the scripts\n> and script libraries.\n> \n> This has not been needed since 2b9391bc675 (Makefile: do not replace\n> @@GIT_VERSION@@ in shell scripts, 2012-06-20). On my setup this\n> changes the re-making of 44 targets in a development copy where moved\n> HEAD to 27.\n> \n> The $(GIT_VERSION) was seemingly left here by mistake or omission. We\n> didn't need it since 2b9391bc675, but in the later\n> e4dd89ab984 (Makefile: update scripts when build-time parameters\n> change, 2012-06-20) it was added to SCRIPT_DEFINES.\n> \n> The two were part of the same series of patches, and given the summary\n> in [1] and [2] it looks like this was probably a case of some earlier\n> version of a later patch being combined with an updated earlier patch.\n>\nYeah, that seems likely. No such version hit the list, but I probably\nwrote the SCRIPT_DEFINES one first, and then realized GIT_VERSION could\nbe dropped, but didn't update the other.\n\nAt any rate, your patch looks like the correct thing to do.\n\n-Peff\n"},{"id":"439581","messageId":"YXcAyWR55UurEUwv@coredump.intra.peff.net","threadId":"56751","inReplyTo":"cover-0.6-00000000000-20211021T195538Z-avarab@gmail.com","subject":"Re: [PATCH 0/6] Makefile: be less eager to re-build, *.sh code cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T19:08:57Z","receivedAt":"2021-10-25T19:09:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2021 at 09:57:54PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> This removes the dependency of our built *.sh on $(GIT_VERSION), which\n> as noted in 2/6 looks to have been the intention back in 2012, but\n> didn't happen then.\n> \n> The rest of the series removes more dead code related to our\n> $(SCRIPT_DEFINES), some of it it not used or needed for 5-15ish years.\n> \n> Ævar Arnfjörð Bjarmason (6):\n> Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)\n> Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)\n> Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)\n> git-instaweb: unconditionally assume that gitweb is mod_perl capable\n> git-sh-setup: remove unused sane_egrep() function\n> git-sh-setup: remove \"sane_grep\", it's not needed anymore\n\nThe first five all look obviously correct to me. The 6th also looks\nfine, but I don't have a strong opinion on the filter-branch thing\neither way.\n\n-Peff\n"}]}