Makefiles are broken as of GNU Make commit 07fcee35f058a876447c8a021f9eb1943f902534

Makefiles are broken as of GNU Make commit 07fcee35f058a876447c8a021f9eb1943f902534

[Dario Gjorgjevski]

Hi,

Git has plenty of conditional statements in its Makefiles which begin
with the recipe prefix (tab).  This used to work in the past, but no
longer does as of GNU Make commit
07fcee35f058a876447c8a021f9eb1943f902534:
https://git.savannah.gnu.org/cgit/make.git/commit/?id=07fcee35f058a876447c8a021f9eb1943f902534.
This commit has not yet landed in a release, but it probably will in
the future.

A similar bug was recently fixed in the Linux kernel:
https://github.com/torvalds/linux/commit/82175d1f9430d5a026e2231782d13da0bf57155c.

Best regards,
Dario

[PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Taylor Blau]

In GNU Make commit 07fcee35 ([SV 64815] Recipe lines cannot contain
conditional statements, 2023-05-22) and following, conditional
statements may no longer be preceded by a tab character (which Make
refers to as the recipe prefix).

There are a handful of spots in our various Makefile(s) which will break
in a future release of Make containing 07fcee35. For instance, trying to
compile the pre-image of this patch with the tip of make.git results in
the following:

    $ make -v | head -1 && make
    GNU Make 4.4.90
    config.mak.uname:842: *** missing 'endif'.  Stop.

The kernel addressed this issue in 82175d1f9430 (kbuild: Replace tabs
with spaces when followed by conditionals, 2024-01-28). Address the
issues in Git's tree by applying the same strategy.

When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
more tab characters, replace each tab character with 8 space characters
with the following:

    find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |
      xargs perl -i -pe '
        s/(\t+)(ifn?eq|ifn?def|else|endif)/" " x (length($1) * 8) . $2/ge unless /\\$/
      '

The "unless /\\$/" removes any false-positives (like "\telse \"
appearing within a shell script as part of a recipe).

After doing so, Git compiles on newer versions of Make:

    $ make -v | head -1 && make
    GNU Make 4.4.90
    GIT_VERSION = 2.44.0.414.gfac1dc44ca9
    [...]

    $ echo $?
    0

Reported-by: Dario Gjorgjevski <dario.gjorgjevski@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Makefile          |  96 ++++++++++++++---------------
 config.mak.uname  | 154 +++++++++++++++++++++++-----------------------
 git-gui/Makefile  |  16 ++---
 gitk-git/Makefile |   4 +-
 4 files changed, 135 insertions(+), 135 deletions(-)

diff --git a/Makefile b/Makefile
index c43c1bd1a05..ac603fb7678 100644
--- a/Makefile
+++ b/Makefile
@@ -1557,23 +1557,23 @@ ifneq (,$(SOCKLEN_T))
 endif
 
 ifeq ($(uname_S),Darwin)
-	ifndef NO_FINK
-		ifeq ($(shell test -d /sw/lib && echo y),y)
+        ifndef NO_FINK
+                ifeq ($(shell test -d /sw/lib && echo y),y)
 			BASIC_CFLAGS += -I/sw/include
 			BASIC_LDFLAGS += -L/sw/lib
-		endif
-	endif
-	ifndef NO_DARWIN_PORTS
-		ifeq ($(shell test -d /opt/local/lib && echo y),y)
+                endif
+        endif
+        ifndef NO_DARWIN_PORTS
+                ifeq ($(shell test -d /opt/local/lib && echo y),y)
 			BASIC_CFLAGS += -I/opt/local/include
 			BASIC_LDFLAGS += -L/opt/local/lib
-		endif
-	endif
-	ifndef NO_APPLE_COMMON_CRYPTO
+                endif
+        endif
+        ifndef NO_APPLE_COMMON_CRYPTO
 		NO_OPENSSL = YesPlease
 		APPLE_COMMON_CRYPTO = YesPlease
 		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
-	endif
+        endif
 	PTHREAD_LIBS =
 endif
 
@@ -1612,23 +1612,23 @@ ifdef NO_CURL
 	REMOTE_CURL_NAMES =
 	EXCLUDED_PROGRAMS += git-http-fetch git-http-push
 else
-	ifdef CURLDIR
+        ifdef CURLDIR
 		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
 		CURL_CFLAGS = -I$(CURLDIR)/include
 		CURL_LIBCURL = $(call libpath_template,$(CURLDIR)/$(lib))
-	else
+        else
 		CURL_CFLAGS =
 		CURL_LIBCURL =
-	endif
+        endif
 
-	ifndef CURL_LDFLAGS
+        ifndef CURL_LDFLAGS
 		CURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)
-	endif
+        endif
 	CURL_LIBCURL += $(CURL_LDFLAGS)
 
-	ifndef CURL_CFLAGS
+        ifndef CURL_CFLAGS
 		CURL_CFLAGS = $(eval CURL_CFLAGS := $$(shell $$(CURL_CONFIG) --cflags))$(CURL_CFLAGS)
-	endif
+        endif
 	BASIC_CFLAGS += $(CURL_CFLAGS)
 
 	REMOTE_CURL_PRIMARY = git-remote-http$X
