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

DeviceTree: Convert 'generated_dts_board.h' into a source file #12680

Merged

Conversation

SebastianBoe
Copy link
Collaborator

extract_dts_includes.py has been generating DT output and then
concatenating it with fixup header files to create
'generated_dts_board.h'.

In this patch we instead introduce a source file named
'generated_dts_board.h' and have it #include the appropriate DT
output and fixup files.

This results in a simpler system because users can now read
'generated_dts_board.h' as C source code to see how fixup files and
generated DT output relate to each other. Whereas before they would
have to either read documentation or python code to gain the same
understanding.

Also, it reduces the scope and complexity of one of our most bloated
python scripts, extract_dts_includes.py.

Signed-off-by: Sebastian Bøe sebastian.boe@nordicsemi.no

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jan 24, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

scripts/dts/extract_dts_includes.py Outdated Show resolved Hide resolved
include/generated_dts_board.h Outdated Show resolved Hide resolved
include/generated_dts_board.h Outdated Show resolved Hide resolved
@SebastianBoe SebastianBoe force-pushed the simplify_fixup_mechanism branch 2 times, most recently from 679b0aa to ad8126a Compare January 24, 2019 09:37
@codecov-io
Copy link

codecov-io commented Jan 24, 2019

Codecov Report

Merging #12680 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12680   +/-   ##
=======================================
  Coverage   53.96%   53.96%           
=======================================
  Files         242      242           
  Lines       27671    27671           
  Branches     6720     6720           
=======================================
  Hits        14934    14934           
  Misses       9932     9932           
  Partials     2805     2805

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0289a41...0d6a61d. Read the comment docs.

@SebastianBoe SebastianBoe force-pushed the simplify_fixup_mechanism branch 2 times, most recently from 777a3bc to 040b873 Compare January 24, 2019 10:50
extract_dts_includes.py has been generating DT output and then
concatenating it with fixup header files to create
'generated_dts_board.h'.

In this patch we instead introduce a source file named
'generated_dts_board.h' and have it \#include the appropriate DT
output and fixup files.

This results in a simpler system because users can now read
'generated_dts_board.h' as C source code to see how fixup files and
generated DT output relate to each other. Whereas before they would
have to either read documentation or python code to gain the same
understanding.

Also, it reduces the scope and complexity of one of our most bloated
python scripts, extract_dts_includes.py.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
The header comment in 'generated_dts_board_unfixed.h' was including
'compatible' as a helpful text.

But something has been broken with how we extract 'compatible' because
it was being evaluated to 'q'. Giving a confusing header text:

Generated include file q

Since the mechanism is broken, and does not appear to be important, we
remove it.

Also, modernize how we generate multi-line strings.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
@galak galak merged commit 361daf8 into zephyrproject-rtos:master Jan 25, 2019
zephyr_compile_definitions(${fixup_source}="${${fixup_source}}")
set_property(GLOBAL APPEND PROPERTY
PROPERTY_LINKER_SCRIPT_DEFINES
-D${fixup_source}="${${fixup_source}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this isn't really correct. I'm now getting storm of:

/home/pfalcon/projects-3rdparty/Embedded/Zephyr/zephyr/include/generated_dts_board.h:19:10: error: #include expects "FILENAME" or <FI>
 #include DTS_BOARD_FIXUP_FILE
          ^~~~~~~~~~~~~~~~~~~~

when building MicroPython. And the error message is correct - #include argument should be string quotes. You generate it w/o quotes (the quotes you have are part of shell syntax, not of the value of the string.

# Add defines such that generated_dts_board.h knows which fixups
# are present and where they
# are. e.g. -DDTS_SOC_FIXUP_FILE=/path/to/fixup.h
zephyr_compile_definitions(${fixup_source}="${${fixup_source}}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, for compiler flags, the problem is really in this line. Changing it to:

-       zephyr_compile_definitions(${fixup_source}="${${fixup_source}}")
+       zephyr_compile_definitions(${fixup_source}="\\\"${${fixup_source}}\\\"")

Generates proper CFLAGS for MicroPython compilation. But when it comes to compiling Zephyr itself, the defines are over-quoted (extra \"), and it fails.

In other words, we need to extra-quote these defines for future output by zephyr_get_compile_definitions_for_lang_as_string, but don't need to quote for Zephyr's own needs. @SebastianBoe, my random patchings around cmake can't do that, looking for help with that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aurel32
Copy link
Collaborator

aurel32 commented Jan 26, 2019

Note that this commit also broke openthread support in Zephyr as OT is not compiled with the extra -D options.

@SebastianBoe
Copy link
Collaborator Author

@pfalcon , @aurel32 : I'm working on fixing the regressions.

SebastianBoe added a commit to SebastianBoe/zephyr that referenced this pull request Jan 28, 2019
A recent change in how the fixup mechanism works caused several
regressions. See
zephyrproject-rtos#12680 for more
details about the regressions.

The core of the matter is that using defines to pass on include paths
causes interopability issues when integrating with external build
systems.

To resolve this we re-write the fixup mechanism to instead generate an
aggregated fixup file that is ready to be included at build time. Then
no paths are passed through defines and we resolve the regressions
reported.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
nashif pushed a commit that referenced this pull request Jan 28, 2019
A recent change in how the fixup mechanism works caused several
regressions. See
#12680 for more
details about the regressions.

The core of the matter is that using defines to pass on include paths
causes interopability issues when integrating with external build
systems.

To resolve this we re-write the fixup mechanism to instead generate an
aggregated fixup file that is ready to be included at build time. Then
no paths are passed through defines and we resolve the regressions
reported.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants