Skip to content

cmake: Execute arch/CMakeLists.txt before subsys/CMakeList.txt #5609

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

Merged

Conversation

SebastianBoe
Copy link
Collaborator

@SebastianBoe SebastianBoe commented Jan 9, 2018

Re-order the execution of the arch/ and subsys/ CMakeLists.txt code to
work around a manifestation of issue #5605 . When OpenThread created an
External project in subsys it did not have access to important
toolchain flags added in arch/.

Intuitively, subsystems might depends on how the ARCH is configured,
but the ARCH shouldn't depend on any subsystem.

Signed-off-by: Sebastian Bøe [email protected]

@SebastianBoe SebastianBoe requested a review from nashif as a code owner January 9, 2018 09:57
CMakeLists.txt Outdated
@@ -297,8 +297,8 @@ add_subdirectory(misc)
include(misc/generated/CMakeLists.txt)
add_subdirectory(boards)
add_subdirectory(ext)
add_subdirectory(subsys)
add_subdirectory(arch)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like 'arch' should be first before boards, etc.. and we should add a comment about why its first here.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

I think arch should be first because the same reason for subsys needing compiler flags from it, I can see boards & ext, etc needing that as well.

We should have a comment that explains arch should be first and why.

Re-order the execution of the arch/ and subsys/ CMakeLists.txt code to
work around a manifestation of issue zephyrproject-rtos#6505. When OpenThread created an
External project in subsys, it did not have access to important
toolchain flags added in arch/.

Intuitively, subsystems might depends on how the ARCH is configured,
but the ARCH shouldn't depend on any subsystem.

Signed-off-by: Sebastian Bøe <[email protected]>
@SebastianBoe SebastianBoe force-pushed the cmake_reorder_arch_and_subsys branch from 89db4df to fe8ee98 Compare January 11, 2018 09:44
@codecov-io
Copy link

Codecov Report

Merging #5609 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5609      +/-   ##
==========================================
- Coverage   53.56%   53.55%   -0.01%     
==========================================
  Files         428      428              
  Lines       41191    41191              
  Branches     7856     7856              
==========================================
- Hits        22062    22060       -2     
- Misses      19018    19020       +2     
  Partials      111      111
Impacted Files Coverage Δ
arch/posix/core/posix_core.c 87.06% <0%> (-1.73%) ⬇️

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 df37588...fe8ee98. Read the comment docs.

@SebastianBoe
Copy link
Collaborator Author

I think arch should be first because the same reason for subsys needing compiler flags from it, I can see boards & ext, etc needing that as well.

We should have a comment that explains arch should be first and why.
-- @galak

@galak : Both points are now fixed.

@nashif nashif merged commit 6f946e2 into zephyrproject-rtos:master Jan 11, 2018
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.

4 participants