search

espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.35k stars 7.21k forks source link

CMake FetchContent fails when called before idf_component_register (IDFGH-8469) #9929

Open higaski opened 1 year ago

higaski commented 1 year ago

Answers checklist.

IDF version.

ESP-IDF v5.1-dev-992-gaf28c1fa21

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

No response

What is the expected behavior?

For some reason using the CMake FetchContent module before registering a component fails spectacularly. 

include(FetchContent)  
FetchContent_Declare(...)

file(GLOB_RECURSE SOURCES *.c *.cpp)
idf_component_register(SRCS  ${SOURCES} ...)

What is the actual behavior?

CMake errors out. See "Build or installation Logs." for further details.

Steps to reproduce.

1.) Create new project from hello_world example 2.) Copy/paste any FetchContent_Declare call before idf_component_register, e.g:

      include(FetchContent)
      FetchContent_Declare(
        fmt
        GIT_REPOSITORY https://github.com/fmtlib/fmt.git
        GIT_TAG 9.1.0
        GIT_PROGRESS TRUE)
      FetchContent_MakeAvailable(fmt)

Build or installation Logs.

CMake Error at /home/vinci/esp/esp-idf/tools/cmake/component.cmake:224 (message):
  CMake Warning (dev) at build_properties.temp.cmake:8:

    Syntax Warning in cmake code at column 47

    Argument not separated from preceding token by whitespace.

  Call Stack (most recent call first):

    /home/vinci/esp/esp-idf/tools/cmake/scripts/component_get_requirements.cmake:3 (include)

  This warning is for project developers.  Use -Wno-dev to suppress it.

  fatal: not a git repository (or any parent up to mount point /)

  Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

  CMake Error at /usr/share/cmake/Modules/FetchContent.cmake:1149
  (define_property):

    define_property command is not scriptable

  Call Stack (most recent call first):

    /usr/share/cmake/Modules/FetchContent.cmake:1300:EVAL:1 (__FetchContent_declareDetails)
    /usr/share/cmake/Modules/FetchContent.cmake:1300 (cmake_language)
    /home/vinci/Develop/VSCode/hello_world/main/CMakeLists.txt:2 (FetchContent_Declare)
    /home/vinci/esp/esp-idf/tools/cmake/scripts/component_get_requirements.cmake:106 (include)
    /home/vinci/esp/esp-idf/tools/cmake/scripts/component_get_requirements.cmake:124 (__component_get_requirements)

More Information.

No response

igrr commented 1 year ago

Hi @higaski, The issue likely happens because component CMakeLists files are evaluated twice during the build process. First, they are evaluated early during the build in CMake script mode to extract component requirements (REQUIRES and PRIV_REQUIRES arguments of idf_component_register). Evaluation stops after encountering the call to idf_component_register() or a return(). Once the list of components has been determined component CMakeLists files are evaluated again, this time using the normal add_subdirectory call. I think you need to make sure the additional logic in your component CMakeLists file doesn't run during the first evaluation. You can do this either by moving FetchContent after idf_component_register or by wrapping the additional logic into if(NOT CMAKE_BUILD_EARLY_EXPANSION) ... endif().

By the way, there is an example of adding an external library which could be helpful: https://github.com/espressif/esp-idf/blob/master/examples/build_system/cmake/import_lib/components/tinyxml2/CMakeLists.txt

higaski commented 1 year ago

Thank you for the detailed explanation. Unfortunately, this only confirms my assumption that ESP-IDF is generally working against CMake and not with it. The reason people use CMake in the first place is because everybody else does so. "CMake has a beautiful syntax and is a pleasure to work with" is a sentence nobody ever has nor ever will say... By introducing the concept of components ESP-IDF basically takes the one and only thing away from CMake it excels at. Maybe I'm missing something, but currently I really can't see the point of it all?

/edit Is there any way to add e.g. mdns as component without using either the component manager nor manually downloading it?

igrr commented 1 year ago

@higaski I understand your sentiment, and I agree that it would be better if ESP-IDF started off with a more idiomatic CMake build system. I think this has been discussed extensively back in the day when IDF CMake-based build system was introduced in https://www.esp32.com/viewtopic.php?f=13&t=5559 and in follow-up topics on the forum.

The main reason for having 2-stage processing is to support Kconfig based project configuration. To allow component CMakeLists files to use Kconfig options, configuration processing step has to occur before the component libraries are added to the project using add_subdirectory(). In order to generate the configuration, we need to know the list of components which will participate in the build and contribute their Kconfig files. Since we don't want to place the burden on the user to list all the used components in the project CMakeLists.txt file, we need to figure out the list of components by expanding their requirements.

