abcminiuser / dmbs

Dean's Makefile Build System - making MAKE easier.
51 stars 18 forks source link

Update WritingYourOwnModules.md #14

Closed NicoHood closed 7 years ago

NicoHood commented 7 years ago

Corrects the displayed markdown

Also the text suggests to include the dmbs core etc. The only reference I have is lufa, and LUFA just ignores that if I am right: https://github.com/abcminiuser/lufa/tree/master/LUFA/Build/LUFA

Do you have any other examples for dmbs modules? A Module template would be nice for a library with a .h and .c file.

I will use this PR to ask a few questions:

abcminiuser commented 7 years ago

Also the text suggests to include the dmbs core etc. The only reference I have is lufa, and LUFA just ignores that if I am right: https://github.com/abcminiuser/lufa/tree/master/LUFA/Build/LUFA

Hrm, good catch. If I remember right I didn't include the DMBS core in the LUFA extension modules as the path to the former wasn't easy to infer from the latter, but it was so long ago that I don't remember exactly why I omitted it. I'll have to experiment locally and see if re-adding the core include is possible.

Do you have any other examples for dmbs modules? A Module template would be nice for a library with a .h and .c file.

The LUFA ones are the only ones I'm aware of - DMBS has all the modules I need included already, and I'm not sure anyone else has had enough interest to try to write their own extensions.

The "module hooks" add some information what the module defines (targets, input/output vars etc). To me it seems that this is not important at all, because I can leave out the information and targets will still work and variables will give errors even when they are listed as mandatory. So the sanity check still is part of the module. To me the hooks header seems useless or has some bugs?

Those values (DMBS_BUILD_MODULES, DMBS_BUILD_TARGETS, etc.) in the header are used by the core module, when make help is invoked. It will/should print a list of all the available included module names, targets, mandatory variables, optional variables, etc. to the user on the command line. It's also used for the sanity checks within the module to show a helpful error message if a mandatory variable isn't set, but its primary purpose is for the CORE module's help output or to allow you to add an explicit dependency check on another module.

NicoHood commented 7 years ago

Thanks for the fast response, some more questions/notes following:

I tried to include the dmbs core module, but if you ask me the readme instruction is wrong. My setup looks like this:

makefile
main.c (etc)
AES/AES-sources.mk
AES/aes.c (etc)
../dmbs (somewhere extern)

My current makefile

For a better overview my sources look like this: https://github.com/limpkin/mooltipass/tree/master/source_code/src

But with the provided solution it searches the core in AES/core.mk which is wrong. I've modified it like this:

# Conditionally import the CORE module of DMBS if it is not already imported
# DMBS_MODULE_PATH := $(patsubst %/,%,$(dir $(lastword $(MAKEFILE_LIST))))
ifeq ($(findstring CORE, $(DMBS_BUILD_MODULES)),)
#   include $(DMBS_MODULE_PATH)/core.mk
    include $(DMBS_PATH)/core.mk
endif

This requires the DMBS_PATH defined first. A simpler check should be added for this too.

Lufa uses those settings (i've adapted this)

SHELL = /bin/sh

ERROR_IF_UNSET   ?= $(if $(filter undefined, $(origin $(strip $(1)))), $(error Makefile $(strip $(1)) value not set))
ERROR_IF_EMPTY   ?= $(if $(strip $($(strip $(1)))), , $(error Makefile $(strip $(1)) option cannot be blank))
ERROR_IF_NONBOOL ?= $(if $(filter Y N, $($(strip $(1)))), , $(error Makefile $(strip $(1)) option must be Y or N))

# Sanity check user supplied values
$(call ERROR_IF_EMPTY, DMBS_PATH)

If you ask me this should be included from the core.mk file. Also the core.mk should do the check of the mandatory variables (except DMBS_PATH as this is required to include the core).

So the conecpt would look like this:

Error if DMBS_PATH not defined
inlcude core.mk if not done yet
core.mk check mandatory variables
core.mk defines the error checks
Can't core.mk also replace the PHONY then?

Now I want to add the different sources to my module.

If you look at the AES library I want to provide a module for AES which defines all required sources for AES. Now I am wondering if I need to still add those to the SRC array in my makefile similar to this or rather do this in the module itself (which mean including the module will add the sources. thats more what i expect it will do).

If it will add it to SRC it needs to ensure that a multiple inclution of the module doesnt cause problems. For example if the makefile includes AES and another library also includes the AES module you'd have the aes.c twice if i got this right. DMBS would need to remove duplicate entries, maybe the makefile already does, i dont know. Another idea would be to use a DEPENDS array that checks if a module is also included (but from the main makefile, not the module itself to avoid include guards).

And finally I want to compile those tests which are excluded from the module by default. The nessie test should only compile the nessie file + AES and nothing more of the main program, as its a standalone test. Is that possible? I know I need to add a target for that, but how would I write it?

NicoHood commented 7 years ago

What I mean is something like this. This could be possibly put into a wrapper modules.mk which does all the jobs.

# TODO inclusion check of the module itself like in c files

DMBS_BUILD_MODULES         += AES
DMBS_BUILD_TARGETS         += aes256-nessie-test
DMBS_BUILD_MANDATORY_VARS  += DMBS_PATH
DMBS_BUILD_OPTIONAL_VARS   +=
DMBS_BUILD_PROVIDED_VARS   +=
DMBS_BUILD_PROVIDED_MACROS +=

# Conditionally import the CORE module of DMBS if it is not already imported
ifeq ($(findstring CORE, $(DMBS_BUILD_MODULES)),)
    # Sanity check user supplied DMBS path
ifndef DMBS_PATH
    $(error Makefile Please include core.mk first or set DMBS_PATH)
endif

    include $(DMBS_PATH)/core.mk

    # TODO core.mk needs to check mandatory vars
endif

DMBS_MODULE_PATH := $(patsubst %/,%,$(dir $(lastword $(MAKEFILE_LIST))))

Actually you can leave out the conditionally include if core.mk already had an include guard (which is a good idea)

# Include Guard
ifeq ($(findstring AES, $(DMBS_BUILD_MODULES)),)

# Sanity check user supplied DMBS path
ifndef DMBS_PATH
$(error Makefile DMBS_PATH option cannot be blank)
endif
include $(DMBS_PATH)/core.mk

DMBS_BUILD_MODULES         += AES
DMBS_BUILD_TARGETS         += aes256-nessie-test
DMBS_BUILD_MANDATORY_VARS  += DMBS_PATH
DMBS_BUILD_OPTIONAL_VARS   +=
DMBS_BUILD_PROVIDED_VARS   +=
DMBS_BUILD_PROVIDED_MACROS +=

endif