@@ -1636,29 +1636,29 @@ else
 	REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
 	PROGRAM_OBJS += http-fetch.o
 	PROGRAMS += $(REMOTE_CURL_NAMES)
-	ifndef NO_EXPAT
+        ifndef NO_EXPAT
 		PROGRAM_OBJS += http-push.o
-	endif
+        endif
 	curl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
-	ifeq "$(curl_check)" "072200"
+        ifeq "$(curl_check)" "072200"
 		USE_CURL_FOR_IMAP_SEND = YesPlease
-	endif
-	ifdef USE_CURL_FOR_IMAP_SEND
+        endif
+        ifdef USE_CURL_FOR_IMAP_SEND
 		BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
 		IMAP_SEND_BUILDDEPS = http.o
 		IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
-	endif
-	ifndef NO_EXPAT
-		ifdef EXPATDIR
+        endif
+        ifndef NO_EXPAT
+                ifdef EXPATDIR
 			BASIC_CFLAGS += -I$(EXPATDIR)/include
 			EXPAT_LIBEXPAT = $(call libpath_template,$(EXPATDIR)/$(lib)) -lexpat
-		else
+                else
 			EXPAT_LIBEXPAT = -lexpat
-		endif
-		ifdef EXPAT_NEEDS_XMLPARSE_H
+                endif
+                ifdef EXPAT_NEEDS_XMLPARSE_H
 			BASIC_CFLAGS += -DEXPAT_NEEDS_XMLPARSE_H
-		endif
-	endif
+                endif
+        endif
 endif
 IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
 
@@ -1670,15 +1670,15 @@ EXTLIBS += -lz
 
 ifndef NO_OPENSSL
 	OPENSSL_LIBSSL = -lssl
-	ifdef OPENSSLDIR
+        ifdef OPENSSLDIR
 		BASIC_CFLAGS += -I$(OPENSSLDIR)/include
 		OPENSSL_LINK = $(call libpath_template,$(OPENSSLDIR)/$(lib))
-	else
+        else
 		OPENSSL_LINK =
-	endif
-	ifdef NEEDS_CRYPTO_WITH_SSL
+        endif
+        ifdef NEEDS_CRYPTO_WITH_SSL
 		OPENSSL_LIBSSL += -lcrypto
-	endif
+        endif
 else
 	BASIC_CFLAGS += -DNO_OPENSSL
 	OPENSSL_LIBSSL =
@@ -1696,18 +1696,18 @@ ifdef APPLE_COMMON_CRYPTO
 endif
 endif
 ifndef NO_ICONV
-	ifdef NEEDS_LIBICONV
-		ifdef ICONVDIR
+        ifdef NEEDS_LIBICONV
+                ifdef ICONVDIR
 			BASIC_CFLAGS += -I$(ICONVDIR)/include
 			ICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib))
-		else
+                else
 			ICONV_LINK =
-		endif
-		ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
+                endif
+                ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
 			ICONV_LINK += -lintl
-		endif
+                endif
 		EXTLIBS += $(ICONV_LINK) -liconv
-	endif
+        endif
 endif
 ifdef ICONV_OMITS_BOM
 	BASIC_CFLAGS += -DICONV_OMITS_BOM
@@ -1828,10 +1828,10 @@ ifdef NO_MMAP
 	COMPAT_CFLAGS += -DNO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 else
-	ifdef USE_WIN32_MMAP
+        ifdef USE_WIN32_MMAP
 		COMPAT_CFLAGS += -DUSE_WIN32_MMAP
 		COMPAT_OBJS += compat/win32mmap.o
-	endif
+        endif
 endif
 ifdef MMAP_PREVENTS_DELETE
 	BASIC_CFLAGS += -DMMAP_PREVENTS_DELETE
@@ -1956,11 +1956,11 @@ else
 	BASIC_CFLAGS += -DSHA1_DC
 	LIB_OBJS += sha1dc_git.o
 ifdef DC_SHA1_EXTERNAL
-	ifdef DC_SHA1_SUBMODULE
-		ifneq ($(DC_SHA1_SUBMODULE),auto)
+        ifdef DC_SHA1_SUBMODULE
+                ifneq ($(DC_SHA1_SUBMODULE),auto)
 $(error Only set DC_SHA1_EXTERNAL or DC_SHA1_SUBMODULE, not both)
-		endif
-	endif
+                endif
+        endif
 	BASIC_CFLAGS += -DDC_SHA1_EXTERNAL
 	EXTLIBS += -lsha1detectcoll
 else
diff --git a/config.mak.uname b/config.mak.uname
index d0dcca2ec55..b11408e67b4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -65,9 +65,9 @@ ifeq ($(uname_S),Linux)
 	HAVE_PLATFORM_PROCINFO = YesPlease
 	COMPAT_OBJS += compat/linux/procinfo.o
 	# centos7/rhel7 provides gcc 4.8.5 and zlib 1.2.7.
-	ifneq ($(findstring .el7.,$(uname_R)),)
+        ifneq ($(findstring .el7.,$(uname_R)),)
 		BASIC_CFLAGS += -std=c99