I'm sure you know this, but for others who might stumble on this ticket I'll note that it is still possible to write a library which works with normal CMake and IDF (and Zephyr) at the same time. LVGL is one such example: https://github.com/lvgl/lvgl/blob/master/CMakeLists.txt

igrr commented 1 year ago

Is there any way to add e.g. mdns as component without using either the component manager nor manually downloading it?

Two options that I can suggest, not sure if these qualify as "manually downloading it":

higaski commented 1 year ago

Adding the component with FetchContent would be my preferred way yes. When and where do I append the path to EXTRA_COMPONENT_DIRS though? If I wrap the FetchContent calls in an "CMAKE_BUILD_EARLY_EXPANSION"-if the folder doesn't seem to exist the first time when CMake is executed in script mode.

/edit Ok, I just realized that there are a whole bunch of options added to the sdkconfig when adding the mdns component with the component manager. Maybe I stick with that way after all? Or would those options also be added if I add the component through setting the EXTRA_COMPONENT_DIRS path?

igrr commented 1 year ago

If you don't have restrictions preventing you from using the IDF component manager for downloading ESP-specific components, then I'd recommend using that. It will be just simpler for us to support you in case of any problem since for us it's sort of a standard solution.

However, if you prefer to use FetchContent to download an IDF component, you can do something along these lines. This code would go into your project CMakeLists.txt file. (The thing I explained above regarding CMAKE_BUILD_EARLY_EXPANSION applies to component CMakeLists.txt files only, so in the project you don't have to deal with that.)

cmake_minimum_required(VERSION 3.16)

# use FetchContent to download esp-protocols repository
include(FetchContent)
FetchContent_Declare(
        esp-protocols
        GIT_REPOSITORY https://github.com/espressif/esp-protocols
        GIT_SHALLOW TRUE
)
FetchContent_MakeAvailable(esp-protocols)

# tell IDF build system to add the downloaded "mdns" component to the build
set(EXTRA_COMPONENT_DIRS ${CMAKE_CURRENT_BINARY_DIR}/_deps/esp-protocols-src/components/mdns)

include($ENV{IDF_PATH}/tools/cmake/project.cmake)
project(import_lib)

(I would be slightly worried about hardcoding _deps/<name>-src/ part of the path as it looks like some FetchContent implementation detail. But since CMake docs give exactly this example, I think it's unlikely to change: https://cmake.org/cmake/help/latest/module/FetchContent.html#populating-content-without-adding-it-to-the-build)

Ok, I just realized that there are a whole bunch of options added to the sdkconfig when adding the mdns component with the component manager.

There is no difference between the two approaches. The config options offered by mdns component will be available in both cases.

higaski commented 1 year ago

(The thing I explained above regarding CMAKE_BUILD_EARLY_EXPANSION applies to component CMakeLists.txt files only, so in the project you don't have to deal with that.)

Ok, this finally makes this issue an actual bug. :smile: For some reason I don't understand yet this only seems to work with the example you've just posted. Maybe because it contains an actual "component"? Or at least a component folder?

Trying to add any other repository (let's stick with the fmt example)

cmake_minimum_required(VERSION 3.16)

# use FetchContent to download esp-protocols repository
include(FetchContent)
FetchContent_Declare(
  fmt
  GIT_REPOSITORY https://github.com/fmtlib/fmt.git
  GIT_TAG 9.1.0
  GIT_PROGRESS TRUE)
FetchContent_MakeAvailable(fmt)

include($ENV{IDF_PATH}/tools/cmake/project.cmake)
project(hello_world)

results in another CMake error

CMake Error at /home/vinci/esp/esp-idf/components/xtensa/project_include.cmake:3 (message):
  Internal error, toolchain has not been set correctly by project (or an
  invalid CMakeCache.txt file has been generated somehow)
Call Stack (most recent call first):
  /home/vinci/esp/esp-idf/tools/cmake/build.cmake:398 (include)
  /home/vinci/esp/esp-idf/tools/cmake/build.cmake:617 (__build_process_project_includes)
  /home/vinci/esp/esp-idf/tools/cmake/project.cmake:440 (idf_build_process)
  CMakeLists.txt:15 (project)

Hence my question before where I should had put the set(EXTRA_COMPONENT_DIRS...). I simply was expecting for calls to FetchContent to fail in this place as well.

igrr commented 1 year ago

The error you are running into occurs because FetchContent downloads the code and tries to configure and build it as a CMake project (similar to ExternalProject_add, but using add_subdirectory instead).

(Search CMake FetchContent docs for "If the top directory of the populated content contains a CMakeLists.txt file, call add_subdirectory() to add it to the main build..." for more information)

However at this point, the toolchain file hasn't been set up yet by project.cmake, so while processing fmt, CMake picks the wrong toolchain (the host one) which later gets reported by IDF as an error.

If you need to add an external library like fmt (which isn't an IDF component) conceptually there are two ways to do this:

I also recommend checking the Build System chapter in the IDF Programming Guide — looks like some of the sections there might be relevant for you: "Writing Pure CMake Components", "Using Third-Party CMake Projects with Components", "Using ESP-IDF in Custom CMake Projects".

higaski commented 1 year ago

Once again, thank you for the crystal clear explanation. I was just about to write something along the lines of "I'm going to stick with the component manager"... and then some things about how everything else feels like a workaround and calls to FetchContent/MakeAvailable are outright dangerous because you never know what the CMakeLists.txt file of the project your fetching actually does... if it even has one at all.

The "idf_as_lib" example also doesn't particularly appeal to me to be honest although I see the use cases of course. Funnily enough I was even one of the first contributers to LVGLs CMake compatibility, but I simply passed whatever there already was on to some "ESP-IDF" CMake function or file or so. :stuck_out_tongue_closed_eyes:

The solution I now found for me is to simply invoke CMake directly. That way I can control whatever toolchain file gets picked and I don't need to worry about CMake running in script mode or anything. Also I don't loose any of the IDF tool benefits like 'idf.py monitor' or such.

Oh and btw, the path in the esp-protocols example you've posted earlier (this one) ${CMAKE_CURRENT_BINARY_DIR}/_deps/esp-protocols-src/components/mdns could be written as ${esp-protocols_SOURCE_DIR}/components/mdns

Thanks again for all your help! Really appreciate it.

igrr commented 1 year ago

Just as a follow-up, https://github.com/espressif/idf-extra-components/pull/98 will add a fmtlib component to https://components.espressif.com/.

higaski commented 1 year ago

I'm sure you know this, but for others who might stumble on this ticket I'll note that it is still possible to write a library which works with normal CMake and IDF (and Zephyr) at the same time. LVGL is one such example: https://github.com/lvgl/lvgl/blob/master/CMakeLists.txt

I'm currently failing miserably to write even the simplest component so I though I should take a look at LVGL. Turns out LVGL isn't working either.

I even tried adding it to the component manager

dependencies:
  idf: ">=5.0"
  espressif/esp_tinyusb: ">=1.0.2"
  espressif/mdns: ">=1.0.7"
  lvgl:
    path: lvgl
    git: https://github.com/lvgl/lvgl.git

Which results in a huge Python traceback...

Traceback (most recent call last):

    File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
      return _run_code(code, main_globals, None,
    File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
      exec(code, run_globals)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/prepare_components/__main__.py", line 6, in <module>
      main()
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/prepare_components/prepare.py", line 124, in main
      args.func(args)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/prepare_components/prepare.py", line 27, in prepare_dep_dirs
      ComponentManager(
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/core.py", line 62, in wrapper
      return func(self, *args, **kwargs)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/core.py", line 438, in prepare_dep_dirs
      downloaded_component_paths, downloaded_component_version_dict = download_project_dependencies(
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/dependencies.py", line 111, in download_project_dependencies
      solution = solver.solve()
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/version_solver/version_solver.py", line 34, in solve
      self.solve_manifest(manifest)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/version_solver/version_solver.py", line 53, in solve_manifest
      self.solve_component(requirement)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_manager/version_solver/version_solver.py", line 56, in solve_component
      cmp_with_versions = requirement.source.versions(
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/sources/git.py", line 136, in versions
      commit_id = self._checkout_git_source(version, temp_dir, selected_paths=[self.component_path])
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/sources/git.py", line 52, in _checkout_git_source
      return self._client.prepare_ref(
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/git_client.py", line 41, in wrapper
      return func(self, *args, **kwargs)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/git_client.py", line 72, in wrapper
      return func(self, *args, **kwargs)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/git_client.py", line 133, in prepare_ref
      self.run(checkout_command)
    File "/home/vinci/.espressif/python_env/idf5.1_py3.10_env/lib/python3.10/site-packages/idf_component_tools/git_client.py", line 183, in run
      raise GitCommandError(

  idf_component_tools.git_client.GitCommandError: 'git --work-tree
  /tmp/tmp_xia4uwq --git-dir
  /home/vinci/.cache/Espressif/ComponentManager/b_git_023cb0a1 checkout
  --force 162e451396cfe974786a58d3738af745a29f5979 -- lvgl' failed with exit
  code 1

  error: pathspec 'lvgl' did not match any file(s) known to git

I'm sorry but this is really starting to piss me off. How the f* are people using ESPs in the real world? My best guess is that everyone is copy pasting source files like back in 1980...

igrr commented 1 year ago

The reason for this failure is that there is no subdirectory lvgl (which you have specified in path) in https://github.com/lvgl/lvgl.git

You can add lvgl like this: https://github.com/espressif/esp-bsp/blob/f9a521aaf96722e4ea0bf43794eaebf80388f5e4/esp32_s3_usb_otg/idf_component.yml#L11

higaski commented 1 year ago

I see, that's what path does. Thank you for clearing that up. I apologize for my last post, that was unnecessary.

The component manager still needs a lot of documentation work though, but it seems it's well on its way (https://github.com/espressif/idf-component-manager/pull/17)

igrr commented 1 year ago

No problem, I do agree with you it looks very frustrating when it fails like that!

Component manager documentation improvements are underway and we'll add a better message for this type of error.

If you are looking into LVGL, I may suggest also esp_lvgl_port component which includes an implementation of LVGL port layer on top of ESP-LCD. In example of its usage can be seen in this esp-box component: https://github.com/espressif/esp-bsp/blob/master/esp-box/esp-box.c.

igrr commented 1 year ago

By there way, there is now a sample of using CMake FetchContent inside an IDF component: https://github.com/erhankur/esp32-spdlog/blob/08e076789e46572cc4f8219cbea6d07ada914bd4/components/spdlog/CMakeLists.txt#L15

phelter commented 1 year ago

I'd like to add my sentiment about the arcane way IDF has been integrated in with cmake.

It would have been easy enough to add a set of Kconfig functions to tie a cmake add_custom_target() or an add_custom_command() to a native cmake target rather than an idf_component. All of the custom targets would then depend on a single sdkconfig file which when changed will regenerate everything necessary.

The other option would be to add them in as CMake Native Cached variables and options for each component. The CMakeLists.txt in each of the directories would then be able to contain ALL of the configuration (Kconfig) and all of the definition of the module, and tie the defines to the native cmake targets An example for esp_idf/components/log would be:

# Configuration

# Add Config documentation here.
# Note with this you don't need the independent LOG_DEFAULT_LEVEL_<NONE|ERROR|WARN|INFO...> or LOG_MAXIMUM_LEVEL_<*>

set(LOG_DEFAULT_VALUE 3 CACHE STRING "Default log verbosity 0=none 1=error 2=warn 3=info 4=debug 5=verbose")
set(LOG_MAXIMUM_VALUE 3 CACHE STRING "Maximum log verbosity 0=none 1=error 2=warn 3=info 4=debug 5=verbose")
set(LOG_COLORS ON CACHE BOOL "Use ANSI terminal colors in log output")
set(LOG_TIMESTAMP_SOURCE 0 CACHE STRING "Log Timestamp 0=rtos 1=system")

add_library(log STATIC)
add_library(idf::log ALIAS log)

target_sources(log
    PRIVATE
       include/esp_log_internal.h
       include/esp_log.h
       esp_log_private.h
       log_buffers.c
       $<IF:$<STREQUAL:${IDF_TARGET},linux>, log_linux.c, log_freertos.c>
       # $ENV{IDF_PATH}/components/log/log_noos.c - for bootloader but leaving out for now.
       log/log.c
)

target_include_directories(log SYSTEM # System required for bugs in headers see below.
    PUBLIC
         include
)

target_compile_definitions(log
  # Note all public since used in headers only, but if only used in underlying c files they would be private.
   PUBLIC
        CONFIG_LOG_DEFAULT_LEVEL=${LOG_DEFAULT_LEVEL}
        CONFIG_LOG_MAXIMUM_LEVEL=${LOG_MAXIMUM_LEVEL}
        CONFIG_LOG_COLORS=${LOG_COLORS}
        CONFIG_LOG_TIMESTAMP_SOURCE=${LOG_TIMESTAMP_SOURCE }
)

target_compile_options(log
    PRIVATE
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-qual>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-declaration-after-statement>
        $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-implicit-function-declaration>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-implicit-int-conversion>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-prototypes>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-pointer-arith>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-shorten-64-to-32>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
        $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-unused-variable>
        $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-zero-length-array>
)

target_link_libraries(log
    PUBLIC
        idf::sdkconfig
        idf::esp_rom
    PRIVATE
        idf::esp_common
)

Also note I've been building pieces of the esp_idf with native cmake - typically just the interface portion of each component except for rare cases, and have come across LOTS of clang and GNU warnings as shown above. It would be great if the code was cleaned up with the strictest of warnings defined or at least acknowledge which ones the esp_idf team has decided to ignore in a manner similar to above.

Also a few other notes:

higaski commented 1 year ago

I've found new pain points which bug me...

  1. ESP-IDF and the whole component thing ignores CMake's target_compile_features.
  2. The component manager ignores global compile options.

This leads to situation where I have to specify the C++ standard I'd like to use twice. Once globally with add_compile_options to satisfy "non-component" libs and once with target_compile_options to satisfy ESP-IDF components...