zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.12k stars 6.21k forks source link

`find_package` goes into an infinite loop on windows #24909

Closed CrustyAuklet closed 4 years ago

CrustyAuklet commented 4 years ago

Describe the bug when using CMake to build an Zephyr project on windows (without west) the find_package goes into an infinite loop.

This is caused by the following code, since on windows we never reach / but instead C:/ or D:/ etc.

https://github.com/zephyrproject-rtos/zephyr/blob/fc828fde6feb4fe1b849877e0546b2e3f7fc0f80/share/zephyr-package/cmake/zephyr_package_search.cmake#L15

I fixed the issue on my machine by changing the line to while(NOT ((SEARCH_PATH STREQUAL "/") OR (SEARCH_PATH STREQUAL "D:/")))

To Reproduce Steps to reproduce the behavior:

  1. Make a copy of the sample/blinky project into %HOME%\myBlinky
  2. set up tools as specified on Windows (
    • ZEPHYR_TOOLCHAIN_VARIANT=gnuarmemb
    • GNUARMEMB_TOOLCHAIN_PATH=C:\gnu_arm_embedded
    • CMake module path exported with west (in registry)
  3. in %HOME%\myBlinky run cmake --trace -B build -GNinja -DBOARD=adafruit_feather_m0_basic_proto .

Expected behavior Find package succeeds and CMake completes configure step successfully.

Impact I can't configure and work on a zephyr project using native cmake in windows without modifying the cmake find package files.

Screenshots or console output with cmake trace enabled, the following repeats forever:

foreach(preference  )
list(APPEND SEARCH_PATHS ${SEARCH_PATH}/zephyr )
list(APPEND SEARCH_PATHS ${SEARCH_PATH} )
get_filename_component(SEARCH_PATH ${SEARCH_PATH} DIRECTORY )
foreach(preference  )
list(APPEND SEARCH_PATHS ${SEARCH_PATH}/zephyr )
list(APPEND SEARCH_PATHS ${SEARCH_PATH} )
get_filename_component(SEARCH_PATH ${SEARCH_PATH} DIRECTORY )
foreach(preference  )
list(APPEND SEARCH_PATHS ${SEARCH_PATH}/zephyr )
list(APPEND SEARCH_PATHS ${SEARCH_PATH} )
get_filename_component(SEARCH_PATH ${SEARCH_PATH} DIRECTORY )
foreach(preference  )

Environment (please complete the following information):

Additional context I will search for a more universal solution later, unless someone else knows off the top of their head.

CrustyAuklet commented 4 years ago

Seems like a more portable fix is to compare against the path from the last iteration, this works on my machine:

# This macro returns a list of parent folders to use for later searches.
macro(get_search_paths START_PATH SEARCH_PATHS PREFERENCE_LIST)
  get_filename_component(SEARCH_PATH ${START_PATH} DIRECTORY)
  while(NOT ((SEARCH_PATH STREQUAL LAST_SEARCH_PATH)))        # CHANGED
    MESSAGE(search path: ${LAST_SEARCH_PATH} -> ${SEARCH_PATH})
    foreach(preference ${PREFERENCE_LIST})
      list(APPEND SEARCH_PATHS ${SEARCH_PATH}/${preference})
    endforeach()
    list(APPEND SEARCH_PATHS ${SEARCH_PATH}/zephyr)
    list(APPEND SEARCH_PATHS ${SEARCH_PATH})
    set(LAST_SEARCH_PATH ${SEARCH_PATH})        # NEW LINE
    get_filename_component(SEARCH_PATH ${SEARCH_PATH} DIRECTORY)
  endwhile()
endmacro()
carlescufi commented 4 years ago

@CrustyAuklet thanks for the report. Would you mind sending a Pull Request?

tejlmand commented 4 years ago

@CrustyAuklet thanks for this, and the fix. I verified the suggestion and it work as expected, so I created #24934.

If you have the signed-off-by line with you mail, I will be happy to include it in the commit message.

CrustyAuklet commented 4 years ago

@tejlmand Thanks, I was going to get around to PR but I am trying to focus on learning the OS and some examples first. I've also found some path issues with mixed / and \ in the final makefiles that I am fixing with a post configure sed command right now. I can open that as a separate issue later though.

For a signed-off-by line does this work? Ethan Slattery ethan@wildlifecomputers.com

tejlmand commented 4 years ago

I've also found some path issues with mixed / and \ in the final makefiles that I am fixing with a post configure sed command right now. I can open that as a separate issue later though.

please do. I'm sometimes building with make, to try catch issues where build system has flaws. But on windows, the only official shell supported is the cmd.exe (though other should work). And therefore, the Ninja generator is the safe bet on windows.

Are you using the MinGW Makefiles on windows ?

CrustyAuklet commented 4 years ago

But on windows, the only official shell supported is the cmd.exe (though other should work). And therefore, the Ninja generator is the safe bet on windows.

Are you using the MinGW Makefiles on windows ?

I am using ninja, in a mix of CMD and msys2. I also try to work through CLion, which is where I see the path problem. I use CMake a lot and never see this issue myself though, so I suspect the \ characters are hard coded somewhere. Generally in CMake you should always use / and CMake will do the "right thing".

Thanks for pointing out that cmd is the only officially supported shell on windows though. When I find the source I will make sure it doesn't affect the officially supported shells before I open any PRs :)

tejlmand commented 4 years ago

Generally in CMake you should always use / and CMake will do the "right thing".

Completely agree. But there are places where a \ may come into the system, as example from output from another script or external command. As example what I discovered here: https://github.com/zephyrproject-rtos/west/pull/190

Which is also why I would like get such issues removed.