test/Makefile
branchmake-cleanup
changeset 3199 e16d89af41ad
parent 3180 f7ce02c58571
child 3200 329005a93c6a
     1.1 --- a/test/Makefile	Thu Dec 20 12:57:00 2018 +0100
     1.2 +++ b/test/Makefile	Thu Dec 20 21:47:52 2018 +0100
     1.3 @@ -6,8 +6,11 @@
     1.4  
     1.5  include ../default.conf
     1.6  
     1.7 +# I tend to not use spacing around assignment operators in Make. Sometimes weird things happen.
     1.8 +# Also -- this is very much a matter of taste -- my python scripts all have '#!/usr/bin/env python3' as the first line, and it's the user's job for there to always be an appropriate python3. I wouldn't add any business logic in the makefile regarding finding the python interpreter, if I just run my own python scripts.
     1.9  PY_ENV := $(shell command -v python3 2> /dev/null)
    1.10  
    1.11 +# Since you 'include' before evaluating CURDIR, I think it's possible that CURDIR does not have the value you think it might have. Yes, make is awkward.
    1.12  HERE:=$(CURDIR)
    1.13  TEST_HOME=$(HERE)/pEp_test_home
    1.14  
    1.15 @@ -17,11 +20,15 @@
    1.16  OBJS := $(addsuffix .o,$(basename $(SRCS)))
    1.17  DEPS := $(OBJS:.o=.d)
    1.18  
    1.19 +# Using '/usr/local/include' is very much a "works on my machine" solution to the problem. Either, the compiler search paths are enough, otherwise define a BLA_INC and BLA_LIB path to make clear to other developers what you are looking for outside the standard paths.
    1.20  INC_DIRS := ./include /usr/local/include ../src ../sync ../asn.1
    1.21 +# Whoa dude... addprefix is brilliant. Why didn't I find this before!? This could make setting all the *_INC and *_LIB so much nicer!
    1.22  INC_FLAGS := $(addprefix -I,$(INC_DIRS)) $(GPGME_INC) $(CPPUNIT_INC)
    1.23  
    1.24 +# '+=' is a magic assignment operators. It ensures that there is exactly one space between the old value and the suffix. Yes, this means it might strip spaces. At any rate, there is no need to add a space after the operator.
    1.25  LDFLAGS += -L/usr/local/lib
    1.26  
    1.27 +# For setting both C and C++ compiler flags, there is CPPFLAGS
    1.28  CFLAGS += -Wno-deprecated
    1.29  CXXFLAGS += -Wno-deprecated
    1.30  CFLAGS:=$(filter-out -Wall,$(CFLAGS))
    1.31 @@ -29,6 +36,7 @@
    1.32  LDFLAGS+= $(ETPAN_LIB) $(CPPUNIT_LIB) -L../asn.1 -L../src
    1.33  LDLIBS+= -letpan -lpEpEngine -lstdc++ -lasn1
    1.34  
    1.35 +# Caution: you're using tab to indent here, but none of the lines from here to 60 are recipes. Indentation should be done with spaces. This only works because Make is such a gratious tool. (Yeah, right...)
    1.36  ifeq ($(BUILD_FOR),Linux)
    1.37  	LDLIBS+= -luuid
    1.38  endif
    1.39 @@ -45,6 +53,7 @@
    1.40  	LDLIBS+= -lsqlite3
    1.41  endif
    1.42  
    1.43 +# Looks like both LIBPATH and LLDB_BIN aren't needed any more.
    1.44  ifeq ($(shell uname),Darwin)
    1.45  	LIBPATH=DYLD_LIBRARY_PATH
    1.46  	LLDB_BIN=/Applications/Xcode.app/Contents/Developer/usr/bin/lldb
    1.47 @@ -55,7 +64,7 @@
    1.48  
    1.49  LDLIBS+= -lcpptest
    1.50  
    1.51 -
    1.52 +# Probably everything from here to 81 is not needed any more either...
    1.53  # Create a list of the extra library paths for the loader. I do not assume that the engine (and its dependencies) are installed for testing.
    1.54  # Note that += can not be used here, as it changes the amount of whitespace
    1.55  EXTRA_LIB_PATHS=../src:
    1.56 @@ -88,8 +97,10 @@
    1.57  
    1.58  CPPFLAGS += $(INC_FLAGS) -MMD -MP
    1.59  
    1.60 +# Forgot the phony for all?
    1.61  all: suitemaker $(TARGET) test_home_ scripts
    1.62  	
    1.63 +# There is probably a built-in rules that makes this redundant. Unless we're not using '.cc' or 'CPPFLAGS' as is canon in GNU Make land.
    1.64  %.o: %.cc
    1.65  	$(CXX) -c $(CFLAGS) $(CPPFLAGS) $< -o $@
    1.66  
    1.67 @@ -109,16 +120,19 @@
    1.68  endif
    1.69  endif
    1.70  		
    1.71 +# This target does nothing.
    1.72  .PHONY: test_home_
    1.73  test_home_: 
    1.74  	
    1.75  
    1.76 +# So, if there is no python, then 'scripts' will trivially be up to date, because there is no recipe associated with this rule. That's probably not what you want.
    1.77  .PHONY: scripts
    1.78  scripts: 
    1.79  ifdef PY_ENV
    1.80  	$(PY_ENV) genscripts.py
    1.81  endif
    1.82  
    1.83 +# Ah, yes, here it is. Finally someone tripped over me introducing both Makefile.conf and default.conf. TEST_DEBUGGER is documented in one, but not the other Makefile. Yeah, I should clean this up, otherwise every time a new knob is added, it will only appear in either Makefile.conf or default.conf.
    1.84  .PHONY: test
    1.85  test: all
    1.86  	$(TEST_DEBUGGER) ./$(TARGET)
    1.87 @@ -130,5 +144,6 @@
    1.88  
    1.89  
    1.90  
    1.91 +# You know, I think we have this "compiler generates Makefiles" pattern in another makefile in the engine, but there it looks completly different. From reading just this makefile: are you sure, the compiler generated Makefiles are actually every generated? You don't have DEPS as a prerequisite anywhere. Is there some built-in rule that has %.d as a prerequisite? Without further investigation I am not convinced the %.d files are ever generated...
    1.92  -include $(DEPS)
    1.93