| Message ID | 9d14c08ca6cc06cdf8fb4ba33d2470053dca3966.1712591504.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Makefile(s): avoid recipe prefix in conditional statements | expand |
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.
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
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
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.
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".
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
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
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.
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.
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.
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?
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
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(-)