make: add possibility to provide board specific application dependencies in a separate Makefile

[aabadie]

Contribution description

This PR adds a way for providing complex (for example board related) application dependencies in a separate Makefile.dep in the application directory.

There is a direct consequence on the behaviour of the info-boards-supported target: when the Makefile.dep is provided, the result of this target the same one as the expected one returned by the build system. In short, this PR allows for fixing a weak point of info-boards-supported.

This was the cleanest solution to solve build/ci issues in #12724 and #12304 (in fact these PRs include the commit provided here).

Testing procedure

• A green Murdock
• make info-boards-supported is unchanged in the modified test applications.

Issues/PRs references

Required by #12304 and #12724

[fjmolinas]

Testing procedure.

git checkout pr-12755 -f; for app in tests/gnrc_sock_dns tests/gnrc_tcp tests/gnrc_rpl_srh tests/gnrc_ipv6_ext_frag tests/gnrc_ipv6_ext; do make --no-print-directory -C ${app} info-boards-supported; done > pr.txt; git checkout upstream/master; for app in tests/gnrc_sock_dns tests/gnrc_tcp tests/gnrc_rpl_srh tests/gnrc_ipv6_ext_frag tests/gnrc_ipv6_ext; do make --no-print-directory -C ${app} info-boards-supported; done > master.txt; diff pr.txt master.txt

NOTE: the only reason why there is no change is because netdev_tap is not a FEATURE and stdio_ethos only require periph_uart which is already supported by all whitelisted boards. So the weak point is not exposed here.

[fjmolinas]

This looks good to me, Its something that had already been mentioned in the past and that I had though of doing as well. @miri64 you had concerns in #12724 regarding your workflow, this does not do it so I assume you are OK with this.

[fjmolinas]

@aabadie maybe add a static check so no

ifeq (native,$(BOARD))
  USEMODULE += ...
  DISABLE_MODULE += ...
  FEATURES_% += ...
  ...
endif

happens in examples/Makefile or tests/Makefile.

[miri64]

Why does moving them to a separate Makefile make make info-boards-supported possible with keeping native the default board while it doesn't work when it is in the main Makefile? What difference does it make if I say USEMODULE for a certain board in the Makefile of the application directly vs. collecting it in the dependency collection phase (except for making it more complicated)? This should work without any of these special handlings. Or am I not understanding our build system at all... :-/?

[miri64]

Don't get me wrong, I really appreciate that you guys try to get rid of BOARD_BLACKLIST, but don't forget the main goal here: this should make it easier for the user. At the moment this more looks like another heap of patchwork to me.

[kaspar030]

Why does moving them to a separate Makefile make make info-boards-supported possible with keeping native the default board while it doesn't work when it is in the main Makefile? What difference does it make if I say USEMODULE for a certain board in the Makefile of the application directly vs. collecting it in the dependency collection phase (except for making it more complicated)?

The "info-boards-supported" machinery is executed after the application makefile. Thus any logic within the application logic has been applied. That includes any conditionals based on the (default) board.

E.g., if the application makefile has BOARD ?= native; if BOARD==native: FEATURES_REQUIRED+=foobar, foobar is required for all boards from the perspective of info-boards-supported, as FEATURES_REQUIRED+=foobar has been evaluated before including the corresponding makefiles/info-global.inc.mk. (that is btw the file I use in presentations about why make sucks for logic 😉).

[miri64]

The "info-boards-supported" machinery is executed after the application makefile. Thus any logic within the application logic has been applied. That includes any conditionals based on the (default) board.

E.g., if the application makefile has BOARD ?= native; if BOARD==native: FEATURES_REQUIRED+=foobar, foobar is required for all boards from the perspective of info-boards-supported, as FEATURES_REQUIRED+=foobar has been evaluated before including the corresponding makefiles/info-global.inc.mk. (that is btw the file I use in presentations about why make sucks for logic wink).

And how does adding this to a separate Makefile.dep change that? info-boards-supported would still be executed after the dependencies are resolved, right?

[kaspar030]

And how does adding this to a separate Makefile.dep change that? info-boards-supported would still be executed after the dependencies are resolved, right?

No, the separate Makefile.dep is be included from the main Makefile.dep, which is in turn evaluated from info-boards-supported, which sets the correct BOARD value in each iteration.

[miri64]

I think I slowly start to understand...

[miri64]

