Skip to content

core: replace header guards with #pragma once#21405

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
JerelJr:fix-core-headers
Apr 16, 2025
Merged

core: replace header guards with #pragma once#21405
maribu merged 1 commit intoRIOT-OS:masterfrom
JerelJr:fix-core-headers

Conversation

@JerelJr
Copy link
Copy Markdown
Contributor

@JerelJr JerelJr commented Apr 12, 2025

Contribution description

Updated header files in the core directory and its subdirectories by switching from header guards to #pragma once

Testing procedure

I used this script to find and correct header files that need to be changed. Change RIOT_PATH to the directory to be fixed

Issues/PRs references

See issue #21335

@JerelJr JerelJr requested a review from kaspar030 as a code owner April 12, 2025 22:18
@github-actions github-actions bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Apr 12, 2025
@crasbe crasbe added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Process: needs >1 ACK Integration Process: This PR requires more than one ACK Process: missing approvals Integration Process: PR needs more ACKS (handled by action) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 12, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Apr 12, 2025 •
edited
Loading

Murdock results

✔️ PASSED

04dafd5 core: replace header guards with #pragma once

Success Failures Total Runtime
10299 0 10299 11m:30s

Artifacts

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Apr 14, 2025

Hello @JerelJr,

First all all welcome to the RIOT community and thank you for your first contribution. :)

It seems like the changes broke something, as the CI build does not complete anymore: https://ci.riot-os.org/details/9d47c2b748804fc4820232a5810c9d0e

Generally speaking, the core is the most critical part of RIOT and therefore requires special care when handling it, which is why I added the "needs >1 ACK" label.
Therefore it would've been better to start with an area with lower impact, but we'll see what we can do now.

Today the Hard Freeze will start and the Soft Freeze is already in place, so this is not a feature that could be merged before the next release: https://forum.riot-os.org/t/release-2025-04-soft-freeze-in-effect/4497/1

Perhaps you can look at the error in the CI system to try to locate which change broke the build in the meantime :)

@JerelJr
Copy link
Copy Markdown
Contributor Author

JerelJr commented Apr 14, 2025

Thank you for your feedback!

I started with core because it seemed like a relatively small amount of file changes, but I did not consider how critical that part of the code base is. I'll make sure to keep that in mind in the future. I will look into the CI output now to see what went wrong.

@maribu maribu changed the title Fix core headers core: use pragma once Apr 15, 2025
maribu
maribu reviewed Apr 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Would you mind squashing the commits into one?

@JerelJr
core: replace header guards with #pragma once
04dafd5 
core/lib/include/ringbuffer.h: removed trailing newline

core/lib/include/irq.h: restored header guard for irq.h to resolve build errors

core/lib/include/irq.h: added trailing newline to irq.h
@maribu maribu changed the title core: use pragma once core: replace header guards with #pragma once Apr 16, 2025
@maribu
Copy link
Copy Markdown
Member

maribu commented Apr 16, 2025

Since an agreement to use #pragma once has been reached in the weekly coordination, let's consider this an implicit 2nd ACK.

@maribu maribu removed Process: needs >1 ACK Integration Process: This PR requires more than one ACK Process: missing approvals Integration Process: PR needs more ACKS (handled by action) labels Apr 16, 2025
@maribu maribu enabled auto-merge April 16, 2025 05:30
@maribu maribu added this pull request to the merge queue Apr 16, 2025
Merged via the queue into RIOT-OS:master with commit 55f9d1c Apr 16, 2025
27 of 30 checks passed
@maribu
Copy link
Copy Markdown
Member

maribu commented Apr 16, 2025

Thx a lot!

@JerelJr
Copy link
Copy Markdown
Contributor Author

JerelJr commented Apr 16, 2025

Thanks for your review and feedback!

@JerelJr JerelJr deleted the fix-core-headers branch April 17, 2025 20:37
@maribu maribu mentioned this pull request May 5, 2025
Merged
7 tasks
crasbe added a commit to crasbe/RIOT that referenced this pull request May 14, 2025
@crasbe
dist/tools/headerguards: revert 082a37b
699eb28 
With the migration to #pragma once in the core header in RIOT-OS#21405,
the workaround for the header name check when using #include_next
is not necessary anymore and can be dropped.
crasbe added a commit to crasbe/RIOT that referenced this pull request May 14, 2025
@crasbe
dist/tools/headerguards: revert 082a37b
193b5e1 
With the migration to #pragma once in the core header in RIOT-OS#21405,
the workaround for the header name check when using #include_next
is not necessary anymore and can be dropped.
@N11cc00 N11cc00 mentioned this pull request May 16, 2025
Closed
8 tasks
crasbe
crasbe reviewed Sep 5, 2025
View reviewed changes
core/include/thread_flags.h
Comment on lines -57 to -59
#ifndef THREAD_FLAGS_H
#define THREAD_FLAGS_H

Copy link
Copy Markdown
Contributor

@crasbe crasbe Sep 5, 2025

Choose a reason for hiding this comment

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

This PR (partly) introduced a regression that if no empty line was present in front of the #ifndef, the end of the Doxygen comment block "touches" the #include.

I'm not 100% sure if this is worth fixing though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crasbe crasbe crasbe left review comments

@maribu maribu maribu approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

Assignees

No one assigned

Labels

Area: core Area: RIOT kernel. Handle PRs marked with this with care! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JerelJr @riot-ci @crasbe @maribu