-	endif
+        endif
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
@@ -95,13 +95,13 @@ ifeq ($(uname_S),UnixWare)
 	NO_MEMMEM = YesPlease
 endif
 ifeq ($(uname_S),SCO_SV)
-	ifeq ($(uname_R),3.2)
+        ifeq ($(uname_R),3.2)
 		CFLAGS = -O2
-	endif
-	ifeq ($(uname_R),5)
+        endif
+        ifeq ($(uname_R),5)
 		CC = cc
 		BASIC_CFLAGS += -Kthread
-	endif
+        endif
 	NEEDS_SOCKET = YesPlease
 	NEEDS_NSL = YesPlease
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
@@ -124,19 +124,19 @@ ifeq ($(uname_S),Darwin)
 	# - MacOS 10.0.* and MacOS 10.1.0 = Darwin 1.*
 	# - MacOS 10.x.* = Darwin (x+4).* for (1 <= x)
 	# i.e. "begins with [15678] and a dot" means "10.4.* or older".
-	ifeq ($(shell expr "$(uname_R)" : '[15678]\.'),2)
+        ifeq ($(shell expr "$(uname_R)" : '[15678]\.'),2)
 		OLD_ICONV = UnfortunatelyYes
 		NO_APPLE_COMMON_CRYPTO = YesPlease
-	endif
-	ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
+        endif
+        ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
 		NO_STRLCPY = YesPlease
-	endif
-	ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 11 && echo 1),1)
+        endif
+        ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 11 && echo 1),1)
 		HAVE_GETDELIM = YesPlease
-	endif
-	ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 20 && echo 1),1)
+        endif
+        ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 20 && echo 1),1)
 		OPEN_RETURNS_EINTR = UnfortunatelyYes
-	endif
+        endif
 	NO_MEMMEM = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	HAVE_DEV_TTY = YesPlease
@@ -152,12 +152,12 @@ ifeq ($(uname_S),Darwin)
 	# Workaround for `gettext` being keg-only and not even being linked via
 	# `brew link --force gettext`, should be obsolete as of
 	# https://github.com/Homebrew/homebrew-core/pull/53489
-	ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+        ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
 		BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
 		BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
-		ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+                ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
 			MSGFMT = /usr/local/opt/gettext/bin/msgfmt
-		endif
+                endif
 	# On newer ARM-based machines the default installation path has changed to
 	# /opt/homebrew. Include it in our search paths so that the user does not
 	# have to configure this manually.
@@ -165,22 +165,22 @@ ifeq ($(uname_S),Darwin)
 	# Note that we do not employ the same workaround as above where we manually
 	# add gettext. The issue was fixed more than three years ago by now, and at
 	# that point there haven't been any ARM-based Macs yet.
-	else ifeq ($(shell test -d /opt/homebrew/ && echo y),y)
+        else ifeq ($(shell test -d /opt/homebrew/ && echo y),y)
 		BASIC_CFLAGS += -I/opt/homebrew/include
 		BASIC_LDFLAGS += -L/opt/homebrew/lib
-		ifeq ($(shell test -x /opt/homebrew/bin/msgfmt && echo y),y)
+                ifeq ($(shell test -x /opt/homebrew/bin/msgfmt && echo y),y)
 			MSGFMT = /opt/homebrew/bin/msgfmt
-		endif
-	endif
+                endif
+        endif
 
 	# The builtin FSMonitor on MacOS builds upon Simple-IPC.  Both require
 	# Unix domain sockets and PThreads.
-	ifndef NO_PTHREADS
-	ifndef NO_UNIX_SOCKETS
+        ifndef NO_PTHREADS
+        ifndef NO_UNIX_SOCKETS
 	FSMONITOR_DAEMON_BACKEND = darwin
 	FSMONITOR_OS_SETTINGS = darwin
-	endif
-	endif
+        endif
+        endif
 
 	BASIC_LDFLAGS += -framework CoreServices
 endif
@@ -196,7 +196,7 @@ ifeq ($(uname_S),SunOS)
 	NO_REGEX = YesPlease
 	NO_MSGFMT_EXTENDED_OPTIONS = YesPlease
 	HAVE_DEV_TTY = YesPlease
-	ifeq ($(uname_R),5.6)
+        ifeq ($(uname_R),5.6)
 		SOCKLEN_T = int
 		NO_HSTRERROR = YesPlease
 		NO_IPV6 = YesPlease
@@ -206,8 +206,8 @@ ifeq ($(uname_S),SunOS)
 		NO_STRLCPY = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
-	endif
-	ifeq ($(uname_R),5.7)
+        endif
+        ifeq ($(uname_R),5.7)
 		NEEDS_RESOLV = YesPlease
 		NO_IPV6 = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
@@ -216,25 +216,25 @@ ifeq ($(uname_S),SunOS)
 		NO_STRLCPY = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
-	endif
-	ifeq ($(uname_R),5.8)
+        endif
+        ifeq ($(uname_R),5.8)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
-	endif
-	ifeq ($(uname_R),5.9)
+        endif
+        ifeq ($(uname_R),5.9)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
-	endif
+        endif
 	INSTALL = /usr/ucb/install
 	TAR = gtar
 	BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
 endif
 ifeq ($(uname_O),Cygwin)
