cmake: OpenAMP build doesn't work for multiple build directories #10241

raiden00pl · 2023-08-16T09:37:32Z

cmake -B build_h7m4 -DBOARD_CONFIG=nucleo-h745zi:nsh_cm4_rptun -GNinja
cmake -B build_h7m7 -DBOARD_CONFIG=nucleo-h745zi:nsh_cm7_rptun -GNinja
cmake --build build_h7m4
[...] build OK
cmake --build build_h7m7

/home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/remoteproc/remoteproc.c:912:12: error: redefinition of 'remoteproc_virtio_notify_wait'
  912 | static int remoteproc_virtio_notify_wait(void *priv, uint32_t id)
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/remoteproc/remoteproc.c:902:12: note: previous definition of 'remoteproc_virtio_notify_wait' with type 'int(void *, uint32_t)' {aka 'int(void *, long unsigned int)'}
  902 | static int remoteproc_virtio_notify_wait(void *priv, uint32_t id)
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/remoteproc/remoteproc.c:902:12: warning: 'remoteproc_virtio_notify_wait' defined but not used [-Wunused-function]

In file included from /home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/rpmsg/rpmsg_virtio.c:14:
/home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/include/openamp/rpmsg_virtio.h:167:1: error: redefinition of 'rpmsg_virtio_notify_wait'
  167 | rpmsg_virtio_notify_wait(struct rpmsg_virtio_device *rvdev,
      | ^~~~~~~~~~~~~~~~~~~~~~~~
/home/raiden00/git/RTOS/nuttx/nuttx/openamp/open-amp/lib/include/openamp/rpmsg_virtio.h:158:1: note: previous definition of 'rpmsg_virtio_notify_wait' with type 'int(struct rpmsg_virtio_device *, struct virtqueue *)'
  158 | rpmsg_virtio_notify_wait(struct rpmsg_virtio_device *rvdev,

if we configure another build directory we result in not patched openamp:

cmake -B build_h7m4_2 -DBOARD_CONFIG=nucleo-h745zi:nsh_cm4_rptun -GNinja
cmake --build build_h7m4_2
[...] failure due to not patched OpenAMP

in that case it looks like openamp/open-amp is replaced with not patched version. openamp/.openamp_patch flag is present so fresh openamp source is not modified.

The text was updated successfully, but these errors were encountered:

raiden00pl · 2023-08-22T13:15:22Z

@anchao @xuxin930 Is there any reason why we don't use PATCH_COMMAND in FetchContent_Declare for patching?
It seems that this problem doesn't occur with command like this:

  FetchContent_Declare(
    open-amp
    DOWNLOAD_NAME "libopen-amp-v${OPENAMP_VERSION}.zip"
    DOWNLOAD_DIR ${CMAKE_CURRENT_LIST_DIR}
    URL "https://github.com/OpenAMP/open-amp/archive/v${OPENAMP_VERSION}.zip"
        SOURCE_DIR
        ${CMAKE_CURRENT_LIST_DIR}/open-amp
        BINARY_DIR
        ${CMAKE_BINARY_DIR}/openamp/open-amp
        PATCH_COMMAND
        patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0001-ns-acknowledge-the-received-creation-message.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0002-Negotiate-individual-buffer-size-dynamically.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0003-rpmsg-wait-endpoint-ready-in-rpmsg_send-and-rpmsg_se.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0004-openamp-add-new-ops-notify_wait-support.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0005-rpmsg_virtio-don-t-need-check-status-when-get_tx_pay.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0006-rpmsg-notify-the-user-when-the-remote-address-is-rec.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0007-openamp-avoid-double-calling-ns_bound-when-each-othe.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0008-remoteproc-make-all-elf_-functions-static-except-elf.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0009-Fix-warn-declaration-of-vring_rsc-shadows-a-previous.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0010-rptun-fix-rptun-don-t-wait-issue-when-get-tx-patyloa.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0011-rpmsg-fix-rpmsg_virtio_get_tx_buffer-no-idx-return.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0012-rpmsg-add-new-API-rpdev_release_tx-rx_buffer.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0013-openamp-add-error-log-when-ept-cb-return-error.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0014-rpmsg-add-cache-flash-when-hold-rx-buffer.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0015-rpmsg-do-cache_invalidate-when-real-data-returned.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0016-openamp-add-new-API-rpmsg_virtio_get_rxbuffer_size.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0017-virtio-follow-virtio-1.2-spec-add-more-virtio-status.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0018-virtio-decoupling-the-transport-layer-and-virtio-dev.patch
        && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
        ${CMAKE_CURRENT_LIST_DIR}/0019-virtio.h-add-version-in-device-id-table.patch
    DOWNLOAD_NO_PROGRESS true
    TIMEOUT 30)

xuxin930 · 2023-08-23T03:17:30Z

hi @raiden00pl , I think now this problem is caused by two reasons:

duplicate download
the code is to check whether the .git in the open-amp directory exists, apparently this caused it to be downloaded and decompressed twice in two build process.
wrong patch target location
I think the original intention of patch target is to mark that openAmp has been patched. This should only be executed once, but it is marked on the outer directory. this caused the patch to not be applied again during the second download.

I think we should deal with these two issues separately. for the second issue, I think using PATCH_COMMAND is a better way. what do you think? @raiden00pl @anchao

anchao · 2023-08-23T04:47:01Z

@xuxin930

I think the root cause is download twice, we can just delete the condition of .git suffix

wrong patch target location

patch depends on the target "openamp_patch", the target should only executed once
PATCH_COMMAND is only available for ExternalProject right? FetchContent_Declare may be a better fit due to header file dependencies

raiden00pl · 2023-08-23T07:37:42Z

@anchao According to https://cmake.org/cmake/help/latest/module/FetchContent.html#commands

The can be any of the download, update or patch options that the ExternalProject_Add() command understands.

So PATCH_COMMAND is also available for ExternalProject

anchao · 2023-08-23T08:01:35Z

So PATCH_COMMAND is also available for ExternalProject

@raiden00pl

CMake FetchContent vs. ExternalProject

ExternalProject populates content from the other project at build time. This means the other project’s libraries are not visible until the parent project is built. Since ExternalProject does not combine the project namespaces, ExternalProject may be necessary if you don’t control the other projects.

Since too many modules depend on the header files of openamp, if the download of the content is delayed until the build time, it will cause many files to be incorrectly included, In nuttx, it seems like FetchContent_Declare will be better than ExternalProject

raiden00pl · 2023-08-23T08:14:34Z

@anchao

So PATCH_COMMAND is also available for ExternalProject

typo. Should be: So PATCH_COMMAND is also available for FetchContent.

We can use FetchContent with PATCH_COMMAND

anchao · 2023-08-23T09:16:22Z

All targets that use FetchContent_Declare need to make similar changes:

Delete the dir suffix of .git
Replace all patch targets with PATCH_COMMAND

@xuxin930 , could you please help enhance this feature if feel free ?

xuxin930 · 2023-08-23T09:35:36Z

of course😁 @anchao @raiden00pl
besides these two changes, also need to delete PATCH_TARGET & PATCH_TARGET_DEP.
I will do it now, I'll complete both in nuttx and nuttx-apps, and link to this issue to track progress.

raiden00pl · 2023-08-24T15:50:07Z

Solved by #10379 and apache/nuttx-apps#1993
Thanks @xuxin930

This was referenced Aug 23, 2023

cmake:replace custom_patch_target with PATCH_COMMAND #10379

Merged

cmake:replace custom_patch_target with PATCH_COMMAND apache/nuttx-apps#1993

Merged

raiden00pl mentioned this issue Aug 24, 2023

arch/stm32h7: add CM4 core support #10328

Merged

raiden00pl closed this as completed Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: OpenAMP build doesn't work for multiple build directories #10241

cmake: OpenAMP build doesn't work for multiple build directories #10241

raiden00pl commented Aug 16, 2023

raiden00pl commented Aug 22, 2023

xuxin930 commented Aug 23, 2023 •

edited

Loading

anchao commented Aug 23, 2023

raiden00pl commented Aug 23, 2023

anchao commented Aug 23, 2023

raiden00pl commented Aug 23, 2023 •

edited

Loading

anchao commented Aug 23, 2023

xuxin930 commented Aug 23, 2023

raiden00pl commented Aug 24, 2023

cmake: OpenAMP build doesn't work for multiple build directories #10241

cmake: OpenAMP build doesn't work for multiple build directories #10241

Comments

raiden00pl commented Aug 16, 2023

raiden00pl commented Aug 22, 2023

xuxin930 commented Aug 23, 2023 • edited Loading

anchao commented Aug 23, 2023

raiden00pl commented Aug 23, 2023

anchao commented Aug 23, 2023

raiden00pl commented Aug 23, 2023 • edited Loading

anchao commented Aug 23, 2023

xuxin930 commented Aug 23, 2023

raiden00pl commented Aug 24, 2023

xuxin930 commented Aug 23, 2023 •

edited

Loading

raiden00pl commented Aug 23, 2023 •

edited

Loading