Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Trying to remove recursive makefiles #7670

Closed
wants to merge 28 commits into from

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Oct 4, 2017

This relates to #1242

This PR adds support to build all the sys modules without requiring to change directories with make -C. It consists of 3 parts:

  1. Create static module directories definition for all sys modules
  2. Adapt Makefile.base to add local path to SRC and allow including it multiple times through different modules
  3. Add some magic to know current module makefile path
  4. In sys include all makefiles instead of changing directory

I took care that every commit keeps working (at least compilation of gcoap example`) and should be possible to understand by itself. So I would recommend reviewing commit by commit, you could find problems or have remarks without requiring to understand the whole process at once.

Also some cleanups/fixups could be put to different PRs to integrate them without the rest.

Static sys modules definition

I created Makefile.modules files that define mappings between modules name and absolute path to module directory.

I set MODULE_DIR_modulename = realpath_to_modulename_dir for all modules that where handled in this directory before. Modules whose directory is in $(MODULE)/Makefile are detected by a wildcard in the same way as before. The definition is done for all modules and not only the one in USEMODULE.

This way I can have a static definition of modules directory for all modules from sys.
I could then change sys/Makefile to include all the definitions and only set DIRS once.

Implementation details

I kept the mapping between module and module directory centralized as it was before. I just moved them to Makefile.modules files. Moving them to per module description file is another topic.

I chose to include all sub Makefile.modules in sys/Makefile and not in sys/Makefile.modules itself but it could be changed.

Make Makefile.base ready for non-recursive makefile

Non-recursive makefile means including all the modules definition before trying to execute anything.
This implies including Makefile.base multiple times as one per module. Also, it would be included from another directory than the module directory itself, so it cannot rely on CURDIR.

First step was adding the local directory when doing wildcard and in front of all sources for the build targets. The SRC variables are still without path to be compatible with the current way of setting SRC in module makefile. I used $(D) for the directory name so it is not too intrusive when using it.

Then, as some variables have lazy value management, they should be undefined at the end so they are not kept between modules.

Speaking about sharing values, some module define custom CFLAGS/CXXFLAGS so the build rules should use per module CFLAGS variables. I tried to keep the CXXUWFLAGS and CCASUWFLAGS working but did not really tested the impact. (reminder, the values in the build rule are evaluated at execution time).
I adapted the files I found to use a per module CFLAGS definition.

Non-recursive magic trick to get current directory

To detect the current module directory, I use the 'n - 1' included makefile which should be the including one. Except if Makefile.base was included after including another makefile in the module.
So, keeping it in this form makes it possible to break if module writer do not put enough care when writing.
A solution would be to put this magic into a Makefile.base.head included on top of each module.
But this changes many files so I would rather keep this for later in the PR.

To get the last makefile I also saw this from the linux-mag post where they have a _header.makfile on top.

_MAKEFILES := $(filter-out _header.mak _footer.mak,$(MAKEFILE_LIST))
_MODULE := $(patsubst %/,%,$(dir $(word $(words $(_MAKEFILES)),$(_MAKEFILES))))

http://webcache.googleusercontent.com/search?q=cache:LKx_cEDQepQJ:www.linux-mag.com/id/2101/&num=1&hl=en&gl=us&strip=1&vwsrc=0

This is the most magic part and it should be improved to make it reliable by design.

Using non recursive makefiles in sys

After paying attention that Makefile.base should be included first. It just means replacing DIRS += by include the directories.

Tricks that make it work by chance

Its working now because the build system does make -C DIR all in every directory and that Makefile.base adds $(MODULE).a as a target for all.
Also, DIRS is unset when entering a module so everything works as expected.

Testing

Old and new builds output can be compared by saving the verbose output:

RIOT_VERSION_OVERRIDE='0' make QUIET=0 all clean | ../../../clean_build_output.sh  | tee $(git rev-parse --abbrev-ref HEAD | sed 's#/#_#g').out

And you can find my clean output script here https://gist.github.com/cladmi/29d87b0e66e505f3c6dcea62ed51fa8d it just cleans backslash lines, whitespace, and removes all 'Make -C, 'entering directory' and 'mkdir -p', and sorts the output.

@cgundogan
Copy link
Member

cool (:

@miri64
Copy link
Member

miri64 commented Oct 4, 2017

@cladmi is there any significant speed-up due to this?

@cladmi
Copy link
Contributor Author

cladmi commented Oct 4, 2017

@miri64: I did not do any measurement. My goal is to get rid of the need to run make clean which would allow speeding up a lot.

@miri64
Copy link
Member

miri64 commented Oct 4, 2017

@miri64: I did not do any measurement. My goal is to get rid of the need to run make clean which would allow speeding up a lot.

Yes, but if it speeds up stuff regardless, that would be awesome as well ;-)

@miri64
Copy link
Member

miri64 commented Oct 4, 2017

Just tested it myself (with gnrc_networking for samr21-xpro): The speed-up isn't really significant indeed.

@kaspar030 kaspar030 added Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Area: build system Area: Build system labels Oct 4, 2017
@cladmi
Copy link
Contributor Author

cladmi commented Oct 4, 2017

From looking a bit more into the buildsystem, my handling of SUBMODULES is broken. I do not define the MODULES_DIR_ variables for the submodules.
Also, I just pushed a fix because I was saying tinymt32 was the directory for prng_tinymt32. This was also wrong before but worked as tinymt32 is only pulled by prng_tinymt32.

@cladmi
Copy link
Contributor Author

cladmi commented Oct 4, 2017

For SUBMODULES, I will try adding stuff for core and see how it goes, and should also look at #7241 at the same time.

@cladmi cladmi force-pushed the pr/wip/rm_recursive_make branch 2 times, most recently from b4f69fc to 42dff3e Compare October 12, 2017 12:30
tinymt32 dir is required to build 'tinymt32' module, not 'prng_tinymt32'.

It was working because tinymt32 is only pulled as a prng_tinymt32 dependency.
Define MODULE_DIR_$(module) for each module in sys.
Modules with names matching a subdirectory are automatically created.
Others should be explicitely defined.

Makefile.modules are made to be importable from any directory.
Use MODULE_DIR_$(module) variables to populate DIRS from USEMODULE.
DIRS is now calculaded directly in the main Makefile.
..nothing targets removes print "make[2]: Nothing to be done for 'all'." when
changing directory.
However, because Makefile.base gets imported multiple times, its overrwritten.
This will allow setting modules specific FLAGS in non recursive mode.
… makefiles

By defining 'NONRECURSIVE', multiple modules Makefile can be included in the
same makefile and handled seperately.

WARNING:
If for any reason, a module includes another Makefile before Makefile.base it
will break the including makefile magic. So maybe it would be good separate it
to a Makefile.base.head and Makefile.base.tail for this kind of modules.
Define MODULE_DIR_$(module) for each module in drivers.

Makefile.modules are made to be importable from any directory.
@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Nov 15, 2017
@cladmi
Copy link
Contributor Author

cladmi commented Nov 21, 2017

I will close this for now as I am not going to go further on the whole project right now.

I think that this would require other cleanups steps to be useful:

  • Simply moving the DIRS out of application.inc.mk into the main Makefile.include as a first step and fix things
  • Moving module definitions in the module directory
  • Fixing the mix between command line parsing and execution
    • Right now there is a trick required to allow running make clean all -j and it constraints every goal in Makefile.include. With recursive makefile, every single goal should be said to depend on clean.
  • Fixing handling of NATIVEINCLUDES
  • Cleaning up the dynamic variables definitions (ifneq(,$(filter thing, $(USEMODULE))))
  • Make Makefile.base reliable without make clean

@cladmi cladmi closed this Nov 21, 2017
@cladmi cladmi added the State: archived State: The PR has been archived for possible future re-adaptation label Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Impact: major The PR changes a significant part of the code base. It should be reviewed carefully State: archived State: The PR has been archived for possible future re-adaptation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants