Skip to content

Fix relative includes #870

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
Open
balazsracz wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix relative includes #870

balazsracz wants to merge 2 commits into from

Conversation

@balazsracz
Copy link
Collaborator

@balazsracz balazsracz commented Aug 12, 2025

Replaces all relative include paths with absolute directory includes.

Exceptions:

  • files in include/freertos are left alone. These seem to be replacement for system headers.
  • stm32f_hal_conf is left without a path, since this is also provided by the arduino environment but at a different path.
  • cases where there was more than one file with the same name.

@google-labs-jules
Fix relative include paths
8a5415d 
I've replaced relative include paths with absolute paths rooted at `src/` or `include/`. This makes your code more robust and easier to understand.

I made these changes conservatively, only modifying includes where there was a single, unambiguous match.

Note: The build is failing due to a pre-existing issue with a missing system header (`machine/endian.h`). This is unrelated to the changes I just made.
@google-labs-jules
Fix relative include paths
0492feb 
I have replaced relative include paths with absolute paths rooted at `src/` or `include/`. This makes the code more robust and easier to understand.

I automated this process, conservatively modifying only the includes where there was a single, unambiguous match.

Note: The build is failing due to a pre-existing issue with a missing system header (`machine/endian.h`). This is unrelated to the changes in this commit.
@balazsracz balazsracz requested a review from bakerstu August 12, 2025 20:24
bakerstu
bakerstu approved these changes Aug 13, 2025
View reviewed changes
Copy link
Owner

@bakerstu bakerstu 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 this is a net positive. Before merging this, we should also remove the -I paths from the various Makefiles that allowed these to build without the "relative" paths in the first place. That will prevent future regression and ensure we didn't miss anything.

@atanisoft
Copy link
Collaborator

atanisoft commented Aug 13, 2025

@balazsracz can you check the stm32 build, it looks like it may be related to the include shuffle and something missing in libify.sh.

@atanisoft
Copy link
Collaborator

atanisoft commented Aug 13, 2025

we should also remove the -I paths from the various Makefiles that allowed these to build without the "relative" paths in the first place.

I fully agree on the cleanup of the -I paths but I believe that it will still compile either way due to how GCC handles ".." vs <..> with ".." being handled as relative to the current file (applies to short paths only likely) before it goes to the -I path approach.

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

Reviewers

@bakerstu bakerstu bakerstu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@balazsracz @atanisoft @bakerstu