-	ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
+        ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
 		NO_D_TYPE_IN_DIRENT = YesPlease
 		NO_STRCASESTR = YesPlease
 		NO_MEMMEM = YesPlease
@@ -245,9 +245,9 @@ ifeq ($(uname_O),Cygwin)
 		# On some boxes NO_MMAP is needed, and not so elsewhere.
 		# Try commenting this out if you suspect MMAP is more efficient
 		NO_MMAP = YesPlease
-	else
+        else
 		NO_REGEX = UnfortunatelyYes
-	endif
+        endif
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
@@ -263,25 +263,25 @@ ifeq ($(uname_S),FreeBSD)
 	NEEDS_LIBICONV = YesPlease
 	# Versions up to 10.1 require OLD_ICONV; 10.2 and beyond don't.
 	# A typical version string looks like "10.2-RELEASE".
-	ifeq ($(shell expr "$(uname_R)" : '[1-9]\.'),2)
+        ifeq ($(shell expr "$(uname_R)" : '[1-9]\.'),2)
 		OLD_ICONV = YesPlease
-	endif
-	ifeq ($(firstword $(subst -, ,$(uname_R))),10.0)
+        endif
+        ifeq ($(firstword $(subst -, ,$(uname_R))),10.0)
 		OLD_ICONV = YesPlease
-	endif
-	ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
+        endif
+        ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
 		OLD_ICONV = YesPlease
-	endif
+        endif
 	NO_MEMMEM = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
+        ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
 		PTHREAD_LIBS = -pthread
 		NO_UINTMAX_T = YesPlease
 		NO_STRTOUMAX = YesPlease
-	endif
+        endif
 	PYTHON_PATH = /usr/local/bin/python
 	PERL_PATH = /usr/local/bin/perl
 	HAVE_PATHS_H = YesPlease
@@ -317,9 +317,9 @@ ifeq ($(uname_S),MirBSD)
 	CSPRNG_METHOD = arc4random
 endif
 ifeq ($(uname_S),NetBSD)
-	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
+        ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
 		NEEDS_LIBICONV = YesPlease
-	endif
+        endif
 	BASIC_CFLAGS += -I/usr/pkg/include
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
 	USE_ST_TIMESPEC = YesPlease
@@ -343,14 +343,14 @@ ifeq ($(uname_S),AIX)
 	BASIC_CFLAGS += -D_LARGE_FILES
 	FILENO_IS_A_MACRO = UnfortunatelyYes
 	NEED_ACCESS_ROOT_HANDLER = UnfortunatelyYes
-	ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
+        ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
 		NO_PTHREADS = YesPlease
-	else
+        else
 		PTHREAD_LIBS = -lpthread
-	endif
-	ifeq ($(shell expr "$(uname_V).$(uname_R)" : '5\.1'),3)
+        endif
+        ifeq ($(shell expr "$(uname_V).$(uname_R)" : '5\.1'),3)
 		INLINE = ''
-	endif
+        endif
 	GIT_TEST_CMP = cmp
 endif
 ifeq ($(uname_S),GNU)
@@ -410,29 +410,29 @@ ifeq ($(uname_S),HP-UX)
 	NO_SYS_SELECT_H = YesPlease
 	SNPRINTF_RETURNS_BOGUS = YesPlease
 	NO_NSEC = YesPlease
-	ifeq ($(uname_R),B.11.00)
+        ifeq ($(uname_R),B.11.00)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
-	endif
-	ifeq ($(uname_R),B.10.20)
+        endif
+        ifeq ($(uname_R),B.10.20)
 		# Override HP-UX 11.x setting:
 		INLINE =
 		SOCKLEN_T = size_t
 		NO_PREAD = YesPlease
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
-	endif
+        endif
 	GIT_TEST_CMP = cmp
 endif
 ifeq ($(uname_S),Windows)
 	GIT_VERSION := $(GIT_VERSION).MSVC
 	pathsep = ;
 	# Assume that this is built in Git for Windows' SDK
-	ifeq (MINGW32,$(MSYSTEM))
+        ifeq (MINGW32,$(MSYSTEM))
 		prefix = /mingw32
-	else
+        else
 		prefix = /mingw64
-	endif
+        endif
 	# Prepend MSVC 64-bit tool-chain to PATH.
 	#
 	# A regular Git Bash *does not* have cl.exe in its $PATH. As there is a
@@ -550,16 +550,16 @@ ifeq ($(uname_S),Interix)
 	NO_MKDTEMP = YesPlease
 	NO_STRTOUMAX = YesPlease
 	NO_NSEC = YesPlease
-	ifeq ($(uname_R),3.5)
+        ifeq ($(uname_R),3.5)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
-	endif
-	ifeq ($(uname_R),5.2)
+        endif
+        ifeq ($(uname_R),5.2)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
-	endif
+        endif
 endif
 ifeq ($(uname_S),Minix)
 	NO_IPV6 = YesPlease
@@ -579,12 +579,12 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	# still not compile in c89 mode, due to non-const array initializations.
 	CC = cc -c99
 	# Build down-rev compatible objects that don't use our new getopt_long.
