mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.03k stars 45 forks source link

".PHONY<space>:" not judged as valid #63

Open junaruga opened 2 years ago

junaruga commented 2 years ago

Expected behaviour

The checkmake passes for the .PHONY : foo (.PHONY<space>: foo) in the Makefile.

$ checkmake Makefile 

$ echo $?
0

Actual behaviour

The checkmake complains for the .PHONY : foo (.PHONY<space>: foo) in the Makefile. I used this Makefile https://github.com/junaruga/cpp-test/blob/report-checkmake/src/Makefile

$ checkmake Makefile 
    RULE              DESCRIPTION             LINE NUMBER  

  minphony   Missing required phony target    0            
             "all"                                         
  minphony   Missing required phony target    0            
             "clean"                                       
  minphony   Missing required phony target    0            
             "test"

$ echo $?
3   

Seeing the GNU make PHONY document, there are 2 examples for both .PHONY: foo (without space) and .PHONY : foo (with space). So I assume both are valid. https://www.gnu.org/software/make/manual/make.html#Phony-Targets

The checkmake passes the Makefile by applying the following patch. So, I assume that the .PHONY : foo (with space) is judged as invalid. But why?

diff --git a/src/Makefile b/src/Makefile
index 2ff0b4e..c0c1206 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -21,16 +21,16 @@ LIBS =
 .c.o :
        $(CC) -c $(CFLAGS) $(CPPFLAGS) $(INCLUDES) $< -o $@

-.PHONY : all
+.PHONY: all
 all : $(EXE)

 $(EXE) : $(OBJS) main.o
        $(CXX) $(CXXFLAGS) $^ -o $@ $(LIBS)

-.PHONY : clean
+.PHONY: clean
 clean :
        rm -f *.o $(EXE)

-.PHONY : test
+.PHONY: test
 test : all
        echo "Run the test"

Output of checkmake --version

There is no version info. The checkmake was installed today by the steps I wrote at https://github.com/mrtazz/checkmake/issues/62 .

$ checkmake --version
checkmake  built at  by  with 

Output of checkmake --debug <your makefile>

$ checkmake --debug Makefile 
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '  $(CXX) -c $(CXXFLAGS) $(CPPFLAGS) $(INCLUDES) $< -o $@' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '  $(CC) -c $(CFLAGS) $(CPPFLAGS) $(INCLUDES) $< -o $@' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line 'all : $(EXE)' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '$(EXE) : $(OBJS) main.o' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '  $(CXX) $(CXXFLAGS) $^ -o $@ $(LIBS)' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line 'clean :' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '  rm -f *.o $(EXE)' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line 'test : all' to a Rule or Variable
2021/12/03 18:56:56 Unable to match line '  echo "Run the test"' to a Rule or Variable
2021/12/03 18:56:56 Unable to parse config file "checkmake.ini", running with defaults
2021/12/03 18:56:56 Running rule 'phonydeclared'...
2021/12/03 18:56:56 iniFile not initialized
2021/12/03 18:56:56 Running rule 'timestampexpanded'...
2021/12/03 18:56:56 iniFile not initialized
2021/12/03 18:56:56 Running rule 'maxbodylength'...
2021/12/03 18:56:56 iniFile not initialized
2021/12/03 18:56:56 Running rule 'minphony'...
2021/12/03 18:56:56 iniFile not initialized
2021/12/03 18:56:56 iniFile not initialized
    RULE              DESCRIPTION             LINE NUMBER  

  minphony   Missing required phony target    0            
             "all"                                         
  minphony   Missing required phony target    0            
             "clean"                                       
  minphony   Missing required phony target    0            
             "test" 

Output of make --version

$ make --version
GNU Make 4.3
Built for x86_64-redhat-linux-gnu
Copyright (C) 1988-2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Sample Makefile to reproduce issue

(some of these things might not apply but the more you can provide the easier it will be to fix this bug. Thanks!)

Here is the sample Makefile as I wrote above. https://github.com/junaruga/cpp-test/blob/report-checkmake/src/Makefile

$ git clone --branch report-checkmake https://github.com/junaruga/cpp-test.git
$ cd cpp-test/src
$ checkmake Makefile

Thanks!

mrtazz commented 2 years ago

I'm not 100% certain but I believe other make implementations (like BSD and posix make) don't allow the space before the colon. So the checker errs on the side of caution here.

I'm open to changing the rule and have a special rule for BSD make if that is useful. Or make it configurable in for this rule. However I also don't think it really hurts to just not have the space before the colon