bugmake - Bugs: bug #64259, Regression in master: make ignores...

 
 

bug #64259: Regression in master: make ignores statements it should not ignore.

Submitter:  Dmitry Goncharov <dgoncharov>
Submitted:  Sat 27 May 2023 03:33:14 PM UTC
Votes: 1
 
Severity:  3 - Normal Item Group:  Bug
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Component Version:  SCM Operating System:  None
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 18 Dec 2023 10:03:24 AM UTC, comment #3: 

Paul did pretty much explicitly say in the introducer, bug #64185, that he wasn't interested in being told about:

> people indenting their conditionals with the recipe prefix


... and yet here I am, sorry, pointing at the tab still at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Makefile?h=v6.7-rc6#n902 since 2010-10-13 17:12:30 -0400 -email is unavailable- (72441cb1fd77d092f09ddfac748955703884c9a7). There are others but that's the one that's just cost me a few hours to hack out on our driver build installation for 11 kernels of varying ages. I know the Linux build system's compatibility with Gnu Make has been important to a number of other posters over the years. I wonder that we haven't heard from Yamada-san, who we did hear from in bug #64288 after the introducer went in.

 Martin Dorey <mdorey>
Sun 17 Dec 2023 10:53:49 PM UTC, comment #2: 

The change suggested here doesn't treat these two cases quite symmetrically:

martind@stormy:~/tmp/make-missing-endif$ printf 'ifeq "a" "a"\n\tendif\ndefault:;\n' > Makefile; ~/download/make/make
Makefile:2: *** recipe commences before first target.  Stop.
martind@stormy:~/tmp/make-missing-endif$ printf 'ifeq "a" "b"\n\tendif\ndefault:;\n' > Makefile; ~/download/make/make
Makefile:4: *** missing 'endif'.  Stop.
martind@stormy:~/tmp/make-missing-endif$

I'd say that's only a minor code smell.  I consider it an advance on the pushed code, which accepts the first, while rejecting the second.  It better fulfills the NEWS entry by rejecting:

martind@stormy:~/tmp/make-missing-endif$ printf '\tifeq "" ""\nendif\ndefault:;\n' > Makefile; ~/download/make/make
Makefile:1: *** recipe commences before first target.  Stop.
martind@stormy:~/tmp/make-missing-endif$

... which the pushed code accepts:

martind@stormy:~/tmp/make-missing-endif$ printf '\tifeq "" ""\nendif\ndefault:;\n' > Makefile; ~/bin/make-4.4.90
make-4.4.90: 'default' is up to date.
martind@stormy:~/tmp/make-missing-endif$


I'd like to also suggest this correction to the introducer:

martind@stormy:~/download/make$ git diff tests/scripts/features/conditionals
diff --git a/tests/scripts/features/conditionals b/tests/scripts/features/conditionals
index c1eee95c..27417623 100644
--- a/tests/scripts/features/conditionals
+++ b/tests/scripts/features/conditionals
@@ -153,7 +153,7 @@ endif
 ',
               '', "one\n");

-# SV 64085: Ensure recipe prefixed conditionals are never considered
+# SV 64185: Ensure recipe prefixed conditionals are never considered
 run_make_test(q!
 blah=1
 ifdef blah
martind@stormy:~/download/make$

I struggled to find the right bug, hence to find my way here.

 Martin Dorey <mdorey>
Sat 27 May 2023 03:44:30 PM UTC, comment #1: 

Commit 07fcee35f058a876447c8a021f9eb1943f902534 introduced a regression.

The text in NEWS clarifies that the idea was to never consider a line starting with the recipe prefix as a conditional, to ensure make does not confuse recipe lines with conditionals as described in sv 64185.

However, there are cases where the fix
1. Still considers lines starting with the recipe prefix as conditionals.
2. Ignores lines which it should not ignore.

In this makefile both 'ifdef blah' and 'else' are intended with tabs.


$ ls
makefile
$ cat makefile
        ifdef blah
$(info true)
        else
$(info false)
endif
all:
$ make-4.4
false
make-4.4: Nothing to be done for 'all'.
$ # this is the latest make from master
$ ~/src/gmake/make/m64/make
make: Nothing to be done for 'all'.


We can see the latest make
1. honored 'ifdef blah', even though it is intended with a tab.
2. failed to run '$(info false)'.

The patch in the attachment causes make to fail with a syntax error here.
The patch keeps NEWS intact.

 Dmitry Goncharov <dgoncharov>
Sat 27 May 2023 03:33:14 PM UTC, original submission:  

.

 Dmitry Goncharov <dgoncharov>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54791:  sv64259_fix.diff added by dgoncharov (2KiB - text/x-patch)
file #54792:  sv64259_test.diff added by dgoncharov (1KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mdorey (Voted in favor of this item)
  • -email is unavailable- added by mdorey (Posted a comment)
  • -email is unavailable- added by dgoncharov (Submitted the item)

    •  

    There is 1 vote so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-12-17 mdorey Carbon-Copy- Added mdorey
    2023-05-27 dgoncharov Attached File- Added sv64259_fix.diff, #54791
        Attached File- Added sv64259_test.diff, #54792

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-397b.
    Corresponding source code