[bug #62206] Fix % substitution in pattern rules.

[Dmitry Goncharov]

Follow-up Comment #4, bug #62206 (project make):

Such a workaround with 2 rules could be something like


.SECONDEXPANSION:
all: lib/hello.o hello.o
%.o: 3$$*.c; $(info $@ from $<)
lib/%.o: lib/3$$*.c; $(info $@ from $<)



___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

Re: GNU Make bug report: broken dry-run functionality with sub-make invocations

[Edward Welbourne]

Ambrus Sumegi (21 March 2022 14:22) wrote:
> If the invocation is a function, i.e., `$(make,"external_target") |
> tee logs/external_task.log` then Make knows exactly where the call to
> the sub-make ends without having to parse a shell command. So, when
> running with the -n switch, it can simply print "make external_target
> | tee logs/external_task.log" and proceed to show the output of `make
> external_target -n`

However, that doesn't help cases like

subdirs:; find sub/ -type f -name Makefile | \
sed -e 's!/Makefile!!' | grep . | \
while read dir; do $(MAKE) -C $$dir; done

where the failure to run the rest of the command makes the call to
$(MAKE) never happen; and replacing the last line with

while read dir; do $(make -C $$dir); done

or similar won't save you.
Fixing one use-case while breaking all others is no solution,

Eddy.

[bug #62206] Fix % substitution in pattern rules.

[Dmitry Goncharov]

Follow-up Comment #3, bug #62206 (project make):

> What I usually do is recommend people use $$* instead of % in secondary
expansion expressions like this as it's more clear and less confusing, and
ends up being the same result.

There are situations where the difference between % and $$* matters.

For example, let us say there are files 3hello.c and lib/3hello.c and the
following makefile.



.SECONDEXPANSION:
all: lib/hello.o hello.o
%.o: 3%.c; $(info $@ from $<)



This makefile allows to build both hello.o from 3hello.c and lib/hello.o from
lib/3hello.c with a single rule.

It is possible to workaround this as well by using two rules lib/%.o: and
%.o:. So, i was thinking maybe fixing this is not worth the effort. On the
other hand, lots of users may not be familiar with make enough to figure this
out. With this patch it just works.


___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

RE: GNU Make bug report: broken dry-run functionality with sub-make invocations

[Ambrus Sumegi]

If the invocation is a function, i.e., `$(make,"external_target") | tee 
logs/external_task.log` then Make knows exactly where the call to the sub-make 
ends without having to parse a shell command. So, when running with the -n 
switch, it can simply print "make external_target | tee logs/external_task.log" 
and proceed to show the output of `make external_target -n`

-Original Message-
From: Paul Smith 
Sent: 21 March 2022 14:18
To: Ambrus Sumegi ; bug-make@gnu.org
Subject: Re: GNU Make bug report: broken dry-run functionality with sub-make 
invocations

On Mon, 2022-03-21 at 09:34 +, Ambrus Sumegi wrote:
> For the record, I’ve thought of a sort-of-solution to the “would you
> have Make parse the shell command” question over the weekend. If the
> sub-make was called through a function rather than a variable, the
> whole issue could be a lot more contained. Having $(make, “ passed to sub-make>”) as the canonical form of invoking other Make
> instances could guarantee that no other program gets called when the
> -n switch is used. This way any elaborate multi-line command
> containing $(MAKE) could more or less be fixed with a sed command to
> completely eliminate this problem class.

I don't see how this would work.

The command you wanted to run was:

$(MAKE) external_target | tee logs/external_task.log

You wanted to pipe the output of the make command to another command.
How does putting make in a function allow this to happen?

> Of course, one could still do something like $(make, $(shell
> (“some_reckless_command”)), but it would be more obvious, and Make
> could output an explicit warning about a shell being called together
> with the sub-make invocation, or even refuse to execute said shell
> call. Since on Fri 18/03/2022 20:14, psm...@gnu.org pointed out that
> this is also a potential security problem, a future release could
> deprecate the $(MAKE) variable and switch to the function-based
> invocation, with shell calls disabled within it by default for the
> sake of that extra bit of security.

It's simply not possible to make this change.  People want to do all kinds of 
complicated things before or after or instead of invoking the sub-make, so the 
full power of the shell needs to be available.

If you wanted to simply run a make command with no surrounding shell 
operations, then running:

$(MAKE) ...

all by itself _already_ does exactly what you want.  It's only when you need to 
add extra operations into the same recipe line that you run into problems with 
"-n", and that's exactly the place where using a make function instead won't 
work.
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

Re: GNU Make bug report: broken dry-run functionality with sub-make invocations

[Paul Smith]

(We generally prefer to use inline replies on the GNU lists, rather
than top-posted replies, thanks)

On Mon, 2022-03-21 at 13:22 +, Ambrus Sumegi wrote:
> If the invocation is a function, i.e., `$(make,"external_target") |
> tee logs/external_task.log` then Make knows exactly where the call to
> the sub-make ends without having to parse a shell command. So, when
> running with the -n switch, it can simply print "make external_target
> | tee logs/external_task.log" and proceed to show the output of `make
> external_target -n`

So if someone runs make -n it would be OK if only the make was invoked,
without actually sending any of the output to the tee (because clearly
the tee would not be invoked)?  That idea seems just as fraught, of not
moreso, as the original behavior.

What if your recipe looked like this:

test -f afile || $(MAKE) -C foo

If you change your makefile to use the putative function like this:

test -f afile || $(make -C foo)

Are you saying that if "make -n" is invoked this recipe will ALWAYS run
the sub-make, regardless of whether the "afile" file exists or not?

If you check makefiles out in the world you'll see that many times the
recipe lines that are used to invoke sub-makes are complex with lots of
additional shell operations in the same recipe line.  IMO a facility
that reached into the middle of all that scripting and plucked out the
sub-make by itself and ran it without any of that surrounding context,
when make -n was given, would be at least as dangerous as what we have
now.

Re: GNU Make bug report: broken dry-run functionality with sub-make invocations

[Paul Smith]

On Mon, 2022-03-21 at 09:34 +, Ambrus Sumegi wrote:
> For the record, I’ve thought of a sort-of-solution to the “would you
> have Make parse the shell command” question over the weekend. If the
> sub-make was called through a function rather than a variable, the
> whole issue could be a lot more contained. Having $(make, “ passed to sub-make>”) as the canonical form of invoking other Make
> instances could guarantee that no other program gets called when the
> -n switch is used. This way any elaborate multi-line command
> containing $(MAKE) could more or less be fixed with a sed command to
> completely eliminate this problem class.

I don't see how this would work.

The command you wanted to run was:

$(MAKE) external_target | tee logs/external_task.log

You wanted to pipe the output of the make command to another command. 
How does putting make in a function allow this to happen?
 
> Of course, one could still do something like $(make, $(shell
> (“some_reckless_command”)), but it would be more obvious, and Make
> could output an explicit warning about a shell being called together
> with the sub-make invocation, or even refuse to execute said shell
> call. Since on Fri 18/03/2022 20:14, psm...@gnu.org pointed out that
> this is also a potential security problem, a future release could
> deprecate the $(MAKE) variable and switch to the function-based
> invocation, with shell calls disabled within it by default for the
> sake of that extra bit of security.

It's simply not possible to make this change.  People want to do all
kinds of complicated things before or after or instead of invoking the
sub-make, so the full power of the shell needs to be available.

If you wanted to simply run a make command with no surrounding shell
operations, then running:

$(MAKE) ...

all by itself _already_ does exactly what you want.  It's only when you
need to add extra operations into the same recipe line that you run
into problems with "-n", and that's exactly the place where using a
make function instead won't work.

RE: GNU Make bug report: broken dry-run functionality with sub-make invocations

[Ambrus Sumegi]

For the record, I've thought of a sort-of-solution to the "would you have Make 
parse the shell command" question over the weekend. If the sub-make was called 
through a function rather than a variable, the whole issue could be a lot more 
contained. Having $(make, "") as the canonical 
form of invoking other Make instances could guarantee that no other program 
gets called when the -n switch is used. This way any elaborate multi-line 
command containing $(MAKE) could more or less be fixed with a sed command to 
completely eliminate this problem class.

Of course, one could still do something like $(make, $(shell 
("some_reckless_command")), but it would be more obvious, and Make could output 
an explicit warning about a shell being called together with the sub-make 
invocation, or even refuse to execute said shell call. Since on Fri 18/03/2022 
20:14, psm...@gnu.org pointed out that this is also a 
potential security problem, a future release could deprecate the $(MAKE) 
variable and switch to the function-based invocation, with shell calls disabled 
within it by default for the sake of that extra bit of security.

From: Martin Dorey 
Sent: 17 March 2022 19:27
To: Ambrus Sumegi ; bug-make@gnu.org
Subject: Re: GNU Make bug report: broken dry-run functionality with sub-make 
invocations

> the statement after the pipe also gets executed

Would you have Make parse the shell command, assuming that SHELL isn't eg 
/usr/bin/perl, splitting off anything after | and maybe ; and && and, why not, 
||, so it only dry-runs the part of the command that invoked $(MAKE)?  The 
current behavior is documented, 
https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable
 saying:

>> whenever a recipe line of a rule contains the variable MAKE, the flags '-t', 
>> '-n' and '-q' do not apply to that line

To put it another way: the Makefile needs to cope.  Not doing the tee if the 
directory doesn't exist would perhaps be most straightforward:

martind@sirius:~/tmp/ambrus-sumegi-2022-03-17$ diff -u Makefile{.orig,}
--- Makefile.orig 2022-03-17 11:13:46.000328340 -0700
+++ Makefile 2022-03-17 11:15:04.980482898 -0700
@@ -8,4 +8,4 @@

 .PHONY: main_target
 main_target: create_logdirs
- $(MAKE) external_target | tee logs/external_task.log
+ $(MAKE) external_target | { if test -d logs; then tee logs/external_task.log; 
else cat; fi; }
martind@sirius:~/tmp/ambrus-sumegi-2022-03-17$

... though that means writing to the log with -n if the directory does exist, 
which is perhaps undesirable.  You could scrape the --dry-run flag out of 
MAKEFLAGS, where it seems to get turned into n:

martind@sirius:~/tmp/ambrus-sumegi-2022-03-17$ diff -u Makefile{.orig,}
--- Makefile.orig 2022-03-17 11:13:46.000328340 -0700
+++ Makefile 2022-03-17 11:23:29.579910454 -0700
@@ -8,4 +8,4 @@

 .PHONY: main_target
 main_target: create_logdirs
- $(MAKE) external_target | tee logs/external_task.log
+ $(MAKE) external_target $(if $(findstring n,$(filter-out 
--%,$(MAKEFLAGS))),,| tee logs/external_task.log)
martind@sirius:~/tmp/ambrus-sumegi-2022-03-17$

That coped with -nj --no-print-directory on the one version of Make that I 
tested it with, but I don't know how portable that would prove.

Thank you for a nicely written up report with a minimal test case.


From: Bug-make 
mailto:bug-make-bounces+martin.dorey=hds@gnu.org>>
 on behalf of Ambrus Sumegi 
mailto:ambrus.sum...@arm.com>>
Sent: Thursday, March 17, 2022 08:59
To: bug-make@gnu.org 
mailto:bug-make@gnu.org>>
Subject: GNU Make bug report: broken dry-run functionality with sub-make 
invocations

* EXTERNAL EMAIL *

Dear Devs,

I stumbled upon a rather rare case where Make produces a false error with the 
--dry-run switch. I've attached a sample Makefile for reproduction.



The output of make main_target with this file is the following:

$ make main_target

mkdir logs

make external_target | tee logs/external_task.log

make[1]: Entering directory '/tmp/21176961.tmpdir/bugrepro'

This could be in a separate file somewhere

make[1]: Leaving directory '/tmp/21176961.tmpdir/bugrepro'



Nothing interesting there. But with the -n or --dry-run switch I get:

$ make main_target -n

mkdir logs

make external_target | tee logs/external_task.log

tee: logs/external_task.log: No such file or directory

make[1]: Entering directory '/tmp/21176961.tmpdir/bugrepro'

echo "This could be in a separate file somewhere"

make[1]: Leaving directory '/tmp/21176961.tmpdir/bugrepro'

make: *** [Makefile:11: main_target] Error 1



I get that the sub-make invocation gets executed with the switch passed to it 
as well, and that is correct functionality. But then the statement after the 
pipe also gets executed and throws an error about the missing log directory due 
to the prerequisite target not having been run. This causes false failures in 
release checks that use the --dry-run option of Make.



This 

[bug #62206] Fix % substitution in pattern rules.

[Paul D. Smith]

Follow-up Comment #2, bug #62206 (project make):

What I usually do is recommend people use $$* instead of % in secondary
expansion expressions like this as it's more clear and less confusing, and
ends up being the same result.

I'll look at this change.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/