X Tutup
Skip to content

Scons: Remove system includes#111346

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
Repiteo:scons/remove-system-includes
Oct 7, 2025
Merged

Scons: Remove system includes#111346
Repiteo merged 1 commit intogodotengine:masterfrom
Repiteo:scons/remove-system-includes

Conversation

@Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Oct 6, 2025

Implements the removal of system includes mentioned in the above PR, which will guarantee their include chains are properly tracked when building

@Repiteo Repiteo added this to the 4.6 milestone Oct 6, 2025
@Repiteo Repiteo requested a review from a team as a code owner October 6, 2025 23:00
@Repiteo Repiteo added the bug label Oct 6, 2025
@Repiteo Repiteo requested review from a team as code owners October 6, 2025 23:00
@Repiteo Repiteo force-pushed the scons/remove-system-includes branch from bb08e71 to 4b5d702 Compare October 6, 2025 23:00
@Repiteo Repiteo changed the title Scons/remove system includes Scons: Remove system includes Oct 7, 2025
@Repiteo Repiteo force-pushed the scons/remove-system-includes branch from 4b5d702 to b950247 Compare October 7, 2025 13:12
@Repiteo Repiteo requested review from akien-mga and removed request for a team October 7, 2025 13:12
@Repiteo Repiteo mentioned this pull request Oct 7, 2025
Comment on lines 839 to +840
#ifndef __EMSCRIPTEN__
#ifdef __GNUC__
#if defined(__GNUC__) && !defined(__clang__)
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that it's not a problem upstream, they must support compiling with Clang.

It seems like the #ifndef __EMSCRIPTEN__ here is meant to prevent adding these pragma too, so it could be dropped (and the change PR'ed upstream) as __clang__ is defined for Emscripten too.

@Repiteo Repiteo merged commit 4bfe73c into godotengine:master Oct 7, 2025
20 checks passed
@Repiteo Repiteo deleted the scons/remove-system-includes branch October 7, 2025 17:03
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.

2 participants

X Tutup