git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Paul Smith <psmith@gnu.org>
To: git@vger.kernel.org
Subject: Re: [PATCH] Makefile(s): avoid recipe prefix in conditional statements
Date: Tue, 09 Apr 2024 16:44:45 -0400	[thread overview]
Message-ID: <daa51548ff2d0cfc4407bbdbe99223c84321a503.camel@gnu.org> (raw)
In-Reply-To: <20240409000414.GA1647304@coredump.intra.peff.net>

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.


  parent reply	other threads:[~2024-04-09 20:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08 10:44 Makefiles are broken as of GNU Make commit 07fcee35f058a876447c8a021f9eb1943f902534 Dario Gjorgjevski
2024-04-08 15:51 ` [PATCH] Makefile(s): avoid recipe prefix in conditional statements Taylor Blau
2024-04-08 21:41   ` Junio C Hamano
2024-04-08 23:24     ` Paul Smith
2024-04-08 23:34       ` Junio C Hamano
2024-04-09 20:41         ` Paul Smith
2024-04-08 23:44       ` Junio C Hamano
2024-04-09 20:42         ` Paul Smith
2024-04-09 21:23           ` Junio C Hamano
2024-04-09  0:04       ` Jeff King
2024-04-09  0:17         ` Jeff King
2024-04-09 20:44         ` Paul Smith [this message]
2024-04-08 23:28     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=daa51548ff2d0cfc4407bbdbe99223c84321a503.camel@gnu.org \
    --to=psmith@gnu.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).