Ok, I'm fine with the changes, but don't have the time to test this right now. There should however be some doc somewhere, clarifying when a Makefile.dep for the application is needed and when not. I can imagine a newcomer reading tests/gnrc_ipv6_ext (because I want to write support for a new extension header e.g.) would get the impression that Makefile.dep is where the modules are defined in. However, this isn't always the case so this needs to be explained properly somewhere.

[kaspar030]

There should however be some doc somewhere, clarifying when a Makefile.dep for the application is needed and when not.

+1. IMO, rule of thumb should be, only use Makefile.dep for the necessary rules.

[miri64]

+1. IMO, rule of thumb should be, only use Makefile.dep for the necessary rules.

Yepp, but it is very important to state what those "necessary rules" are ;-)

[aabadie]

rule of thumb should be, only use Makefile.dep for the necessary rules.

Ok, I think that this PR is now following this rule. In each application Makefile.dep, I added a comment that explain what should be added there.

[aabadie]

Let me know if/when I can squash ;)

[miri64]

Still don't have any confirmation what those "necassary rules" are. Also, as there is a clear distinction between the Makefile.dep we use elsewhere (all dependencies in this scope) and those application Makefile.dep (only special dependencies in this scope), I think we should use different name, to avoid confusion (not sure what, as I don't know chat the rules for that files are).

[miri64]

[…] (not sure what, as I don't know chat the rules for that files are).

Maybe Makefile.board_dep (if this goes into the direction I think it is going).

[aabadie]

what those "necassary rules" are

As I understood @kaspar030,'s comment, these are only the rules that are board specific and defined in the application Makefile.
Remember that Makefile.dep in the application directory is only there to ensure board dependent dependencies defined at application level are also taken into account by the info-board-supported target (because the $(APPDIR)/Makefile.dep is parsed).
In the current state (e.g. in master), the application Makefile is not parsed by the info-board-supported target: any board specific dependency that has hardware requirements (for example ethos here) is not evaluated.
The new Makefile.dep is not different than other Makefile.dep, it's just filling a hole to make info-board-supported target consistent with the "normal" dependency resolution mechanism.

[miri64]

The new Makefile.dep is not different than other Makefile.dep, it's just filling a hole to make info-board-supported target consistent with the "normal" dependency resolution mechanism.

If that were true, all USEMODULE of an application should go there. But you just made clear, that this isn't the case. And I'm also sure that we don't want that.

[aabadie]

But you just made clear, that this isn't the case

It seems to me that you are misunderstanding how make info-board-supported works. You should have a look at the code, that would help.

Of course, the application Makefile is parsed, so all USEMODULE, FEATURES_REQUIRED are parsed but not the ones that are dependent on a board (e.g. inside an ifeq ($(BOARD), <board>) block) that is not the default board.

We need to handle that board specific logic in a an extra file that is parsed by the normal build and by info-board-supported.

[miri64]

But you just made clear, that this isn't the case

It seems to me that you are misunderstanding how make info-board-supported works. You should have a look at the code, that would help.

Of course, the application Makefile is parsed, so all USEMODULE, FEATURES_REQUIRED are parsed but not the ones that are dependent on a board (e.g. inside an ifeq ($(BOARD), <board>) block) that is not the default board.

We need to handle that board specific logic in a an extra file that is parsed by the normal build and by info-board-supported.

I think there is a misunderstanding, yes, but not about functional things. I'm arguing that the name Makefile.dep is misleading, as it could imply to a user, that all dependencies of an application are supposed to be in that file. Which isn't the case, as there are also USEMODULE and USEPKG assignments in the main Makefile. So my argument is completely non-functional.

[miri64]

(and my point is: we should rename Makefile.dep to something else, as they are obviously more special [from a purely non-functional view] as normal Makefile.dep files)

[aabadie]

@miri64, ok to squash ?

[miri64]

For my part, yes.

[aabadie]

For my part, yes.

Thanks, then I squashed.

[aabadie]

This PR could also be considered as an improvement for #9913

[fjmolinas]

No changes in dependencies. ACK!

git checkout pr-12755 -f; for app in tests/gnrc_sock_dns tests/gnrc_tcp tests/gnrc_rpl_srh tests/gnrc_ipv6_ext_frag tests/gnrc_ipv6_ext; do make --no-print-directory -C ${app} info-boards-supported; done > pr.txt; git checkout upstream/master; for app in tests/gnrc_sock_dns tests/gnrc_tcp tests/gnrc_rpl_srh tests/gnrc_ipv6_ext_frag tests/gnrc_ipv6_ext; do make --no-print-directory -C ${app} info-boards-supported; done > master.txt; diff pr.txt master.txt
#no diff

[fjmolinas]

GO!