make: add targets to debug dependencies variables #12004

cladmi · 2019-08-13T14:32:14Z

Contribution description

Add a 'dependency_debug' and a 'DEPENDENCY_DEBUG=1' option for
'info-boards-supported' to save some variables used when resolving
dependencies.

This should help tracking changes introduced when refactoring the
dependency parsing.

Testing procedure

As described in the file documentation:

The goal is to get the value of variables used for dependency resolution
and the impact of refactoring

Files output can be generated through info-boards-supported evaluation with

DEPENDENCY_DEBUG=1 make info-boards-supported

And for each board in the normal make context with

BOARD=board_name make dependency_debug

To compare in an aggregated file, you can run in an application directory:

for board in $(make info-boards); do DEPENDENCY_DEBUG_OUTPUT_DIR=bin/info BOARD=${board} make dependency_debug; done; cat bin/info/* > bin/deps_info
DEPENDENCY_DEBUG=1 DEPENDENCY_DEBUG_OUTPUT_DIR=bin/info-global make info-boards-supported; cat bin/info-global/* > bin/deps_info-boards-supported
# And compare both files
diff -u bin/deps_info bin/deps_info-boards-supported

"Funny" things:

Right now, the quick "resolution" done by murdock to know which boards are supported is using a subset of the values it must be using.
USEMODULE does not have all the modules and some FEATURES_REQUIRED are absent.

It can easily be seen already with one board in examples/hello-world

BOARD=samr21-xpro make dependency_debug
BOARDS=samr21-xpro DEPENDENCY_DEBUG=1 make info-boards-supported
samr21-xpro
diff -u dependencies_info_samr21-xpro dependencies_info-boards-supported_samr21-xpro

--- dependencies_info_samr21-xpro       2019-08-13 16:22:54.301280964 +0200
+++ dependencies_info-boards-supported_samr21-xpro      2019-08-13 16:22:52.285264349 +0200
@@ -1,14 +1,14 @@
 BOARD = samr21-xpro
-CPU = samd21
-CPU_MODEL = samr21g18a
-CPU_FAM = samd21
+CPU =
+CPU_MODEL =
+CPU_FAM =
 FEATURES_PROVIDED = periph_adc periph_i2c periph_pwm periph_rtc periph_rtt periph_spi periph_timer periph_uart periph_usbdev riotboot puf_sram periph_cpuid periph_flashpage periph_flashpage_raw periph_flashpage_rwee periph_gpio periph_gpio_irq periph_uart_modecfg periph_pm cpp cpu_check_address
-FEATURES_REQUIRED = periph_uart periph_uart
+FEATURES_REQUIRED =
 FEATURES_OPTIONAL = periph_gpio periph_pm periph_gpio periph_pm
-FEATURES_USED = periph_gpio periph_pm periph_uart
+FEATURES_USED = periph_gpio periph_pm
 FEATURES_MISSING =
 FEATURES_CONFLICT =
 FEATURES_CONFLICTING =
-USEMODULE = auto_init board core core_msg cortexm_common cortexm_common_periph cpu newlib newlib_nano newlib_syscalls_default periph periph_common periph_gpio periph_pm periph_uart pm_layered sam0_common_periph stdio_uart sys
-DEFAULT_MODULE = board cpu core core_msg sys auto_init
+USEMODULE = periph_common periph_gpio periph_pm
+DEFAULT_MODULE =
 DISABLE_MODULE =

Issues/PRs references

Build dependencies - processing order issues Build dependencies - processing order issues #9913
- Tracking: move CPU/CPU_MODEL to Makefile.features Tracking: move CPU/CPU_MODEL to Makefile.features #11477

cladmi · 2019-08-13T14:35:34Z

~~I did not have a script that does a whole repository dump yet but I can write it and add it here if wanted (or in another PR).~~

Script added, I am running it right now.

cladmi · 2019-08-13T15:13:10Z

@fjmolinas you may be interested in this as I would like to use this for helping reviewing changes done with #11477

cladmi · 2019-08-14T09:05:06Z

Running the script took 130m on my machine though.

fjmolinas · 2019-08-14T10:05:17Z

Running the script took 130m on my machine though.

Can you give me the exact command you executed please.

cladmi · 2019-08-14T10:24:48Z

Sure, the command I used was:

time ./dist/tools/buildsystem_sanity_check/save_all_dependencies_resolution_variables.sh build/deps/master

With time to get the execution time and build/deps/master as base output directory.

cladmi · 2019-08-14T14:20:18Z

I will change something.

As I want to compare the output from the dependency_debug and info-boards-supported for large scale analysis I should not generate any file when the board is blacklisted/not in the whitelist.

Otherwise it makes comparing way harder as concatenating all files and diff is invalid.

cladmi · 2019-08-14T15:28:30Z

If one currently wants to compare the output of the deps_info and deps_info-boards-supported, I would recommend applying #12009 first (or deleting the output for tests/cpu_efm32_features) as the BOARD is ignored in the tests and also remove the tests/nordic_softdevice/bin/dependencies_info-boards-supported_reel file as it currently generates an error in Makefile.include before dependency resolution.

cladmi · 2019-08-15T12:46:16Z

I rebased due to conflicts in Makefile.include.

fjmolinas · 2019-09-24T15:16:49Z

This PR was tested while reviewing #12092.

cladmi · 2019-09-24T16:09:12Z

Now that #11470 is merged, I could rebase this PR and integrate the content of the generate_aggregated.sh script in the save_all_dependencies_resolution_variables.sh script.
The main use is to compare the difference between both implementations of dependency parsing.

I did not before as it required removing erroneous files and preferred to be explicit in the testing PRs.

fjmolinas · 2019-09-24T16:42:06Z

Now that #11470 is merged, I could rebase this PR and integrate the content of the generate_aggregated.sh script in the save_all_dependencies_resolution_variables.sh script.
The main use is to compare the difference between both implementations of dependency parsing.

Please do.

fjmolinas

Some general comments regarding code (shellcheck is happy :) ).

dist/tools/buildsystem_sanity_check/save_all_dependencies_resolution_variables.sh

fjmolinas · 2019-09-25T08:11:40Z

makefiles/dependencies_debug.inc.mk

+.PHONY: dependency_debug
+# Only generate the dependencies when the board is not disabled
+# This will allow comparing with the output of `info-boards-supported` more easily
+dependency_debug:


Just a style remark, why "_" most recipe we have use "-" as far as I can tell.

Indeed, will change.

fjmolinas · 2019-09-25T08:14:11Z

makefiles/dependencies_debug.inc.mk

+# The goal is to get the value of variables used for dependency resolution
+# and the impact of refactoring
+
+# Files output can be generated through `info-boards-supported` evaluation with


This returns incorrect resolution in some cases if I'm understanding right from your PR description, right?

The script returns the resolutions from the build system, but the build system returns incorrect parsing when doing info-boards-supported as it ignores the content of boards/cpu/Makefile.include that are currently required.

Maybe we need to add a note saying they are different and that there is a "correct" one, similar to your PR comment.

Sure I add this.

I will flip the definitions to have the "normal" one first.

cladmi · 2019-09-25T12:02:14Z

Please do.

I will add it and remove the documentation explaining how to do it.

cladmi · 2019-09-26T13:26:50Z

I still kept the documentation in makefiles/dependency_debug.inc.mk as I realized it makes sense on its own even with currently having the script in dist/tools as it gives direct usage documentation.

fjmolinas · 2019-10-04T14:38:52Z

I still kept the documentation in makefiles/dependency_debug.inc.mk as I realized it makes sense on its own even with currently having the script in dist/tools as it gives direct usage documentation.

Ok, although I think this and many tools should somehow have there documentation more exposed, e.g. in advanced-build-sysyem-tricks, but lets not stall because of this. Please squash.

cladmi · 2019-10-07T12:44:58Z

Ok, although I think this and many tools should somehow have there documentation more exposed, e.g. in advanced-build-sysyem-tricks, but lets not stall because of this. Please squash.

You are right. I squash and add a reference in there.

Add a 'dependency-debug' and a 'DEPENDENCY_DEBUG=1' option for 'info-boards-supported' to save some variables used when resolving dependencies. Print some some 'sorted' variables to simplify comparing the actual value when the parsing order changed. This should help tracking changes introduced when refactoring the dependency parsing.

Add a script saving all applications and boards dependency resolution variables and also aggregated files to compare between both dependencies handling. It is slow but should dump everything.

fjmolinas

I have a single comment for the new doc, otherwise I think this PR is ok. If you agree with my suggestion please squash.

fjmolinas · 2019-10-08T06:39:22Z

doc/doxygen/src/advanced-build-system-tricks.md

+Or with the version used by murdock to know supported boards to a
+`dependencies_info-boards-supported_board_name` file:


Or with the "quick" version used by murdock (this is an incomplete resolution, details in makefiles/dependencies_debug.inc.mk):

I changed it this way to still keep the rest of sentence.
I squash directly.

diff --git a/doc/doxygen/src/advanced-build-system-tricks.md b/doc/doxygen/src/advanced-build-system-tricks.md index b5f0e9b67..c7614a8be 100644 --- a/doc/doxygen/src/advanced-build-system-tricks.md +++ b/doc/doxygen/src/advanced-build-system-tricks.md @@ -58,8 +58,9 @@ Generate the variables dump with the normal dependency resolution to a BOARD=board_name make dependency-debug ~~~~~~~~~~~~~~~~~~~ -Or with the version used by murdock to know supported boards to a -`dependencies_info-boards-supported_board_name` file: +Or with the "quick" version used by murdock to know supported boards +(this is an incomplete resolution, details in `makefiles/dependencies_debug.inc.mk`) +to a `dependencies_info-boards-supported_board_name` file: ~~~~~~~~~~~~~~~~~~~ BOARDS=board_name DEPENDENCY_DEBUG=1 make info-boards-supported

Yep looks good!

Add short documentation for the dependency resolution debug targets. Point to the main files for more infos.

fjmolinas

ACK!

fjmolinas · 2019-10-08T15:41:31Z

GO!

cladmi · 2019-10-09T09:38:43Z

Thank you for the review!

cladmi mentioned this pull request Aug 13, 2019

Tracking: move CPU/CPU_MODEL to Makefile.features #11477

Closed

18 tasks

This was referenced Aug 14, 2019

makefiles/info-global.inc.mk: bug: fix DISABLE_MODULE_GLOBAL value #12010

Merged

makefiles/info-global.inc.mk: bug: honor DEFAULT_MODULES #12011

Merged

cladmi force-pushed the pr/make/dependencies/debug_targets branch from 0ecad5f to a6b45a0 Compare August 15, 2019 12:45

cladmi mentioned this pull request Aug 15, 2019

boards: move CPU/CPU_MODEL definition to Makefile.features #12014

Merged

cladmi force-pushed the pr/make/dependencies/debug_targets branch 2 times, most recently from a6b45a0 to 19f538e Compare August 22, 2019 11:07

cladmi mentioned this pull request Aug 26, 2019

boards with side-effect: move CPU/CPU_MODEL definition to Makefile.features #12092

Merged

miri64 added Area: build system Area: Build system Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Sep 10, 2019

fjmolinas reviewed Sep 25, 2019

View reviewed changes

buildsystem_sanity_check: script to save deps resolution variables

7d0b922

Add a script saving all applications and boards dependency resolution variables and also aggregated files to compare between both dependencies handling. It is slow but should dump everything.

cladmi force-pushed the pr/make/dependencies/debug_targets branch from b4a49c7 to 7d0b922 Compare October 7, 2019 12:48

fjmolinas reviewed Oct 8, 2019

View reviewed changes

doc: add documentation on dependency debug targets

9940a15

Add short documentation for the dependency resolution debug targets. Point to the main files for more infos.

cladmi force-pushed the pr/make/dependencies/debug_targets branch from 040df2f to 9940a15 Compare October 8, 2019 09:26

fjmolinas approved these changes Oct 8, 2019

View reviewed changes

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 8, 2019

fjmolinas merged commit 1c5c027 into RIOT-OS:master Oct 8, 2019

cladmi deleted the pr/make/dependencies/debug_targets branch October 9, 2019 09:39

kb2ma added this to the Release 2019.10 milestone Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make: add targets to debug dependencies variables #12004

make: add targets to debug dependencies variables #12004

cladmi commented Aug 13, 2019

cladmi commented Aug 13, 2019 •

edited

Loading

cladmi commented Aug 13, 2019

cladmi commented Aug 14, 2019

fjmolinas commented Aug 14, 2019

cladmi commented Aug 14, 2019

cladmi commented Aug 14, 2019

cladmi commented Aug 14, 2019 •

edited

Loading

cladmi commented Aug 15, 2019

fjmolinas commented Sep 24, 2019

cladmi commented Sep 24, 2019

fjmolinas commented Sep 24, 2019

fjmolinas left a comment •

edited

Loading

fjmolinas Sep 25, 2019

cladmi Sep 25, 2019

fjmolinas Sep 25, 2019

cladmi Sep 25, 2019

fjmolinas Sep 25, 2019

cladmi Sep 26, 2019

cladmi Sep 26, 2019

cladmi commented Sep 25, 2019

cladmi commented Sep 26, 2019

fjmolinas commented Oct 4, 2019

cladmi commented Oct 7, 2019

fjmolinas left a comment

fjmolinas Oct 8, 2019

cladmi Oct 8, 2019

fjmolinas Oct 8, 2019

fjmolinas left a comment

fjmolinas commented Oct 8, 2019

cladmi commented Oct 9, 2019

		Or with the version used by murdock to know supported boards to a
		`dependencies_info-boards-supported_board_name` file:

make: add targets to debug dependencies variables #12004

make: add targets to debug dependencies variables #12004

Conversation

cladmi commented Aug 13, 2019

Contribution description

Testing procedure

"Funny" things:

Issues/PRs references

cladmi commented Aug 13, 2019 • edited Loading

cladmi commented Aug 13, 2019

cladmi commented Aug 14, 2019

fjmolinas commented Aug 14, 2019

cladmi commented Aug 14, 2019

cladmi commented Aug 14, 2019

cladmi commented Aug 14, 2019 • edited Loading

cladmi commented Aug 15, 2019

fjmolinas commented Sep 24, 2019

cladmi commented Sep 24, 2019

fjmolinas commented Sep 24, 2019

fjmolinas left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cladmi commented Sep 25, 2019

cladmi commented Sep 26, 2019

fjmolinas commented Oct 4, 2019

cladmi commented Oct 7, 2019

fjmolinas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas commented Oct 8, 2019

cladmi commented Oct 9, 2019

cladmi commented Aug 13, 2019 •

edited

Loading

cladmi commented Aug 14, 2019 •

edited

Loading

fjmolinas left a comment •

edited

Loading