OpenAMP / open-amp

The main OpenAMP library implementing RPMSG, Virtio, and Remoteproc for RTOS etc
https://www.openampproject.org/
Other
706 stars 288 forks source link

version: use generated version_def.h #469

Closed kernelchuk closed 1 year ago

kernelchuk commented 1 year ago

Using command line to define the OPENAMP_VERSION string macro breaks Cygwin build on some releases of Microsoft Windows and CMake. Remove all OPENAMP_VERSION macros from cmake compiler flags and use the ones from the generated version_def.h file.

arnopo commented 1 year ago

Please Could you please provide more details on the issue ? I would like to understand the problem you are facing.

kernelchuk commented 1 year ago

The error

src/open-amp/lib/version.c: In function 'openamp_version':
        <command-line>: error: too many decimal points in number
C:src/open-amp/lib/version.c:24:16: note: in expansion of macro 'OPENAMP_VERSION'
24 |         return OPENAMP_VERSION;
   |                ^~~~~~~~~~~~~~~

happens during open-amp build in Vitis on MS-Windows under Cygwin. The top-level make is started via

make -C psu_cortexr5_0/libsrc/openamp_v1_8/src -s libs "SHELL=CMD"
  "COMPILER=armr5-none-eabi-gcc" "ASSEMBLER=armr5-none-eabi-as"  "ARCHIVER=armr5-none-eabi-ar"
  "COMPILER_FLAGS= -O2 -c -mcpu=cortex-r5" ...

where each compiler option is enclosed in double quotes. This collides with the quotes from https://github.com/OpenAMP/open-amp/blob/246540f3d6fd9bd86d6ae82c68d285570076670b/cmake/options.cmake#L19 One option is to try to get the quotes to work for both Linux and Windows build. The other is to follow the libmetal example of lib/config.h. The open-amp repo does not have config.h, but it generates version_def.h from VERSION. This way we avoid the build system shell dependencies and rely only on the CMake and the preprocessor.

arnopo commented 1 year ago

The error

src/open-amp/lib/version.c: In function 'openamp_version':
        <command-line>: error: too many decimal points in number
C:src/open-amp/lib/version.c:24:16: note: in expansion of macro 'OPENAMP_VERSION'
24 |         return OPENAMP_VERSION;
   |                ^~~~~~~~~~~~~~~

happens during open-amp build in Vitis on MS-Windows under Cygwin. The top-level make is started via

make -C psu_cortexr5_0/libsrc/openamp_v1_8/src -s libs "SHELL=CMD"
  "COMPILER=armr5-none-eabi-gcc" "ASSEMBLER=armr5-none-eabi-as"  "ARCHIVER=armr5-none-eabi-ar"
  "COMPILER_FLAGS= -O2 -c -mcpu=cortex-r5" ...

where each compiler option is enclosed in double quotes. This collides with the quotes from

https://github.com/OpenAMP/open-amp/blob/246540f3d6fd9bd86d6ae82c68d285570076670b/cmake/options.cmake#L19

One option is to try to get the quotes to work for both Linux and Windows build. The other is to follow the libmetal example of lib/config.h. The open-amp repo does not have config.h, but it generates version_def.h from VERSION. This way we avoid the build system shell dependencies and rely only on the CMake and the preprocessor.

Thanks for the explanation, it's much clearer now. Regarding some posts, seems that another way to fix this is to use add_definitions( -DOPENAMP_VERSION="\\"${PROJECT_VERSION}\\"" )

That said, I can not find any reason to keep definitions in cmake, so your proposal seems to me ok

arnopo commented 1 year ago

@edmooring can you give a feedback on this one as it is a candidate for the release? Thanks!