threadx icon indicating copy to clipboard operation
threadx copied to clipboard

Missing SYSTEM flag for PUBLIC include directories in CMakeLists.txt

Open MarkAtBosch opened this issue 2 years ago • 7 comments

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 avatar Jul 31 '23 07:07 MarkAtBosch

Gentle ping ... @goldscott @yuxin-azrtos

FYI @ryanwinter @gdelazzari

MarkAtBosch avatar Dec 06 '23 06:12 MarkAtBosch

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

williamelamie avatar Feb 05 '24 22:02 williamelamie

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?

MarkAtBosch avatar Feb 23 '24 15:02 MarkAtBosch

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 avatar Feb 23 '24 17:02 williamelamie

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: @.***>

williamelamie avatar Feb 27 '24 01:02 williamelamie

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 avatar Feb 27 '24 15:02 MarkAtBosch