-	ifeq ($(uname_R).$(uname_V),J06.21)
+        ifeq ($(uname_R).$(uname_V),J06.21)
 		CC += -WRVU=J06.20
-	endif
-	ifeq ($(uname_R).$(uname_V),L17.02)
+        endif
+        ifeq ($(uname_R).$(uname_V),L17.02)
 		CC += -WRVU=L16.05
-	endif
+        endif
 	# Disable all optimization, seems to result in bad code, with -O or -O2
 	# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects
 	# abends on "git push". Needs more investigation.
@@ -651,9 +651,9 @@ ifeq ($(uname_S),OS/390)
 	NEEDS_MODE_TRANSLATION = YesPlease
 endif
 ifeq ($(uname_S),MINGW)
-	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
+        ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
 		$(error "Building with MSys is no longer supported")
-	endif
+        endif
 	pathsep = ;
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease
@@ -712,22 +712,22 @@ ifeq ($(uname_S),MINGW)
 	# Enable DEP
 	BASIC_LDFLAGS += -Wl,--nxcompat
 	# Enable ASLR (unless debugging)
-	ifneq (,$(findstring -O,$(filter-out -O0 -Og,$(CFLAGS))))
+        ifneq (,$(findstring -O,$(filter-out -O0 -Og,$(CFLAGS))))
 		BASIC_LDFLAGS += -Wl,--dynamicbase
-	endif
-	ifeq (MINGW32,$(MSYSTEM))
+        endif
+        ifeq (MINGW32,$(MSYSTEM))
 		prefix = /mingw32
 		HOST_CPU = i686
 		BASIC_LDFLAGS += -Wl,--pic-executable,-e,_mainCRTStartup
-	endif
-	ifeq (MINGW64,$(MSYSTEM))
+        endif
+        ifeq (MINGW64,$(MSYSTEM))
 		prefix = /mingw64
 		HOST_CPU = x86_64
 		BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup
-	else
+        else
 		COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
 		BASIC_LDFLAGS += -Wl,--large-address-aware
-	endif
+        endif
 	CC = gcc
 	COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \
 		-fstack-protector-strong
@@ -739,11 +739,11 @@ ifeq ($(uname_S),MINGW)
 	USE_GETTEXT_SCHEME = fallthrough
 	USE_LIBPCRE = YesPlease
 	USE_NED_ALLOCATOR = YesPlease
-	ifeq (/mingw64,$(subst 32,64,$(prefix)))
+        ifeq (/mingw64,$(subst 32,64,$(prefix)))
 		# Move system config into top-level /etc/
 		ETC_GITCONFIG = ../etc/gitconfig
 		ETC_GITATTRIBUTES = ../etc/gitattributes
-	endif
+        endif
 endif
 ifeq ($(uname_S),QNX)
 	COMPAT_CFLAGS += -DSA_RESTART=0
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 3f80435436c..667c39ed564 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -107,12 +107,12 @@ endif
 
 ifeq ($(uname_S),Darwin)
 	TKFRAMEWORK = /Library/Frameworks/Tk.framework/Resources/Wish.app
-	ifeq ($(shell echo "$(uname_R)" | awk -F. '{if ($$1 >= 9) print "y"}')_$(shell test -d $(TKFRAMEWORK) || echo n),y_n)
+        ifeq ($(shell echo "$(uname_R)" | awk -F. '{if ($$1 >= 9) print "y"}')_$(shell test -d $(TKFRAMEWORK) || echo n),y_n)
 		TKFRAMEWORK = /System/Library/Frameworks/Tk.framework/Resources/Wish.app
-		ifeq ($(shell test -d $(TKFRAMEWORK) || echo n),n)
+                ifeq ($(shell test -d $(TKFRAMEWORK) || echo n),n)
 			TKFRAMEWORK = /System/Library/Frameworks/Tk.framework/Resources/Wish\ Shell.app
-		endif
-	endif
+                endif
+        endif
 	TKEXECUTABLE = $(shell basename "$(TKFRAMEWORK)" .app)
 endif
 
@@ -143,9 +143,9 @@ ifeq ($(exedir),$(gg_libdir))
 endif
 gg_libdir_sed_in := $(gg_libdir)
 ifeq ($(uname_S),Darwin)
-	ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)
+        ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)
 		GITGUI_MACOSXAPP := YesPlease
-	endif
+        endif
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
 ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
@@ -220,9 +220,9 @@ ifdef NO_MSGFMT
 	MSGFMT ?= $(TCL_PATH) po/po2msg.sh
 else
 	MSGFMT ?= msgfmt
-	ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
+        ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
 		MSGFMT := $(TCL_PATH) po/po2msg.sh
-	endif
+        endif
 endif
 
 msgsdir     = $(gg_libdir)/msgs
diff --git a/gitk-git/Makefile b/gitk-git/Makefile
index 5bdd52a6ebf..e1f0aff4a19 100644
--- a/gitk-git/Makefile
+++ b/gitk-git/Makefile
@@ -33,9 +33,9 @@ ifdef NO_MSGFMT
 	MSGFMT ?= $(TCL_PATH) po/po2msg.sh
 else
 	MSGFMT ?= msgfmt
