{"action":"created","issue":{"url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/7670","repository_url":"https://api.github.com/repos/RIOT-OS/RIOT","labels_url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/7670/labels{/name}","comments_url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/7670/comments","events_url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/7670/events","html_url":"https://github.com/RIOT-OS/RIOT/pull/7670","id":262742996,"number":7670,"title":"WIP Trying to remove recursive makefiles","user":{"login":"cladmi","id":241401,"avatar_url":"https://avatars3.githubusercontent.com/u/241401?v=4","gravatar_id":"","url":"https://api.github.com/users/cladmi","html_url":"https://github.com/cladmi","followers_url":"https://api.github.com/users/cladmi/followers","following_url":"https://api.github.com/users/cladmi/following{/other_user}","gists_url":"https://api.github.com/users/cladmi/gists{/gist_id}","starred_url":"https://api.github.com/users/cladmi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cladmi/subscriptions","organizations_url":"https://api.github.com/users/cladmi/orgs","repos_url":"https://api.github.com/users/cladmi/repos","events_url":"https://api.github.com/users/cladmi/events{/privacy}","received_events_url":"https://api.github.com/users/cladmi/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2017-10-04T10:46:29Z","updated_at":"2017-10-04T12:08:25Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/RIOT-OS/RIOT/pulls/7670","html_url":"https://github.com/RIOT-OS/RIOT/pull/7670","diff_url":"https://github.com/RIOT-OS/RIOT/pull/7670.diff","patch_url":"https://github.com/RIOT-OS/RIOT/pull/7670.patch"},"body":"This relates to https://github.com/RIOT-OS/RIOT/issues/1242\r\n\r\nThis PR adds support to build all the `sys` modules without requiring to change directories with `make -C`. It consists of 3 parts:\r\n\r\n1. Create static module directories definition for all `sys` modules\r\n2. Adapt `Makefile.base` to add local path to SRC and allow including it multiple times through different modules\r\n3. Add some magic to know current module makefile path\r\n4. In `sys` include all makefiles instead of changing directory\r\n\r\nI 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.\r\n\r\nAlso some cleanups/fixups could be put to different PRs to integrate them without the rest.\r\n\r\n### Static sys modules definition\r\n\r\nI created `Makefile.modules` files that define mappings between modules name and absolute path to module directory.\r\n\r\nI 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. \r\n\r\nThis way I can have a static definition of modules directory for all modules from `sys`.\r\nI could then change `sys/Makefile` to include all the definitions and only set `DIRS` once.\r\n\r\n#### Implementation details\r\n\r\nI 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.\r\n\r\nI chose to include all sub `Makefile.modules` in `sys/Makefile` and not in `sys/Makefile.modules` itself but it could be changed.\r\n\r\n### Make `Makefile.base` ready for non-recursive makefile\r\n\r\nNon-recursive makefile means including all the modules definition before trying to execute anything.\r\nThis 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`.\r\n\r\nFirst 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.\r\n\r\nThen, as some variables have `lazy` value management, they should be undefined at the end so they are not kept between modules.\r\n\r\nSpeaking 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).\r\nI adapted the files I found to use a per module CFLAGS definition.\r\n\r\n### Non-recursive magic trick to get current directory\r\n\r\nTo 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.\r\nSo, keeping it in this form makes it possible to break if module writer do not put enough care when writing.\r\nA solution would be to put this magic into a `Makefile.base.head` included on top of each module.\r\nBut this changes many files so I would rather keep this for later in the PR.\r\n\r\nTo get the last makefile I also saw this from the linux-mag post where they have a `_header.mak`file on top.\r\n```\r\n_MAKEFILES := $(filter-out _header.mak _footer.mak,$(MAKEFILE_LIST))\r\n_MODULE := $(patsubst %/,%,$(dir $(word $(words $(_MAKEFILES)),$(_MAKEFILES))))\r\n```\r\nhttp://webcache.googleusercontent.com/search?q=cache:LKx_cEDQepQJ:www.linux-mag.com/id/2101/&num=1&hl=en&gl=us&strip=1&vwsrc=0\r\n\r\nThis is the most magic part and it should be improved to make it reliable by design.\r\n\r\n\r\n### Using non recursive makefiles in `sys`\r\n\r\nAfter paying attention that  `Makefile.base` should be included first. It just means replacing `DIRS +=` by include the directories.\r\n\r\n### Tricks that make it work by chance\r\n\r\nIts 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`.\r\nAlso, DIRS is unset when entering a module so everything works as expected.\r\n\r\n\r\n### Testing\r\n\r\nOld and new builds output can be compared by saving the verbose output:\r\n```\r\nRIOT_VERSION_OVERRIDE='0' make QUIET=0 all clean | ../../../clean_build_output.sh  | tee $(git rev-parse --abbrev-ref HEAD | sed 's#/#_#g').out\r\n```\r\nAnd 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."},"comment":{"url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/comments/334134681","html_url":"https://github.com/RIOT-OS/RIOT/pull/7670#issuecomment-334134681","issue_url":"https://api.github.com/repos/RIOT-OS/RIOT/issues/7670","id":334134681,"user":{"login":"miri64","id":675644,"avatar_url":"https://avatars1.githubusercontent.com/u/675644?v=4","gravatar_id":"","url":"https://api.github.com/users/miri64","html_url":"https://github.com/miri64","followers_url":"https://api.github.com/users/miri64/followers","following_url":"https://api.github.com/users/miri64/following{/other_user}","gists_url":"https://api.github.com/users/miri64/gists{/gist_id}","starred_url":"https://api.github.com/users/miri64/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/miri64/subscriptions","organizations_url":"https://api.github.com/users/miri64/orgs","repos_url":"https://api.github.com/users/miri64/repos","events_url":"https://api.github.com/users/miri64/events{/privacy}","received_events_url":"https://api.github.com/users/miri64/received_events","type":"User","site_admin":false},"created_at":"2017-10-04T12:08:25Z","updated_at":"2017-10-04T12:08:25Z","author_association":"OWNER","body":"Just tested it myself (with `gnrc_networking` for `samr21-xpro`): The speed-up isn't really significant indeed."}}
{"actor":{"display_login":"miri64"}}