eclipse-threadx / threadx

Eclipse ThreadX is an advanced real-time operating system (RTOS) designed specifically for deeply embedded applications.
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/threadx/index.md
MIT License
2.92k stars 797 forks source link

Missing SYSTEM flag for PUBLIC include directories in CMakeLists.txt #290

Open MarkAtBosch opened 1 year ago

MarkAtBosch commented 1 year ago

Unfortunately, ThreadX 6.1.12 fixed issue #138 CMake project include directories should be "system" ones only partially. The problem was fixed in common/CMakeLists.txt but not in all other CMakeLists.txt files which publish parts of the ThreadX API.

Changes in common/CMakeLists.txt at commit 8c3c08f10826c4d5332a6d8e2fdf014b793a456a:

@@ -202,6 +202,7 @@ target_sources(${PROJECT_NAME}

   # Add the Common/inc directory to the project include list
   target_include_directories(${PROJECT_NAME} 
+      SYSTEM
       PUBLIC 
       ${CMAKE_CURRENT_LIST_DIR}/inc
   )

At least for all CMakeLists.txt files in ports/THREADX_ARCH/THREADX_TOOLCHAIN the same fix is required to handle all ThreadX include directories as system includes and thus compiler warnings will be skipped.

MarkAtBosch commented 11 months ago

Gentle ping ... @goldscott @yuxin-azrtos

FYI @ryanwinter @gdelazzari

williamelamie commented 9 months ago

Hi Mark, Yuxin is with me over at rtosx.com (px5rtos.com).... I'll ask him tomorrow about this.

MarkAtBosch commented 8 months ago

Hi Mark, Yuxin is with me over at rtosx.com (px5rtos.com).... I'll ask him tomorrow about this.

Hi @williamelamie and @yuxin-azrtos, any news on this?

williamelamie commented 8 months ago

Hi Mark, I'm sorry about that... Hope to have some feedback early next week. Best regards, Bill

On Fri, Feb 23, 2024 at 7:47 AM MarkAtBosch @.***> wrote:

Hi Mark, Yuxin is with me over at rtosx.com (px5rtos.com).... I'll ask him tomorrow about this.

Hi @williamelamie https://github.com/williamelamie and @yuxin-azrtos https://github.com/yuxin-azrtos, any news on this?

— Reply to this email directly, view it on GitHub https://github.com/eclipse-threadx/threadx/issues/290#issuecomment-1961565488, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3YCNAPBCSX6MAWSTBQDB7DYVC2Y3AVCNFSM6AAAAAA253KXKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRRGU3DKNBYHA . You are receiving this because you were mentioned.Message ID: @.***>

williamelamie commented 8 months ago

Hi Mark,

I've discussed this with Yuxin, and we believe what you proposed earlier in the thread is correct. Is your request simply that it needs be updated in the repository?

Best regards,

Bill

Please also feel free to reach out to me at @.***

On Fri, Feb 23, 2024 at 7:47 AM MarkAtBosch @.***> wrote:

Hi Mark, Yuxin is with me over at rtosx.com (px5rtos.com).... I'll ask him tomorrow about this.

Hi @williamelamie https://github.com/williamelamie and @yuxin-azrtos https://github.com/yuxin-azrtos, any news on this?

— Reply to this email directly, view it on GitHub https://github.com/eclipse-threadx/threadx/issues/290#issuecomment-1961565488, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3YCNAPBCSX6MAWSTBQDB7DYVC2Y3AVCNFSM6AAAAAA253KXKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRRGU3DKNBYHA . You are receiving this because you were mentioned.Message ID: @.***>

MarkAtBosch commented 8 months ago

Hi @williamelamie,

Yes, it is still my recommendation to apply the fix from common/CMakeLists.txt at commit 8c3c08f10826c4d5332a6d8e2fdf014b793a456a

@@ -202,6 +202,7 @@ target_sources(${PROJECT_NAME}

   # Add the Common/inc directory to the project include list
   target_include_directories(${PROJECT_NAME} 
+      SYSTEM
       PUBLIC 
       ${CMAKE_CURRENT_LIST_DIR}/inc
   )

at least into the CMakeLists.txt files in ports/THREADX_ARCH/THREADX_TOOLCHAIN.

Essentially, the fix should be applied to all CMakeLists.txt files which publish parts of the ThreadX API.

Best regards, Mark

MarkAtBosch commented 5 months ago

Hi @williamelamie and @yuxin-azrtos, any news on this?