-	ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
+        ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
 		MSGFMT := $(TCL_PATH) po/po2msg.sh
-	endif
+        endif
 endif
 
 PO_TEMPLATE = po/gitk.pot
-- 
2.44.0.414.g38c9f58cd0c

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
> more tab characters, replace each tab character with 8 space characters
> with the following:
>
>     find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |
>       xargs perl -i -pe '
>         s/(\t+)(ifn?eq|ifn?def|else|endif)/" " x (length($1) * 8) . $2/ge unless /\\$/
>       '

Yuck, it means auto indenting Makefile and its pieces almost
impossible X-<.  I'll take the patch as there is no way to revert
the change to GNU make, though.

Thanks.

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Paul Smith]

On Mon, 2024-04-08 at 14:41 -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> 
> > When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by
> > one or
> > more tab characters, replace each tab character with 8 space
> > characters
> > with the following:
> > 
> >      find . -type f -not -path './.git/*' -name Makefile -or -name
> > '*.mak' |
> >        xargs perl -i -pe '
> >          s/(\t+)(ifn?eq|ifn?def|else|endif)/" " x (length($1) * 8)
> > . $2/ge unless /\\$/
> >        '
> 
> Yuck, it means auto indenting Makefile and its pieces almost
> impossible X-<.  I'll take the patch as there is no way to revert
> the change to GNU make, though.

I am considering whether to turn this error into a warning, for the
next release only, since it seems to be causing problems.  I have not
decided for sure yet since the change is needed to avoid a very real
parsing error (see the Savannah bug for details) which then could not
be fixed in this release.

Just to note that this usage clearly contravenes the documentation,
which states that preprocessor statement lines cannot begin with a TAB.
It was a bug that this was allowed by the GNU Make parser.

I understand that in many projects (Linux, probably Git :)) if the
documentation and behavior disagreed then the documentation would be
changed, not the behavior.

I'd love to do that as well but unfortunately there's just no way to
get coherent behavior out of GNU Make if this TAB prefix is allowed. 
If the original authors of GNU Make had followed the lead of the BSD
make folks (or C) and used some reserved character to introduce
preprocessor statements (BSD make uses ".if"/".else" etc. which would
work) then we wouldn't be in this predicament.  But make's parser is so
ad hoc that it's impossible to fix issues like this in a completely
backward-compatible manner.

I know that the coding style in some projects is to use TAB for each
level of indentation, but I do not think it's possible to extend that
same coding style from C into makefiles.  In makefiles, a TAB is not
just whitespace it's a meaningful token and has to be reserved for use
only in places where that meaning is required: to introduce a recipe
line.  Hopefully everyone's editors have the concept of separate modes
for different types of files, with different indentation styles for
each.

My personal recommendation is that you do not take this patch as-is,
and instead request a patch that converts every TAB character that
indents a GNU Make preprocessor statement into TWO spaces rather than
8.  Or even THREE spaces (to avoid indentation that matches a TAB in
width which could cause visual confusion).  However of course that's up
to you all.


If you wanted to make an even bigger change, which might save some
hair-pulling down the road but is a very serious decision, you could
introduce the use of the .RECIPEPREFIX [1] variable to change the
recipe prefix from TAB to some other character (as it should have been
when make was created back in the 1970's).

.RECIPEPREFIX was introduced in GNU Make 3.82, which was released in
2010 FYI.


Again, apologies for the churn... :(


[1] https://www.gnu.org/software/make/manual/html_node/Special-Variables.html#index-_002eRECIPEPREFIX-_0028change-the-recipe-prefix-character_0029

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Junio C Hamano]

Paul Smith <psmith@gnu.org> writes:

> Just to note that this usage clearly contravenes the documentation,
> which states that preprocessor statement lines cannot begin with a TAB.
> It was a bug that this was allowed by the GNU Make parser.
>
> I understand that in many projects (Linux, probably Git :)) if the
> documentation and behavior disagreed then the documentation would be
> changed, not the behavior.

If a bug is left in a released version long enough, it becomes a
feature your users depend upon.  We saw that happen to us, I am sure
the mantra "don't break userspace" the kernel project had comes from
the same place.

I am not sure what benefits are gained by the existing users with
this change to ease fixing some parser bug (I didn't bother to see
your bug tracker) so I cannot judge if the benefit outweighs the
cost of them all having to scramble and adjust to the new world
order.

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Paul Smith]

On Mon, 2024-04-08 at 16:34 -0700, Junio C Hamano wrote:
> I am not sure what benefits are gained by the existing users with
> this change to ease fixing some parser bug (I didn't bother to see
> your bug tracker) so I cannot judge if the benefit outweighs the
> cost of them all having to scramble and adjust to the new world
> order.

Just to point out that it's actually unusual (in my experience) for
makefiles to use TAB as indentation.  There are so many situations
where this comes back to bite you (see my other emails) that people
simply don't do it.

I realize Git and Linux (using Linus's coding style) are committed to
each-TAB-is-one-indentation-level, even insofar as using it inside
makefiles not just C code, but they are outliers IME.

So I'm not sure I'm ready to concede (yet) that the "cost of them all"
is actually very wide.  Certainly two projects as popular as Git and
Linux with this problem are very concerning.

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Junio C Hamano]

Paul Smith <psmith@gnu.org> writes:

> I'd love to do that as well but unfortunately there's just no way to
> get coherent behavior out of GNU Make if this TAB prefix is allowed. 
> If the original authors of GNU Make had followed the lead of the BSD
> make folks (or C) and used some reserved character to introduce
> preprocessor statements (BSD make uses ".if"/".else" etc. which would
> work) then we wouldn't be in this predicament.  But make's parser is so
> ad hoc that it's impossible to fix issues like this in a completely
> backward-compatible manner.

I wonder if you could ease the transition by leaving the current
parsing rule for conditional constructs that are indented with HT
and clearly mark them as "works as best-effort basis---the parsing
bug for them may remain", introduce BSD compatible .if/.else and
friends, and nudge the users in that direction.

Having to use two different indentation style in the same Makefile
is simply a nightmare, and that might be a good enough incentive for
users to move to the new "you can write with dots like .if and that
way you can continue indenting with HT".

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Paul Smith]

On Mon, 2024-04-08 at 16:44 -0700, Junio C Hamano wrote:
> Paul Smith <psmith@gnu.org> writes:
> 
> > I'd love to do that as well but unfortunately there's just no way
> > to get coherent behavior out of GNU Make if this TAB prefix is
> > allowed.
> 
> I wonder if you could ease the transition by leaving the current
> parsing rule for conditional constructs that are indented with HT
> and clearly mark them as "works as best-effort basis---the parsing
> bug for them may remain",

I'm not sure I understand the suggestion here.  If I preserve the
current parsing behavior what do I tell people who cannot get their
makefiles to work because the current parsing doesn't allow it?

> introduce BSD compatible .if/.else and friends, and nudge the users
> in that direction.
> 
> Having to use two different indentation style in the same Makefile
> is simply a nightmare, and that might be a good enough incentive for
> users to move to the new "you can write with dots like .if and that
> way you can continue indenting with HT".

I agree that it's a nightmare, but IMO trying to continue to work
around this terrible original sin in make (using TAB as a non-
whitespace token) by adding new keywords is the wrong direction.

The right direction is to STOP using TAB as a special token and turn it
back into what it is in other languages: simple whitespace.

That was already accomplished back in 2010 in GNU Make 3.82 with the
introduction of the .RECIPEPREFIX variable.

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Junio C Hamano]

Paul Smith <psmith@gnu.org> writes:

> I'm not sure I understand the suggestion here.  If I preserve the
> current parsing behavior what do I tell people who cannot get their
> makefiles to work because the current parsing doesn't allow it?

Their Makefiles were not working (perhaps began with HT followed by
a command whose name happened to be "ifdef" installed in ~/bin/ifdef
or something silly like that) before your "fix" to forbid using HT
to indent conditional, so your "fix" is not breaking them any
further.

If you optionally allow .if/.else etc., you can tell them to replace
their "ifdef" with ".ifdef".  Of course you can also tell them to
replace their HT indent before "ifdef" to spaces.

But the point is that those whose Makefiles were not parsed correctly
even before your "fix" need to fix their Makefiles anyway.  The
suggestion was about helping those whose Makefiles were happily been
grokked somehow before your "fix".  If you preserve the current code,
their Makefiles that indent their "ifdef" with HT will continue to
work, so you do not have to tell them anything, no?

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Jeff King]

On Mon, Apr 08, 2024 at 07:24:16PM -0400, Paul Smith wrote:

> If you wanted to make an even bigger change, which might save some
> hair-pulling down the road but is a very serious decision, you could
> introduce the use of the .RECIPEPREFIX [1] variable to change the
> recipe prefix from TAB to some other character (as it should have been
> when make was created back in the 1970's).
> 
> .RECIPEPREFIX was introduced in GNU Make 3.82, which was released in
> 2010 FYI.

Unfortunately, that's too recent for us. :( We try to keep the GNU make
dependency to 3.81, since that's the latest one Apple ships (because
they're allergic to GPLv3). Obviously it's not impossible to jump past
that and require people to install make via homebrew, etc, but that
makes it a more significant version bump than usual.

I do find it curious that in:

ifdef FOO
	SOME_VAR += bar
endif

the tab is significant for "ifdef" but not for SOME_VAR (at least that
is implied by Taylor's patch, which does not touch the bodies within the
conditionals).

I may just be showing my ignorance of the parsing issue, though. For
anybody else digging into the details, I think the correct link is:

  https://savannah.gnu.org/bugs/index.php?64185

(the commit has the wrong bug number, 64815).

-Peff

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Jeff King]

On Mon, Apr 08, 2024 at 08:04:14PM -0400, Jeff King wrote:

> I do find it curious that in:
> 
> ifdef FOO
> 	SOME_VAR += bar
> endif
> 
> the tab is significant for "ifdef" but not for SOME_VAR (at least that
> is implied by Taylor's patch, which does not touch the bodies within the
> conditionals).
> 
> I may just be showing my ignorance of the parsing issue, though. For
> anybody else digging into the details, I think the correct link is:
> 
>   https://savannah.gnu.org/bugs/index.php?64185
> 
> (the commit has the wrong bug number, 64815).

