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.

Sign up for GitHub

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

Jump to bottom

gn_build.sh compilation failure happens after PR 7080 landed #7200

Closed
yunhanw-google opened this issue May 27, 2021 · 1 comment
Closed

gn_build.sh compilation failure happens after PR 7080 landed #7200

yunhanw-google opened this issue May 27, 2021 · 1 comment
Assignees
@mspang

Comments

@yunhanw-google
Copy link
Contributor

Problem

see gn_build.sh compilation failure happens after PR 7080 landed, after reverting, it works.

-- Build: GN configure
'--------------------------------
ERROR at //examples/platform/linux/AppMain.cpp:28:11: Include not allowed.
#include <lib/shell/Engine.h>
^-----------------
It is not in any dependency of
//examples/platform/linux:app-main
The include file is in the target(s):
//src/lib/shell:shell_core
which should somehow be reachable.
@turon turon mentioned this issue May 27, 2021
Closed
@mspang mspang assigned mspang and unassigned gjc13 May 27, 2021
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue May 27, 2021
@bzbarsky-apple
Revert "[shell] enable shell as an optional module in apps (project-c…
309e7b8 
…hip#7080)"

This reverts commit c8f286f because
the result does not compile.

Fixes project-chip#7200
@bzbarsky-apple bzbarsky-apple mentioned this issue May 27, 2021
Closed
mspang added a commit to mspang/connectedhomeip that referenced this issue May 27, 2021
@mspang
Fix dependency checker in the unified build
9694931 
Add a preprocessor condition to avoid including <lib/shell/Engine.h>
when shell integration is disabled.

Unfortunately, since the checker doesn't preprocess the code, this
results in a false positive. Suppress that with // nogncheck.

fixes project-chip#7200
@mspang mspang mentioned this issue May 27, 2021
Closed
@mspang
Copy link
Contributor

mspang commented May 27, 2021

This requires a bit of understanding how the checker works

  • Only headers that belong to targets in loaded build files are diagnosed. So if a build file is not loaded, there will be no errors regarding its headers
  • In this case the shell build file is not loaded in CI because the feature is disabled by default
  • The unified configuration happens to load the shell build file through another dependency path, allowing the error to be diagnosed

Adding a preprocessor condition around the #include makes the code correct. Unfortunately, another limitation of the checking is that it doesn't preprocess the code so there will be a false positive. For such cases, there is a suppression available - the conditional include can be tagged with // nogncheck

mspang added a commit to mspang/connectedhomeip that referenced this issue May 27, 2021
@mspang
Fix dependency checker in the unified build
f40dea6 
Add a preprocessor condition to avoid including <lib/shell/Engine.h>
when shell integration is disabled.

Unfortunately, since the checker doesn't preprocess the code, this
results in a false positive. Suppress that with // nogncheck.

fixes project-chip#7200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mspang mspang

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@mspang @gjc13 @yunhanw-google