{"thread":{"id":"61293","subject":"Makefiles are broken as of GNU Make commit 07fcee35f058a876447c8a021f9eb1943f902534","startedAt":"2024-04-08T10:45:04Z","lastAt":"2024-04-09T21:23:53Z","messageCount":13,"participants":["Dario Gjorgjevski","Taylor Blau","Junio C Hamano","Paul Smith","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"492510","messageId":"CAJm4QYOxn_s8ktJiC6ju2j4OyEYaM2ay7Ca--ZWFWa7APVnTbA@mail.gmail.com","threadId":"61293","inReplyTo":null,"subject":"Makefiles are broken as of GNU Make commit 07fcee35f058a876447c8a021f9eb1943f902534","fromName":"Dario Gjorgjevski","fromEmail":"dario.gjorgjevski@gmail.com","sentAt":"2024-04-08T10:44:26Z","receivedAt":"2024-04-08T10:45:04Z","isPatch":false,"sender":{"key":"dario.gjorgjevski@gmail.com","avatar":null},"body":"Hi,\n\nGit has plenty of conditional statements in its Makefiles which begin\nwith the recipe prefix (tab). This used to work in the past, but no\nlonger does as of GNU Make commit\n07fcee35f058a876447c8a021f9eb1943f902534:\nhttps://git.savannah.gnu.org/cgit/make.git/commit/?id=07fcee35f058a876447c8a021f9eb1943f902534.\nThis commit has not yet landed in a release, but it probably will in\nthe future.\n\nA similar bug was recently fixed in the Linux kernel:\nhttps://github.com/torvalds/linux/commit/82175d1f9430d5a026e2231782d13da0bf57155c.\n\nBest regards,\nDario\n"},{"id":"492546","messageId":"9d14c08ca6cc06cdf8fb4ba33d2470053dca3966.1712591504.git.me@ttaylorr.com","threadId":"61293","inReplyTo":"CAJm4QYOxn_s8ktJiC6ju2j4OyEYaM2ay7Ca--ZWFWa7APVnTbA@mail.gmail.com","subject":"[PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-04-08T15:51:44Z","receivedAt":"2024-04-08T15:51:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In GNU Make commit 07fcee35 ([SV 64815] Recipe lines cannot contain\nconditional statements, 2023-05-22) and following, conditional\nstatements may no longer be preceded by a tab character (which Make\nrefers to as the recipe prefix).\n\nThere are a handful of spots in our various Makefile(s) which will break\nin a future release of Make containing 07fcee35. For instance, trying to\ncompile the pre-image of this patch with the tip of make.git results in\nthe following:\n\n $ make -v | head -1 && make\n GNU Make 4.4.90\n config.mak.uname:842: *** missing 'endif'. Stop.\n\nThe kernel addressed this issue in 82175d1f9430 (kbuild: Replace tabs\nwith spaces when followed by conditionals, 2024-01-28). Address the\nissues in Git's tree by applying the same strategy.\n\nWhen a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or\nmore tab characters, replace each tab character with 8 space characters\nwith the following:\n\n find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |\n xargs perl -i -pe '\n s/(\\t+)(ifn?eq|ifn?def|else|endif)/\" \" x (length($1) * 8) . $2/ge unless /\\\\$/\n '\n\nThe \"unless /\\\\$/\" removes any false-positives (like \"\\telse \\\"\nappearing within a shell script as part of a recipe).\n\nAfter doing so, Git compiles on newer versions of Make:\n\n $ make -v | head -1 && make\n GNU Make 4.4.90\n GIT_VERSION = 2.44.0.414.gfac1dc44ca9\n [...]\n\n $ echo $?\n 0\n\nReported-by: Dario Gjorgjevski \nSigned-off-by: Taylor Blau \n---\n Makefile | 96 ++++++++++++++---------------\n config.mak.uname | 154 +++++++++++++++++++++++-----------------------\n git-gui/Makefile | 16 ++---\n gitk-git/Makefile | 4 +-\n 4 files changed, 135 insertions(+), 135 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c43c1bd1a05..ac603fb7678 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1557,23 +1557,23 @@ ifneq (,$(SOCKLEN_T))\n endif\n \n ifeq ($(uname_S),Darwin)\n-\tifndef NO_FINK\n-\t\tifeq ($(shell test -d /sw/lib && echo y),y)\n+ ifndef NO_FINK\n+ ifeq ($(shell test -d /sw/lib && echo y),y)\n \t\t\tBASIC_CFLAGS += -I/sw/include\n \t\t\tBASIC_LDFLAGS += -L/sw/lib\n-\t\tendif\n-\tendif\n-\tifndef NO_DARWIN_PORTS\n-\t\tifeq ($(shell test -d /opt/local/lib && echo y),y)\n+ endif\n+ endif\n+ ifndef NO_DARWIN_PORTS\n+ ifeq ($(shell test -d /opt/local/lib && echo y),y)\n \t\t\tBASIC_CFLAGS += -I/opt/local/include\n \t\t\tBASIC_LDFLAGS += -L/opt/local/lib\n-\t\tendif\n-\tendif\n-\tifndef NO_APPLE_COMMON_CRYPTO\n+ endif\n+ endif\n+ ifndef NO_APPLE_COMMON_CRYPTO\n \t\tNO_OPENSSL = YesPlease\n \t\tAPPLE_COMMON_CRYPTO = YesPlease\n \t\tCOMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO\n-\tendif\n+ endif\n \tPTHREAD_LIBS =\n endif\n \n@@ -1612,23 +1612,23 @@ ifdef NO_CURL\n \tREMOTE_CURL_NAMES =\n \tEXCLUDED_PROGRAMS += git-http-fetch git-http-push\n else\n-\tifdef CURLDIR\n+ ifdef CURLDIR\n \t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n \t\tCURL_CFLAGS = -I$(CURLDIR)/include\n \t\tCURL_LIBCURL = $(call libpath_template,$(CURLDIR)/$(lib))\n-\telse\n+ else\n \t\tCURL_CFLAGS =\n \t\tCURL_LIBCURL =\n-\tendif\n+ endif\n \n-\tifndef CURL_LDFLAGS\n+ ifndef CURL_LDFLAGS\n \t\tCURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)\n-\tendif\n+ endif\n \tCURL_LIBCURL += $(CURL_LDFLAGS)\n \n-\tifndef CURL_CFLAGS\n+ ifndef CURL_CFLAGS\n \t\tCURL_CFLAGS = $(eval CURL_CFLAGS := $$(shell $$(CURL_CONFIG) --cflags))$(CURL_CFLAGS)\n-\tendif\n+ endif\n \tBASIC_CFLAGS += $(CURL_CFLAGS)\n \n \tREMOTE_CURL_PRIMARY = git-remote-http$X\n@@ -1636,29 +1636,29 @@ else\n \tREMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)\n \tPROGRAM_OBJS += http-fetch.o\n \tPROGRAMS += $(REMOTE_CURL_NAMES)\n-\tifndef NO_EXPAT\n+ ifndef NO_EXPAT\n \t\tPROGRAM_OBJS += http-push.o\n-\tendif\n+ endif\n \tcurl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)\n-\tifeq \"$(curl_check)\" \"072200\"\n+ ifeq \"$(curl_check)\" \"072200\"\n \t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n-\tendif\n-\tifdef USE_CURL_FOR_IMAP_SEND\n+ endif\n+ ifdef USE_CURL_FOR_IMAP_SEND\n \t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n \t\tIMAP_SEND_BUILDDEPS = http.o\n \t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n-\tendif\n-\tifndef NO_EXPAT\n-\t\tifdef EXPATDIR\n+ endif\n+ ifndef NO_EXPAT\n+ ifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n \t\t\tEXPAT_LIBEXPAT = $(call libpath_template,$(EXPATDIR)/$(lib)) -lexpat\n-\t\telse\n+ else\n \t\t\tEXPAT_LIBEXPAT = -lexpat\n-\t\tendif\n-\t\tifdef EXPAT_NEEDS_XMLPARSE_H\n+ endif\n+ ifdef EXPAT_NEEDS_XMLPARSE_H\n \t\t\tBASIC_CFLAGS += -DEXPAT_NEEDS_XMLPARSE_H\n-\t\tendif\n-\tendif\n+ endif\n+ endif\n endif\n IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n \n@@ -1670,15 +1670,15 @@ EXTLIBS += -lz\n \n ifndef NO_OPENSSL\n \tOPENSSL_LIBSSL = -lssl\n-\tifdef OPENSSLDIR\n+ ifdef OPENSSLDIR\n \t\tBASIC_CFLAGS += -I$(OPENSSLDIR)/include\n \t\tOPENSSL_LINK = $(call libpath_template,$(OPENSSLDIR)/$(lib))\n-\telse\n+ else\n \t\tOPENSSL_LINK =\n-\tendif\n-\tifdef NEEDS_CRYPTO_WITH_SSL\n+ endif\n+ ifdef NEEDS_CRYPTO_WITH_SSL\n \t\tOPENSSL_LIBSSL += -lcrypto\n-\tendif\n+ endif\n else\n \tBASIC_CFLAGS += -DNO_OPENSSL\n \tOPENSSL_LIBSSL =\n@@ -1696,18 +1696,18 @@ ifdef APPLE_COMMON_CRYPTO\n endif\n endif\n ifndef NO_ICONV\n-\tifdef NEEDS_LIBICONV\n-\t\tifdef ICONVDIR\n+ ifdef NEEDS_LIBICONV\n+ ifdef ICONVDIR\n \t\t\tBASIC_CFLAGS += -I$(ICONVDIR)/include\n \t\t\tICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib))\n-\t\telse\n+ else\n \t\t\tICONV_LINK =\n-\t\tendif\n-\t\tifdef NEEDS_LIBINTL_BEFORE_LIBICONV\n+ endif\n+ ifdef NEEDS_LIBINTL_BEFORE_LIBICONV\n \t\t\tICONV_LINK += -lintl\n-\t\tendif\n+ endif\n \t\tEXTLIBS += $(ICONV_LINK) -liconv\n-\tendif\n+ endif\n endif\n ifdef ICONV_OMITS_BOM\n \tBASIC_CFLAGS += -DICONV_OMITS_BOM\n@@ -1828,10 +1828,10 @@ ifdef NO_MMAP\n \tCOMPAT_CFLAGS += -DNO_MMAP\n \tCOMPAT_OBJS += compat/mmap.o\n else\n-\tifdef USE_WIN32_MMAP\n+ ifdef USE_WIN32_MMAP\n \t\tCOMPAT_CFLAGS += -DUSE_WIN32_MMAP\n \t\tCOMPAT_OBJS += compat/win32mmap.o\n-\tendif\n+ endif\n endif\n ifdef MMAP_PREVENTS_DELETE\n \tBASIC_CFLAGS += -DMMAP_PREVENTS_DELETE\n@@ -1956,11 +1956,11 @@ else\n \tBASIC_CFLAGS += -DSHA1_DC\n \tLIB_OBJS += sha1dc_git.o\n ifdef DC_SHA1_EXTERNAL\n-\tifdef DC_SHA1_SUBMODULE\n-\t\tifneq ($(DC_SHA1_SUBMODULE),auto)\n+ ifdef DC_SHA1_SUBMODULE\n+ ifneq ($(DC_SHA1_SUBMODULE),auto)\n $(error Only set DC_SHA1_EXTERNAL or DC_SHA1_SUBMODULE, not both)\n-\t\tendif\n-\tendif\n+ endif\n+ endif\n \tBASIC_CFLAGS += -DDC_SHA1_EXTERNAL\n \tEXTLIBS += -lsha1detectcoll\n else\ndiff --git a/config.mak.uname b/config.mak.uname\nindex d0dcca2ec55..b11408e67b4 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -65,9 +65,9 @@ ifeq ($(uname_S),Linux)\n \tHAVE_PLATFORM_PROCINFO = YesPlease\n \tCOMPAT_OBJS += compat/linux/procinfo.o\n \t# centos7/rhel7 provides gcc 4.8.5 and zlib 1.2.7.\n-\tifneq ($(findstring .el7.,$(uname_R)),)\n+ ifneq ($(findstring .el7.,$(uname_R)),)\n \t\tBASIC_CFLAGS += -std=c99\n-\tendif\n+ endif\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tHAVE_ALLOCA_H = YesPlease\n@@ -95,13 +95,13 @@ ifeq ($(uname_S),UnixWare)\n \tNO_MEMMEM = YesPlease\n endif\n ifeq ($(uname_S),SCO_SV)\n-\tifeq ($(uname_R),3.2)\n+ ifeq ($(uname_R),3.2)\n \t\tCFLAGS = -O2\n-\tendif\n-\tifeq ($(uname_R),5)\n+ endif\n+ ifeq ($(uname_R),5)\n \t\tCC = cc\n \t\tBASIC_CFLAGS += -Kthread\n-\tendif\n+ endif\n \tNEEDS_SOCKET = YesPlease\n \tNEEDS_NSL = YesPlease\n \tNEEDS_SSL_WITH_CRYPTO = YesPlease\n@@ -124,19 +124,19 @@ ifeq ($(uname_S),Darwin)\n \t# - MacOS 10.0.* and MacOS 10.1.0 = Darwin 1.*\n \t# - MacOS 10.x.* = Darwin (x+4).* for (1 <= x)\n \t# i.e. \"begins with [15678] and a dot\" means \"10.4.* or older\".\n-\tifeq ($(shell expr \"$(uname_R)\" : '[15678]\\.'),2)\n+ ifeq ($(shell expr \"$(uname_R)\" : '[15678]\\.'),2)\n \t\tOLD_ICONV = UnfortunatelyYes\n \t\tNO_APPLE_COMMON_CRYPTO = YesPlease\n-\tendif\n-\tifeq ($(shell expr \"$(uname_R)\" : '[15]\\.'),2)\n+ endif\n+ ifeq ($(shell expr \"$(uname_R)\" : '[15]\\.'),2)\n \t\tNO_STRLCPY = YesPlease\n-\tendif\n-\tifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 11 && echo 1),1)\n+ endif\n+ ifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 11 && echo 1),1)\n \t\tHAVE_GETDELIM = YesPlease\n-\tendif\n-\tifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 20 && echo 1),1)\n+ endif\n+ ifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 20 && echo 1),1)\n \t\tOPEN_RETURNS_EINTR = UnfortunatelyYes\n-\tendif\n+ endif\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n@@ -152,12 +152,12 @@ ifeq ($(uname_S),Darwin)\n \t# Workaround for `gettext` being keg-only and not even being linked via\n \t# `brew link --force gettext`, should be obsolete as of\n \t# https://github.com/Homebrew/homebrew-core/pull/53489\n-\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n+ ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n \t\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n \t\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n-\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n+ ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n \t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n-\t\tendif\n+ endif\n \t# On newer ARM-based machines the default installation path has changed to\n \t# /opt/homebrew. Include it in our search paths so that the user does not\n \t# have to configure this manually.\n@@ -165,22 +165,22 @@ ifeq ($(uname_S),Darwin)\n \t# Note that we do not employ the same workaround as above where we manually\n \t# add gettext. The issue was fixed more than three years ago by now, and at\n \t# that point there haven't been any ARM-based Macs yet.\n-\telse ifeq ($(shell test -d /opt/homebrew/ && echo y),y)\n+ else ifeq ($(shell test -d /opt/homebrew/ && echo y),y)\n \t\tBASIC_CFLAGS += -I/opt/homebrew/include\n \t\tBASIC_LDFLAGS += -L/opt/homebrew/lib\n-\t\tifeq ($(shell test -x /opt/homebrew/bin/msgfmt && echo y),y)\n+ ifeq ($(shell test -x /opt/homebrew/bin/msgfmt && echo y),y)\n \t\t\tMSGFMT = /opt/homebrew/bin/msgfmt\n-\t\tendif\n-\tendif\n+ endif\n+ endif\n \n \t# The builtin FSMonitor on MacOS builds upon Simple-IPC. Both require\n \t# Unix domain sockets and PThreads.\n-\tifndef NO_PTHREADS\n-\tifndef NO_UNIX_SOCKETS\n+ ifndef NO_PTHREADS\n+ ifndef NO_UNIX_SOCKETS\n \tFSMONITOR_DAEMON_BACKEND = darwin\n \tFSMONITOR_OS_SETTINGS = darwin\n-\tendif\n-\tendif\n+ endif\n+ endif\n \n \tBASIC_LDFLAGS += -framework CoreServices\n endif\n@@ -196,7 +196,7 @@ ifeq ($(uname_S),SunOS)\n \tNO_REGEX = YesPlease\n \tNO_MSGFMT_EXTENDED_OPTIONS = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n-\tifeq ($(uname_R),5.6)\n+ ifeq ($(uname_R),5.6)\n \t\tSOCKLEN_T = int\n \t\tNO_HSTRERROR = YesPlease\n \t\tNO_IPV6 = YesPlease\n@@ -206,8 +206,8 @@ ifeq ($(uname_S),SunOS)\n \t\tNO_STRLCPY = YesPlease\n \t\tNO_STRTOUMAX = YesPlease\n \t\tGIT_TEST_CMP = cmp\n-\tendif\n-\tifeq ($(uname_R),5.7)\n+ endif\n+ ifeq ($(uname_R),5.7)\n \t\tNEEDS_RESOLV = YesPlease\n \t\tNO_IPV6 = YesPlease\n \t\tNO_SOCKADDR_STORAGE = YesPlease\n@@ -216,25 +216,25 @@ ifeq ($(uname_S),SunOS)\n \t\tNO_STRLCPY = YesPlease\n \t\tNO_STRTOUMAX = YesPlease\n \t\tGIT_TEST_CMP = cmp\n-\tendif\n-\tifeq ($(uname_R),5.8)\n+ endif\n+ ifeq ($(uname_R),5.8)\n \t\tNO_UNSETENV = YesPlease\n \t\tNO_SETENV = YesPlease\n \t\tNO_STRTOUMAX = YesPlease\n \t\tGIT_TEST_CMP = cmp\n-\tendif\n-\tifeq ($(uname_R),5.9)\n+ endif\n+ ifeq ($(uname_R),5.9)\n \t\tNO_UNSETENV = YesPlease\n \t\tNO_SETENV = YesPlease\n \t\tNO_STRTOUMAX = YesPlease\n \t\tGIT_TEST_CMP = cmp\n-\tendif\n+ endif\n \tINSTALL = /usr/ucb/install\n \tTAR = gtar\n \tBASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__\n endif\n ifeq ($(uname_O),Cygwin)\n-\tifeq ($(shell expr \"$(uname_R)\" : '1\\.[1-6]\\.'),4)\n+ ifeq ($(shell expr \"$(uname_R)\" : '1\\.[1-6]\\.'),4)\n \t\tNO_D_TYPE_IN_DIRENT = YesPlease\n \t\tNO_STRCASESTR = YesPlease\n \t\tNO_MEMMEM = YesPlease\n@@ -245,9 +245,9 @@ ifeq ($(uname_O),Cygwin)\n \t\t# On some boxes NO_MMAP is needed, and not so elsewhere.\n \t\t# Try commenting this out if you suspect MMAP is more efficient\n \t\tNO_MMAP = YesPlease\n-\telse\n+ else\n \t\tNO_REGEX = UnfortunatelyYes\n-\tendif\n+ endif\n \tHAVE_ALLOCA_H = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n \tNO_FAST_WORKING_DIRECTORY = UnfortunatelyYes\n@@ -263,25 +263,25 @@ ifeq ($(uname_S),FreeBSD)\n \tNEEDS_LIBICONV = YesPlease\n \t# Versions up to 10.1 require OLD_ICONV; 10.2 and beyond don't.\n \t# A typical version string looks like \"10.2-RELEASE\".\n-\tifeq ($(shell expr \"$(uname_R)\" : '[1-9]\\.'),2)\n+ ifeq ($(shell expr \"$(uname_R)\" : '[1-9]\\.'),2)\n \t\tOLD_ICONV = YesPlease\n-\tendif\n-\tifeq ($(firstword $(subst -, ,$(uname_R))),10.0)\n+ endif\n+ ifeq ($(firstword $(subst -, ,$(uname_R))),10.0)\n \t\tOLD_ICONV = YesPlease\n-\tendif\n-\tifeq ($(firstword $(subst -, ,$(uname_R))),10.1)\n+ endif\n+ ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)\n \t\tOLD_ICONV = YesPlease\n-\tendif\n+ endif\n \tNO_MEMMEM = YesPlease\n \tBASIC_CFLAGS += -I/usr/local/include\n \tBASIC_LDFLAGS += -L/usr/local/lib\n \tDIR_HAS_BSD_GROUP_SEMANTICS = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n-\tifeq ($(shell expr \"$(uname_R)\" : '4\\.'),2)\n+ ifeq ($(shell expr \"$(uname_R)\" : '4\\.'),2)\n \t\tPTHREAD_LIBS = -pthread\n \t\tNO_UINTMAX_T = YesPlease\n \t\tNO_STRTOUMAX = YesPlease\n-\tendif\n+ endif\n \tPYTHON_PATH = /usr/local/bin/python\n \tPERL_PATH = /usr/local/bin/perl\n \tHAVE_PATHS_H = YesPlease\n@@ -317,9 +317,9 @@ ifeq ($(uname_S),MirBSD)\n \tCSPRNG_METHOD = arc4random\n endif\n ifeq ($(uname_S),NetBSD)\n-\tifeq ($(shell expr \"$(uname_R)\" : '[01]\\.'),2)\n+ ifeq ($(shell expr \"$(uname_R)\" : '[01]\\.'),2)\n \t\tNEEDS_LIBICONV = YesPlease\n-\tendif\n+ endif\n \tBASIC_CFLAGS += -I/usr/pkg/include\n \tBASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib\n \tUSE_ST_TIMESPEC = YesPlease\n@@ -343,14 +343,14 @@ ifeq ($(uname_S),AIX)\n \tBASIC_CFLAGS += -D_LARGE_FILES\n \tFILENO_IS_A_MACRO = UnfortunatelyYes\n \tNEED_ACCESS_ROOT_HANDLER = UnfortunatelyYes\n-\tifeq ($(shell expr \"$(uname_V)\" : '[1234]'),1)\n+ ifeq ($(shell expr \"$(uname_V)\" : '[1234]'),1)\n \t\tNO_PTHREADS = YesPlease\n-\telse\n+ else\n \t\tPTHREAD_LIBS = -lpthread\n-\tendif\n-\tifeq ($(shell expr \"$(uname_V).$(uname_R)\" : '5\\.1'),3)\n+ endif\n+ ifeq ($(shell expr \"$(uname_V).$(uname_R)\" : '5\\.1'),3)\n \t\tINLINE = ''\n-\tendif\n+ endif\n \tGIT_TEST_CMP = cmp\n endif\n ifeq ($(uname_S),GNU)\n@@ -410,29 +410,29 @@ ifeq ($(uname_S),HP-UX)\n \tNO_SYS_SELECT_H = YesPlease\n \tSNPRINTF_RETURNS_BOGUS = YesPlease\n \tNO_NSEC = YesPlease\n-\tifeq ($(uname_R),B.11.00)\n+ ifeq ($(uname_R),B.11.00)\n \t\tNO_INET_NTOP = YesPlease\n \t\tNO_INET_PTON = YesPlease\n-\tendif\n-\tifeq ($(uname_R),B.10.20)\n+ endif\n+ ifeq ($(uname_R),B.10.20)\n \t\t# Override HP-UX 11.x setting:\n \t\tINLINE =\n \t\tSOCKLEN_T = size_t\n \t\tNO_PREAD = YesPlease\n \t\tNO_INET_NTOP = YesPlease\n \t\tNO_INET_PTON = YesPlease\n-\tendif\n+ endif\n \tGIT_TEST_CMP = cmp\n endif\n ifeq ($(uname_S),Windows)\n \tGIT_VERSION := $(GIT_VERSION).MSVC\n \tpathsep = ;\n \t# Assume that this is built in Git for Windows' SDK\n-\tifeq (MINGW32,$(MSYSTEM))\n+ ifeq (MINGW32,$(MSYSTEM))\n \t\tprefix = /mingw32\n-\telse\n+ else\n \t\tprefix = /mingw64\n-\tendif\n+ endif\n \t# Prepend MSVC 64-bit tool-chain to PATH.\n \t#\n \t# A regular Git Bash *does not* have cl.exe in its $PATH. As there is a\n@@ -550,16 +550,16 @@ ifeq ($(uname_S),Interix)\n \tNO_MKDTEMP = YesPlease\n \tNO_STRTOUMAX = YesPlease\n \tNO_NSEC = YesPlease\n-\tifeq ($(uname_R),3.5)\n+ ifeq ($(uname_R),3.5)\n \t\tNO_INET_NTOP = YesPlease\n \t\tNO_INET_PTON = YesPlease\n \t\tNO_SOCKADDR_STORAGE = YesPlease\n-\tendif\n-\tifeq ($(uname_R),5.2)\n+ endif\n+ ifeq ($(uname_R),5.2)\n \t\tNO_INET_NTOP = YesPlease\n \t\tNO_INET_PTON = YesPlease\n \t\tNO_SOCKADDR_STORAGE = YesPlease\n-\tendif\n+ endif\n endif\n ifeq ($(uname_S),Minix)\n \tNO_IPV6 = YesPlease\n@@ -579,12 +579,12 @@ ifeq ($(uname_S),NONSTOP_KERNEL)\n \t# still not compile in c89 mode, due to non-const array initializations.\n \tCC = cc -c99\n \t# Build down-rev compatible objects that don't use our new getopt_long.\n-\tifeq ($(uname_R).$(uname_V),J06.21)\n+ ifeq ($(uname_R).$(uname_V),J06.21)\n \t\tCC += -WRVU=J06.20\n-\tendif\n-\tifeq ($(uname_R).$(uname_V),L17.02)\n+ endif\n+ ifeq ($(uname_R).$(uname_V),L17.02)\n \t\tCC += -WRVU=L16.05\n-\tendif\n+ endif\n \t# Disable all optimization, seems to result in bad code, with -O or -O2\n \t# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects\n \t# abends on \"git push\". Needs more investigation.\n@@ -651,9 +651,9 @@ ifeq ($(uname_S),OS/390)\n \tNEEDS_MODE_TRANSLATION = YesPlease\n endif\n ifeq ($(uname_S),MINGW)\n-\tifeq ($(shell expr \"$(uname_R)\" : '1\\.'),2)\n+ ifeq ($(shell expr \"$(uname_R)\" : '1\\.'),2)\n \t\t$(error \"Building with MSys is no longer supported\")\n-\tendif\n+ endif\n \tpathsep = ;\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n@@ -712,22 +712,22 @@ ifeq ($(uname_S),MINGW)\n \t# Enable DEP\n \tBASIC_LDFLAGS += -Wl,--nxcompat\n \t# Enable ASLR (unless debugging)\n-\tifneq (,$(findstring -O,$(filter-out -O0 -Og,$(CFLAGS))))\n+ ifneq (,$(findstring -O,$(filter-out -O0 -Og,$(CFLAGS))))\n \t\tBASIC_LDFLAGS += -Wl,--dynamicbase\n-\tendif\n-\tifeq (MINGW32,$(MSYSTEM))\n+ endif\n+ ifeq (MINGW32,$(MSYSTEM))\n \t\tprefix = /mingw32\n \t\tHOST_CPU = i686\n \t\tBASIC_LDFLAGS += -Wl,--pic-executable,-e,_mainCRTStartup\n-\tendif\n-\tifeq (MINGW64,$(MSYSTEM))\n+ endif\n+ ifeq (MINGW64,$(MSYSTEM))\n \t\tprefix = /mingw64\n \t\tHOST_CPU = x86_64\n \t\tBASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup\n-\telse\n+ else\n \t\tCOMPAT_CFLAGS += -D_USE_32BIT_TIME_T\n \t\tBASIC_LDFLAGS += -Wl,--large-address-aware\n-\tendif\n+ endif\n \tCC = gcc\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \\\n \t\t-fstack-protector-strong\n@@ -739,11 +739,11 @@ ifeq ($(uname_S),MINGW)\n \tUSE_GETTEXT_SCHEME = fallthrough\n \tUSE_LIBPCRE = YesPlease\n \tUSE_NED_ALLOCATOR = YesPlease\n-\tifeq (/mingw64,$(subst 32,64,$(prefix)))\n+ ifeq (/mingw64,$(subst 32,64,$(prefix)))\n \t\t# Move system config into top-level /etc/\n \t\tETC_GITCONFIG = ../etc/gitconfig\n \t\tETC_GITATTRIBUTES = ../etc/gitattributes\n-\tendif\n+ endif\n endif\n ifeq ($(uname_S),QNX)\n \tCOMPAT_CFLAGS += -DSA_RESTART=0\ndiff --git a/git-gui/Makefile b/git-gui/Makefile\nindex 3f80435436c..667c39ed564 100644\n--- a/git-gui/Makefile\n+++ b/git-gui/Makefile\n@@ -107,12 +107,12 @@ endif\n \n ifeq ($(uname_S),Darwin)\n \tTKFRAMEWORK = /Library/Frameworks/Tk.framework/Resources/Wish.app\n-\tifeq ($(shell echo \"$(uname_R)\" | awk -F. '{if ($$1 >= 9) print \"y\"}')_$(shell test -d $(TKFRAMEWORK) || echo n),y_n)\n+ ifeq ($(shell echo \"$(uname_R)\" | awk -F. '{if ($$1 >= 9) print \"y\"}')_$(shell test -d $(TKFRAMEWORK) || echo n),y_n)\n \t\tTKFRAMEWORK = /System/Library/Frameworks/Tk.framework/Resources/Wish.app\n-\t\tifeq ($(shell test -d $(TKFRAMEWORK) || echo n),n)\n+ ifeq ($(shell test -d $(TKFRAMEWORK) || echo n),n)\n \t\t\tTKFRAMEWORK = /System/Library/Frameworks/Tk.framework/Resources/Wish\\ Shell.app\n-\t\tendif\n-\tendif\n+ endif\n+ endif\n \tTKEXECUTABLE = $(shell basename \"$(TKFRAMEWORK)\" .app)\n endif\n \n@@ -143,9 +143,9 @@ ifeq ($(exedir),$(gg_libdir))\n endif\n gg_libdir_sed_in := $(gg_libdir)\n ifeq ($(uname_S),Darwin)\n-\tifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)\n+ ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)\n \t\tGITGUI_MACOSXAPP := YesPlease\n-\tendif\n+ endif\n endif\n ifneq (,$(findstring MINGW,$(uname_S)))\n ifeq ($(shell expr \"$(uname_R)\" : '1\\.'),2)\n@@ -220,9 +220,9 @@ ifdef NO_MSGFMT\n \tMSGFMT ?= $(TCL_PATH) po/po2msg.sh\n else\n \tMSGFMT ?= msgfmt\n-\tifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)\n+ ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)\n \t\tMSGFMT := $(TCL_PATH) po/po2msg.sh\n-\tendif\n+ endif\n endif\n \n msgsdir = $(gg_libdir)/msgs\ndiff --git a/gitk-git/Makefile b/gitk-git/Makefile\nindex 5bdd52a6ebf..e1f0aff4a19 100644\n--- a/gitk-git/Makefile\n+++ b/gitk-git/Makefile\n@@ -33,9 +33,9 @@ ifdef NO_MSGFMT\n \tMSGFMT ?= $(TCL_PATH) po/po2msg.sh\n else\n \tMSGFMT ?= msgfmt\n-\tifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)\n+ ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)\n \t\tMSGFMT := $(TCL_PATH) po/po2msg.sh\n-\tendif\n+ endif\n endif\n \n PO_TEMPLATE = po/gitk.pot\n-- \n2.44.0.414.g38c9f58cd0c\n"},{"id":"492587","messageId":"xmqqle5n8rcr.fsf@gitster.g","threadId":"61293","inReplyTo":"9d14c08ca6cc06cdf8fb4ba33d2470053dca3966.1712591504.git.me@ttaylorr.com","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-08T21:41:24Z","receivedAt":"2024-04-08T21:41:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or\n> more tab characters, replace each tab character with 8 space characters\n> with the following:\n>\n> find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |\n> xargs perl -i -pe '\n> s/(\\t+)(ifn?eq|ifn?def|else|endif)/\" \" x (length($1) * 8) . $2/ge unless /\\\\$/\n> '\n\nYuck, it means auto indenting Makefile and its pieces almost\nimpossible X-<. I'll take the patch as there is no way to revert\nthe change to GNU make, though.\n\nThanks.\n"},{"id":"492593","messageId":"606990048585347654f3b4b187ec27f4dc1b85e3.camel@gnu.org","threadId":"61293","inReplyTo":"xmqqle5n8rcr.fsf@gitster.g","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Paul Smith","fromEmail":"psmith@gnu.org","sentAt":"2024-04-08T23:24:16Z","receivedAt":"2024-04-08T23:24:19Z","isPatch":true,"sender":{"key":"psmith@gnu.org","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Mon, 2024-04-08 at 14:41 -0700, Junio C Hamano wrote:\n> Taylor Blau writes:\n> \n> > When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by\n> > one or\n> > more tab characters, replace each tab character with 8 space\n> > characters\n> > with the following:\n> > \n> >      find . -type f -not -path './.git/*' -name Makefile -or -name\n> > '*.mak' |\n> >        xargs perl -i -pe '\n> >          s/(\\t+)(ifn?eq|ifn?def|else|endif)/\" \" x (length($1) * 8)\n> > . $2/ge unless /\\\\$/\n> >        '\n> \n> Yuck, it means auto indenting Makefile and its pieces almost\n> impossible X-<.  I'll take the patch as there is no way to revert\n> the change to GNU make, though.\n\nI am considering whether to turn this error into a warning, for the\nnext release only, since it seems to be causing problems. I have not\ndecided for sure yet since the change is needed to avoid a very real\nparsing error (see the Savannah bug for details) which then could not\nbe fixed in this release.\n\nJust to note that this usage clearly contravenes the documentation,\nwhich states that preprocessor statement lines cannot begin with a TAB.\nIt was a bug that this was allowed by the GNU Make parser.\n\nI understand that in many projects (Linux, probably Git :)) if the\ndocumentation and behavior disagreed then the documentation would be\nchanged, not the behavior.\n\nI'd love to do that as well but unfortunately there's just no way to\nget coherent behavior out of GNU Make if this TAB prefix is allowed. \nIf the original authors of GNU Make had followed the lead of the BSD\nmake folks (or C) and used some reserved character to introduce\npreprocessor statements (BSD make uses \".if\"/\".else\" etc. which would\nwork) then we wouldn't be in this predicament. But make's parser is so\nad hoc that it's impossible to fix issues like this in a completely\nbackward-compatible manner.\n\nI know that the coding style in some projects is to use TAB for each\nlevel of indentation, but I do not think it's possible to extend that\nsame coding style from C into makefiles. In makefiles, a TAB is not\njust whitespace it's a meaningful token and has to be reserved for use\nonly in places where that meaning is required: to introduce a recipe\nline. Hopefully everyone's editors have the concept of separate modes\nfor different types of files, with different indentation styles for\neach.\n\nMy personal recommendation is that you do not take this patch as-is,\nand instead request a patch that converts every TAB character that\nindents a GNU Make preprocessor statement into TWO spaces rather than\n8. Or even THREE spaces (to avoid indentation that matches a TAB in\nwidth which could cause visual confusion). However of course that's up\nto you all.\n\n\nIf you wanted to make an even bigger change, which might save some\nhair-pulling down the road but is a very serious decision, you could\nintroduce the use of the .RECIPEPREFIX [1] variable to change the\nrecipe prefix from TAB to some other character (as it should have been\nwhen make was created back in the 1970's).\n\n.RECIPEPREFIX was introduced in GNU Make 3.82, which was released in\n2010 FYI.\n\n\nAgain, apologies for the churn... :(\n\n\n[1] https://www.gnu.org/software/make/manual/html_node/Special-Variables.html#index-_002eRECIPEPREFIX-_0028change-the-recipe-prefix-character_0029\n"},{"id":"492594","messageId":"xmqqpluz5t9j.fsf@gitster.g","threadId":"61293","inReplyTo":"xmqqle5n8rcr.fsf@gitster.g","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-08T23:28:24Z","receivedAt":"2024-04-08T23:28:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Taylor Blau writes:\n>\n>> When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or\n>> more tab characters, replace each tab character with 8 space characters\n>> with the following:\n>>\n>> find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |\n>> xargs perl -i -pe '\n>> s/(\\t+)(ifn?eq|ifn?def|else|endif)/\" \" x (length($1) * 8) . $2/ge unless /\\\\$/\n>> '\n>\n> Yuck, it means auto indenting Makefile and its pieces almost\n> impossible X-<. I'll take the patch as there is no way to revert\n> the change to GNU make, though.\n\nWe'd need something like this on top. Our top-level .gitattributes\ndefines the default whitespace rules with !indent-with-non-tab and\nenables indent-with-non-tab for specific file suffixes like .[ch],\nbut git-gui/.gitattributes enforces indent-with-non-tab for all\nfiles.\n\nAnother thing with this series (and this follow-up) is if we want to\nstart treating git-gui/ as just a subdirectory without no plan to\nfeed our changes to the \"upstream\". I am actually OK with that, as\nthe \"upstream\" we merge (with -Xsubtree merge strategy) from does\nnot seem to be very active and responsive these days.\n\n git-gui/.gitattributes | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git c/git-gui/.gitattributes w/git-gui/.gitattributes\nindex 59cd41dbff..118d56cfbd 100644\n--- c/git-gui/.gitattributes\n+++ w/git-gui/.gitattributes\n@@ -3,3 +3,4 @@\n git-gui.sh encoding=UTF-8\n /po/*.po encoding=UTF-8\n /GIT-VERSION-GEN eol=lf\n+Makefile whitespace=!indent,trail,space\n"},{"id":"492595","messageId":"xmqqh6gb5szm.fsf@gitster.g","threadId":"61293","inReplyTo":"606990048585347654f3b4b187ec27f4dc1b85e3.camel@gnu.org","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-08T23:34:21Z","receivedAt":"2024-04-08T23:34:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Smith writes:\n\n> Just to note that this usage clearly contravenes the documentation,\n> which states that preprocessor statement lines cannot begin with a TAB.\n> It was a bug that this was allowed by the GNU Make parser.\n>\n> I understand that in many projects (Linux, probably Git :)) if the\n> documentation and behavior disagreed then the documentation would be\n> changed, not the behavior.\n\nIf a bug is left in a released version long enough, it becomes a\nfeature your users depend upon. We saw that happen to us, I am sure\nthe mantra \"don't break userspace\" the kernel project had comes from\nthe same place.\n\nI am not sure what benefits are gained by the existing users with\nthis change to ease fixing some parser bug (I didn't bother to see\nyour bug tracker) so I cannot judge if the benefit outweighs the\ncost of them all having to scramble and adjust to the new world\norder.\n\n"},{"id":"492596","messageId":"xmqqcyqz5sie.fsf@gitster.g","threadId":"61293","inReplyTo":"606990048585347654f3b4b187ec27f4dc1b85e3.camel@gnu.org","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-08T23:44:41Z","receivedAt":"2024-04-08T23:44:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Smith writes:\n\n> I'd love to do that as well but unfortunately there's just no way to\n> get coherent behavior out of GNU Make if this TAB prefix is allowed. \n> If the original authors of GNU Make had followed the lead of the BSD\n> make folks (or C) and used some reserved character to introduce\n> preprocessor statements (BSD make uses \".if\"/\".else\" etc. which would\n> work) then we wouldn't be in this predicament. But make's parser is so\n> ad hoc that it's impossible to fix issues like this in a completely\n> backward-compatible manner.\n\nI wonder if you could ease the transition by leaving the current\nparsing rule for conditional constructs that are indented with HT\nand clearly mark them as \"works as best-effort basis---the parsing\nbug for them may remain\", introduce BSD compatible .if/.else and\nfriends, and nudge the users in that direction.\n\nHaving to use two different indentation style in the same Makefile\nis simply a nightmare, and that might be a good enough incentive for\nusers to move to the new \"you can write with dots like .if and that\nway you can continue indenting with HT\".\n"},{"id":"492597","messageId":"20240409000414.GA1647304@coredump.intra.peff.net","threadId":"61293","inReplyTo":"606990048585347654f3b4b187ec27f4dc1b85e3.camel@gnu.org","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-04-09T00:04:14Z","receivedAt":"2024-04-09T00:04:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 08, 2024 at 07:24:16PM -0400, Paul Smith wrote:\n\n> If you wanted to make an even bigger change, which might save some\n> hair-pulling down the road but is a very serious decision, you could\n> introduce the use of the .RECIPEPREFIX [1] variable to change the\n> recipe prefix from TAB to some other character (as it should have been\n> when make was created back in the 1970's).\n> \n> .RECIPEPREFIX was introduced in GNU Make 3.82, which was released in\n> 2010 FYI.\n\nUnfortunately, that's too recent for us. :( We try to keep the GNU make\ndependency to 3.81, since that's the latest one Apple ships (because\nthey're allergic to GPLv3). Obviously it's not impossible to jump past\nthat and require people to install make via homebrew, etc, but that\nmakes it a more significant version bump than usual.\n\nI do find it curious that in:\n\nifdef FOO\n\tSOME_VAR += bar\nendif\n\nthe tab is significant for \"ifdef\" but not for SOME_VAR (at least that\nis implied by Taylor's patch, which does not touch the bodies within the\nconditionals).\n\nI may just be showing my ignorance of the parsing issue, though. For\nanybody else digging into the details, I think the correct link is:\n\n https://savannah.gnu.org/bugs/index.php?64185\n\n(the commit has the wrong bug number, 64815).\n\n-Peff\n"},{"id":"492600","messageId":"20240409001728.GB1647304@coredump.intra.peff.net","threadId":"61293","inReplyTo":"20240409000414.GA1647304@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-04-09T00:17:28Z","receivedAt":"2024-04-09T00:17:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 08, 2024 at 08:04:14PM -0400, Jeff King wrote:\n\n> I do find it curious that in:\n> \n> ifdef FOO\n> \tSOME_VAR += bar\n> endif\n> \n> the tab is significant for \"ifdef\" but not for SOME_VAR (at least that\n> is implied by Taylor's patch, which does not touch the bodies within the\n> conditionals).\n> \n> I may just be showing my ignorance of the parsing issue, though. For\n> anybody else digging into the details, I think the correct link is:\n> \n> https://savannah.gnu.org/bugs/index.php?64185\n> \n> (the commit has the wrong bug number, 64815).\n\nAnswering my own question (at least what I think the answer is): there's\nbasically two levels of parsing going on. The outer layer is respecting\nconditionals to decide which lines to care about at all, and the inner\none is figuring what are assignments, rules, recipes, etc.\n\nSo the outer parser cares about things that look like conditionals, but\nnothing else. The inner one has more context and can more easily realize\nthat \"\\tSOME_VAR += bar\" is not part of a recipe.\n\nI'd guess it's _possible_ to fix the case discussed in the bug by\nletting the outer parser know more of the inner-parser context (i.e.,\nwhatever rules it uses to decide that the assignment line is not a\nrecipe line could similarly be used for a line like \"\\telse\"). But I\nalso wouldn't be at all surprised if it would involve a substantial\nrewrite. At any rate, I'd certainly defer to you on such matters. I'm\nmostly just thinking out loud from my peanut-gallery perspective.\n\n-Peff\n"},{"id":"492647","messageId":"2b392b30614abd9a110515448853aa43eac42d8b.camel@gnu.org","threadId":"61293","inReplyTo":"xmqqh6gb5szm.fsf@gitster.g","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Paul Smith","fromEmail":"psmith@gnu.org","sentAt":"2024-04-09T20:41:56Z","receivedAt":"2024-04-09T20:42:05Z","isPatch":true,"sender":{"key":"psmith@gnu.org","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Mon, 2024-04-08 at 16:34 -0700, Junio C Hamano wrote:\n> I am not sure what benefits are gained by the existing users with\n> this change to ease fixing some parser bug (I didn't bother to see\n> your bug tracker) so I cannot judge if the benefit outweighs the\n> cost of them all having to scramble and adjust to the new world\n> order.\n\nJust to point out that it's actually unusual (in my experience) for\nmakefiles to use TAB as indentation. There are so many situations\nwhere this comes back to bite you (see my other emails) that people\nsimply don't do it.\n\nI realize Git and Linux (using Linus's coding style) are committed to\neach-TAB-is-one-indentation-level, even insofar as using it inside\nmakefiles not just C code, but they are outliers IME.\n\nSo I'm not sure I'm ready to concede (yet) that the \"cost of them all\"\nis actually very wide. Certainly two projects as popular as Git and\nLinux with this problem are very concerning.\n"},{"id":"492648","messageId":"95f2454e449cc0126aaa40d2ab08c76b55ee3c31.camel@gnu.org","threadId":"61293","inReplyTo":"xmqqcyqz5sie.fsf@gitster.g","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Paul Smith","fromEmail":"psmith@gnu.org","sentAt":"2024-04-09T20:42:14Z","receivedAt":"2024-04-09T20:42:16Z","isPatch":true,"sender":{"key":"psmith@gnu.org","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Mon, 2024-04-08 at 16:44 -0700, Junio C Hamano wrote:\n> Paul Smith writes:\n> \n> > I'd love to do that as well but unfortunately there's just no way\n> > to get coherent behavior out of GNU Make if this TAB prefix is\n> > allowed.\n> \n> I wonder if you could ease the transition by leaving the current\n> parsing rule for conditional constructs that are indented with HT\n> and clearly mark them as \"works as best-effort basis---the parsing\n> bug for them may remain\",\n\nI'm not sure I understand the suggestion here. If I preserve the\ncurrent parsing behavior what do I tell people who cannot get their\nmakefiles to work because the current parsing doesn't allow it?\n\n> introduce BSD compatible .if/.else and friends, and nudge the users\n> in that direction.\n> \n> Having to use two different indentation style in the same Makefile\n> is simply a nightmare, and that might be a good enough incentive for\n> users to move to the new \"you can write with dots like .if and that\n> way you can continue indenting with HT\".\n\nI agree that it's a nightmare, but IMO trying to continue to work\naround this terrible original sin in make (using TAB as a non-\nwhitespace token) by adding new keywords is the wrong direction.\n\nThe right direction is to STOP using TAB as a special token and turn it\nback into what it is in other languages: simple whitespace.\n\nThat was already accomplished back in 2010 in GNU Make 3.82 with the\nintroduction of the .RECIPEPREFIX variable.\n\n"},{"id":"492649","messageId":"daa51548ff2d0cfc4407bbdbe99223c84321a503.camel@gnu.org","threadId":"61293","inReplyTo":"20240409000414.GA1647304@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Paul Smith","fromEmail":"psmith@gnu.org","sentAt":"2024-04-09T20:44:45Z","receivedAt":"2024-04-09T20:45:11Z","isPatch":true,"sender":{"key":"psmith@gnu.org","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Mon, 2024-04-08 at 20:04 -0400, Jeff King wrote:\n> > .RECIPEPREFIX was introduced in GNU Make 3.82, which was released\n> > in 2010 FYI.\n> \n> Unfortunately, that's too recent for us. :( We try to keep the GNU\n> make dependency to 3.81, since that's the latest one Apple ships\n> (because they're allergic to GPLv3).\n\nI understand that position, for Git. I hope you can understand that in\nmy position I have no interest in catering to Apple's ridiculous\ncorporate antics and can only shrug and say \"oh well\" :)\n\n> I do find it curious that in:\n> \n> ifdef FOO\n>  SOME_VAR += bar\n> endif\n> \n> the tab is significant for \"ifdef\" but not for SOME_VAR (at least\n> that is implied by Taylor's patch, which does not touch the bodies\n> within the conditionals).\n\nThe handling of TAB in makefiles is actually more subtle than the\nsimple \"if it starts with a TAB it's part of a recipe\". The full\nstatement is, \"if it starts with a TAB _and is in a recipe context_\nthen it's part of a recipe\".\n\nA recipe context starts after a target is defined, and it ends only\nwhen the first non-TAB-indented, non-comment line is parsed (or EOF).\n\nThe text above will work ONLY if the content BEFORE that text is not a\nrecipe. If you add a recipe before that line, then the SOME_VAR += bar\nwill suddenly be considered by make as part of that recipe and will be\nan error when you run that recipe (since that's not valid shell\nsyntax).\n\nSo for example if you have:\n\n $ cat Makefile\n\n # set up the variable SOME_VAR\n ifndef TRUE\n SOME_VAR = bar\n endif\n\n all: ; echo $(SOME_VAR)\n\nThis is fine. But if you then modify your makefile like this:\n\n $ cat Makefile\n\n recurse: ; $(MAKE) -C subdir\n\n # set up the variable SOME_VAR\n ifndef TRUE\n SOME_VAR = bar\n endif\n\n all: ; echo $(SOME_VAR)\n\nIt LOOKS fine but it's completely broken because the SOME_VAR\nassignment now becomes part of the recipe for the recurse target.\n\n (Just to note, this is not due to a recent change in GNU Make, and\n in fact this is not even specific to GNU Make: all versions of make\n behave like this.)\n\nSo while the patch proposed here does not remove all TAB characters, my\nbest advice is that as a project you SHOULD consider pro-actively\nremoving all non-recipe-introducing TAB characters. They are dangerous\nand misleading, even outside of this ifdef kerfuffle.\n\n\nAll I can do is reiterate my original statement: it's a bad idea to\nconsider TAB characters as whitespace or \"just indentation\" AT ALL when\nediting makefiles. TABs are not whitespace. They are meaningful\ntokens, like \"$\" or \"#\", and should only ever be used in places where\nthat meaning is desired. The fact that they look like indentation and\ncan be used in some other places as \"just indentation\" and kinda-sorta\nwork, is an accident waiting to happen if it's taken advantage of.\n"},{"id":"492650","messageId":"xmqqil0q1b8f.fsf@gitster.g","threadId":"61293","inReplyTo":"95f2454e449cc0126aaa40d2ab08c76b55ee3c31.camel@gnu.org","subject":"Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-09T21:23:44Z","receivedAt":"2024-04-09T21:23:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Smith writes:\n\n> I'm not sure I understand the suggestion here. If I preserve the\n> current parsing behavior what do I tell people who cannot get their\n> makefiles to work because the current parsing doesn't allow it?\n\nTheir Makefiles were not working (perhaps began with HT followed by\na command whose name happened to be \"ifdef\" installed in ~/bin/ifdef\nor something silly like that) before your \"fix\" to forbid using HT\nto indent conditional, so your \"fix\" is not breaking them any\nfurther.\n\nIf you optionally allow .if/.else etc., you can tell them to replace\ntheir \"ifdef\" with \".ifdef\". Of course you can also tell them to\nreplace their HT indent before \"ifdef\" to spaces.\n\nBut the point is that those whose Makefiles were not parsed correctly\neven before your \"fix\" need to fix their Makefiles anyway. The\nsuggestion was about helping those whose Makefiles were happily been\ngrokked somehow before your \"fix\". If you preserve the current code,\ntheir Makefiles that indent their \"ifdef\" with HT will continue to\nwork, so you do not have to tell them anything, no?\n"}]}