include/debug.h: Move to include/nuttx/debug.h#18660
Open
PiyushPatle26 wants to merge 2 commits intoapache:masterfrom
Open
include/debug.h: Move to include/nuttx/debug.h#18660PiyushPatle26 wants to merge 2 commits intoapache:masterfrom
PiyushPatle26 wants to merge 2 commits intoapache:masterfrom
Conversation
github-actions
bot
added
Arch: arm
simbit18
requested review from
acassis,
anchao,
jerpelea,
linguini1,
lupyuen,
michallenc and
raiden00pl
PiyushPatle26
force-pushed
the
include/refactor-debug-h-location
branch
from
9921de7 to
968b6ed
Compare
PiyushPatle26
requested review from
Donny9,
adamfeuer,
antmerlino,
fdcavalcanti,
patacongo and
yf13
as code owners
Contributor
Author
|
@xiaoxiang781216 I have done the change you asked for! Can you help me with the CI |
Contributor
|
Please test by enabling the debug features in Kconfig. |
PiyushPatle26
force-pushed
the
include/refactor-debug-h-location
branch
from
968b6ed to
df05041
Compare
Contributor
Author
|
@linguini1 I ran as you asked, I received a bunch of warnings: Now , I have removed the unconditional deprecation #warning from include/debug.h |
Contributor
|
Doesn't the warning indicate that you haven't migrated all places to use the new header location? |
Contributor
Author
No |
Contributor
Author
@linguini1 I think the warning is from checkpatch on the modified files, not from a scan of remaining #include <debug.h> usages. |
Contributor
No, the warnings you pasted in your last comment are from compilation. If I understand correctly, this warning is inside the legacy debug.h header you kept for backwards compatibility? So if it's appearing, it means that header is still getting included somewhere. One inclusion is in nsh_console.c. Maybe you can open a twin PR to nuttx-apps to replace the includes there too |
Contributor
Author
|
@linguini1 I’ve already submitted the PR for nuttx_apps. Regarding the warnings, you’re right. My understanding is that the CI is failing because it checks against the current master of nuttx_apps, which doesn’t yet include the corresponding changes. |
Contributor
My bad, didn't realize! Could you please link to it in the description of this PR?
Hmmm, usually warnings do not cause CI to fail, but I'm on mobile right now so I can't check to give a reason. |
Contributor
Author
|
@linguini1 I have updated the description |
Contributor
Author
|
Hey @linguini1, |
Contributor
|
Hi @PiyushPatle26 see here https://github.com/apache/nuttx/actions/runs/23870320029/job/69600320022?pr=18660
|
Contributor
Author
This failures appear to be pre-existing issues in files not touched by this PR (e.g., |
Contributor
|
You'll need to fix them in this PR, ideally as a separate commit (?) The style checking tools only see what files you edited (which is a lot of them because of the header change), so existing formatting issues are being reported. It's not a result of your changes, but it would be best just to fix them in this PR. Also, if possible, use the "re-request review" button to get my review again instead of the @ mentions (unless it's a reply to something specific); it keeps the notifications in my inbox instead of push notifications :) |
debug.h is a NuttX-specific, non-POSIX header. Placing it in the top-level include/ directory creates naming conflicts with external projects that define their own debug.h. This commit moves the canonical header to include/nuttx/debug.h, following the NuttX convention for non-POSIX/non-standard headers, and updates all in-tree references. A backward-compatibility shim is left at include/debug.h that emits a deprecation #warning and re-includes <nuttx/debug.h>, allowing out-of-tree code to continue building while migrating. Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
PiyushPatle26
force-pushed
the
include/refactor-debug-h-location
branch
from
0cb2a03 to
9391494
Compare
PiyushPatle26
force-pushed
the
include/refactor-debug-h-location
branch
from
9391494 to
558048a
Compare
arch/arm/src/am335x/am335x_edid.c
Outdated
| #include <assert.h> | ||
| #include <errno.h> | ||
| #include <debug.h> | ||
| #include <nuttx/debug.h> |
Contributor
Author
There was a problem hiding this comment.
From which files do i need to remove now ?
Contributor
Author
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment.
#include <errno.h>
#include <nuttx/debug.h>
#include <nuttx/video/edid.h>
558048a to
ae4ce89
Compare
Fixed copespell errors, for CI sucess! Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
PiyushPatle26
force-pushed
the
include/refactor-debug-h-location
branch
from
ae4ce89 to
c5acdb8
Compare
Contributor
Author
|
@xiaoxiang781216 i have done the changes, anything else we must change ? Or the PR is ready to be merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: Please adhere to Contributing Guidelines.
Summary
Move the NuttX-specific
debug.hheader from the top-levelinclude/directory to
include/nuttx/debug.h, and update in-tree includes to use the new path.debug.his not a POSIX or standard C header. Keeping it at the top levelcreates avoidable naming conflicts with external projects that also provide a
debug.h. Moving it underinclude/nuttx/aligns it with NuttX conventions for non-standard headers.A backward-compatibility shim is left at
include/debug.hthat emits a deprecation#warningand re-includes
<nuttx/debug.h>, allowing out-of-tree code to continue building while migrating.Impact
include/nuttx/debug.h.<nuttx/debug.h>.<debug.h>.Other PR for nuttx_apps
Testing
Tested on Ubuntu Linux in the local NuttX tree.
Host:
Target:
Test application:
ostestI have also tested with
sim:nshconfiguration:Test logs
No board runtime testing was performed because this change is a mechanical
header relocation and include-path update.
Fixes #8986