Answering my own question (at least what I think the answer is): there's
basically two levels of parsing going on. The outer layer is respecting
conditionals to decide which lines to care about at all, and the inner
one is figuring what are assignments, rules, recipes, etc.

So the outer parser cares about things that look like conditionals, but
nothing else. The inner one has more context and can more easily realize
that "\tSOME_VAR += bar" is not part of a recipe.

I'd guess it's _possible_ to fix the case discussed in the bug by
letting the outer parser know more of the inner-parser context (i.e.,
whatever rules it uses to decide that the assignment line is not a
recipe line could similarly be used for a line like "\telse"). But I
also wouldn't be at all surprised if it would involve a substantial
rewrite.  At any rate, I'd certainly defer to you on such matters. I'm
mostly just thinking out loud from my peanut-gallery perspective.

-Peff

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Paul Smith]

On Mon, 2024-04-08 at 20:04 -0400, Jeff King wrote:
> > .RECIPEPREFIX was introduced in GNU Make 3.82, which was released
> > in 2010 FYI.
> 
> Unfortunately, that's too recent for us. :( We try to keep the GNU
> make dependency to 3.81, since that's the latest one Apple ships
> (because they're allergic to GPLv3).

I understand that position, for Git.  I hope you can understand that in
my position I have no interest in catering to Apple's ridiculous
corporate antics and can only shrug and say "oh well" :)

> I do find it curious that in:
> 
> ifdef FOO
>  SOME_VAR += bar
> endif
> 
> the tab is significant for "ifdef" but not for SOME_VAR (at least
> that is implied by Taylor's patch, which does not touch the bodies
> within the conditionals).

The handling of TAB in makefiles is actually more subtle than the
simple "if it starts with a TAB it's part of a recipe".  The full
statement is, "if it starts with a TAB _and is in a recipe context_
then it's part of a recipe".

A recipe context starts after a target is defined, and it ends only
when the first non-TAB-indented, non-comment line is parsed (or EOF).

The text above will work ONLY if the content BEFORE that text is not a
recipe.  If you add a recipe before that line, then the SOME_VAR += bar
will suddenly be considered by make as part of that recipe and will be
an error when you run that recipe (since that's not valid shell
syntax).

So for example if you have:

  $ cat Makefile

  # set up the variable SOME_VAR
  ifndef TRUE
  <TAB>SOME_VAR = bar
  endif

  all: ; echo $(SOME_VAR)

This is fine.  But if you then modify your makefile like this:

  $ cat Makefile

  recurse: ; $(MAKE) -C subdir

  # set up the variable SOME_VAR
  ifndef TRUE
  <TAB>SOME_VAR = bar
  endif

  all: ; echo $(SOME_VAR)

It LOOKS fine but it's completely broken because the SOME_VAR
assignment now becomes part of the recipe for the recurse target.

   (Just to note, this is not due to a recent change in GNU Make, and
   in fact this is not even specific to GNU Make: all versions of make
   behave like this.)

So while the patch proposed here does not remove all TAB characters, my
best advice is that as a project you SHOULD consider pro-actively
removing all non-recipe-introducing TAB characters.  They are dangerous
and misleading, even outside of this ifdef kerfuffle.


All I can do is reiterate my original statement: it's a bad idea to
consider TAB characters as whitespace or "just indentation" AT ALL when
editing makefiles.  TABs are not whitespace.  They are meaningful
tokens, like "$" or "#", and should only ever be used in places where
that meaning is desired.  The fact that they look like indentation and
can be used in some other places as "just indentation" and kinda-sorta
work, is an accident waiting to happen if it's taken advantage of.

Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
>> more tab characters, replace each tab character with 8 space characters
>> with the following:
>>
>>     find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |
>>       xargs perl -i -pe '
>>         s/(\t+)(ifn?eq|ifn?def|else|endif)/" " x (length($1) * 8) . $2/ge unless /\\$/
>>       '
>
> Yuck, it means auto indenting Makefile and its pieces almost
> impossible X-<.  I'll take the patch as there is no way to revert
> the change to GNU make, though.

We'd need something like this on top.  Our top-level .gitattributes
defines the default whitespace rules with !indent-with-non-tab and
enables indent-with-non-tab for specific file suffixes like .[ch],
but git-gui/.gitattributes enforces indent-with-non-tab for all
files.

Another thing with this series (and this follow-up) is if we want to
start treating git-gui/ as just a subdirectory without no plan to
feed our changes to the "upstream".  I am actually OK with that, as
the "upstream" we merge (with -Xsubtree merge strategy) from does
not seem to be very active and responsive these days.

 git-gui/.gitattributes | 1 +
 1 file changed, 1 insertion(+)

diff --git c/git-gui/.gitattributes w/git-gui/.gitattributes
index 59cd41dbff..118d56cfbd 100644
--- c/git-gui/.gitattributes
+++ w/git-gui/.gitattributes
@@ -3,3 +3,4 @@
 git-gui.sh  encoding=UTF-8
 /po/*.po    encoding=UTF-8
 /GIT-VERSION-GEN eol=lf
+Makefile    whitespace=!indent,trail,space