Make Generated Output in CMake More Robust

The `_opendds_*_generated_output functions` in CMake didn't have any sort of testing and could give incorrect results for where to put the export header file. This tries to correct both of those issues. Also added workaround for DOCGroup/ACE_TAO#2202
iguessthislldo · Feb 26, 2024 · eb3a167 · eb3a167
1 parent 1c22955
commit eb3a167
Show file tree

Hide file tree

Showing 4 changed files with 237 additions and 76 deletions.
diff --git a/cmake/dds_idl_sources.cmake b/cmake/dds_idl_sources.cmake
@@ -207,17 +207,25 @@ function(_opendds_target_idl_sources target)
     set(rel_path_to_source_tree "${rel_path_to_source_tree}/")
   endif()
 
+  set(set_o_opt FALSE)
+  set(o_opt)
   foreach(flag ${arg_DDS_IDL_FLAGS})
     if("${flag}" MATCHES "^-I(\\.\\..*)")
       list(APPEND opendds_idl_opts "-I${rel_path_to_source_tree}${CMAKE_MATCH_1}")
+    elseif("${flag}" STREQUAL "-o")
+      # Omit orignal -o* options because of https://github.com/DOCGroup/ACE_TAO/issues/2202
+      set(set_o_opt TRUE)
+    elseif(set_o_opt)
+      set(o_opt "${flag}")
+      set(set_o_opt FALSE)
     else()
-      list(APPEND opendds_idl_opts ${flag})
+      list(APPEND opendds_idl_opts "${flag}")
     endif()
   endforeach()
 
   set(opendds_idl_opt_var_prefix "opendds_idl_opt")
   set(opendds_idl_no_value_opts "-SI" "-GfaceTS" "-Wb,java" "-Lc++11" "-Lface")
-  set(opendds_idl_one_value_opts "-o")
+  set(opendds_idl_one_value_opts "")
   set(opendds_idl_multi_value_opts)
   foreach(opt ${opendds_idl_all_opts})
     unset("${opendds_idl_opt_var_prefix}_${opt}")
@@ -234,14 +242,14 @@ function(_opendds_target_idl_sources target)
   if(debug)
     message(STATUS "gen out: ${gen_out}")
     message(STATUS "IDL files:")
+    list(APPEND CMAKE_MESSAGE_INDENT "  ")
   endif()
-  list(APPEND CMAKE_MESSAGE_INDENT "  ")
   foreach(input ${non_generated_idl_files})
     if(debug)
       string(REPLACE "${CMAKE_CURRENT_SOURCE_DIR}/" "" show_idl "${input}")
       message(STATUS "${show_idl}")
+      list(APPEND CMAKE_MESSAGE_INDENT "  ")
     endif()
-    list(APPEND CMAKE_MESSAGE_INDENT "  ")
     get_filename_component(abs_filename ${input} ABSOLUTE)
     get_filename_component(file_ext ${input} EXT)
     get_filename_component(idl_file_dir ${abs_filename} DIRECTORY)
@@ -262,9 +270,10 @@ function(_opendds_target_idl_sources target)
         message(STATUS "SKIP_OPENDDS_IDL")
       endif()
     else()
-      _opendds_get_generated_idl_output(
-        ${target} "${arg_INCLUDE_BASE}" "${input}" "${opendds_idl_opt_-o}" output_prefix output_dir)
-      _opendds_get_generated_output_dir(${target} file_auto_includes O_OPT "${opendds_idl_opt_-o}")
+      _opendds_get_generated_output(${target} "${input}"
+        INCLUDE_BASE "${arg_INCLUDE_BASE}" O_OPT "${o_opt}" MKDIR
+        PREFIX_PATH_VAR output_prefix DIR_PATH_VAR output_dir)
+      _opendds_get_generated_output_dir(${target} file_auto_includes O_OPT "${o_opt}")
       if(arg_INCLUDE_BASE)
         list(APPEND file_auto_includes "${arg_INCLUDE_BASE}")
       endif()
@@ -308,10 +317,16 @@ function(_opendds_target_idl_sources target)
 
       _opendds_tao_append_runtime_lib_dir_to_path(extra_lib_dirs)
 
+      set(opendds_idl_args
+        "-I${idl_file_dir}"
+        ${opendds_idl_opts} ${extra_options}
+        ${abs_filename} -o "${output_dir}"
+      )
       if(debug)
+        message(STATUS "opendds_idl ${opendds_idl_args}")
         foreach(generated_file ${generated_files})
           string(REPLACE "${output_dir}/" "" generated_file "${generated_file}")
-          message(STATUS "opendds_idl: ${generated_file}")
+          message(STATUS "${generated_file}")
         endforeach()
       endif()
       add_custom_command(
@@ -320,9 +335,7 @@ function(_opendds_target_idl_sources target)
         MAIN_DEPENDENCY ${abs_filename}
         COMMAND
           ${CMAKE_COMMAND} -E env "DDS_ROOT=${DDS_ROOT}" "TAO_ROOT=${TAO_INCLUDE_DIR}"
-          "${extra_lib_dirs}"
-          $<TARGET_FILE:OpenDDS::opendds_idl> "-I${idl_file_dir}"
-          ${opendds_idl_opts} ${extra_options} ${abs_filename} -o "${output_dir}"
+          "${extra_lib_dirs}" $<TARGET_FILE:OpenDDS::opendds_idl> ${opendds_idl_args}
       )
 
       list(APPEND tao_idl_opts
@@ -374,9 +387,13 @@ function(_opendds_target_idl_sources target)
     list(APPEND all_mappings ${file_mappings})
     list(REMOVE_DUPLICATES all_mappings)
 
-    _opendds_pop_list(CMAKE_MESSAGE_INDENT)
+    if(debug)
+      _opendds_pop_list(CMAKE_MESSAGE_INDENT)
+    endif()
   endforeach()
-  _opendds_pop_list(CMAKE_MESSAGE_INDENT)
+  if(debug)
+    _opendds_pop_list(CMAKE_MESSAGE_INDENT)
+  endif()
 
   set_property(TARGET ${target}
     PROPERTY "OPENDDS_LANGUAGE_MAPPINGS" ${all_mappings})

diff --git a/cmake/opendds_target_sources.cmake b/cmake/opendds_target_sources.cmake
@@ -208,7 +208,9 @@ function(opendds_export_header target)
     return()
   endif()
 
-  _opendds_get_generated_file_path(${target} "${arg_INCLUDE_BASE}" "${target}_export.h" export_header)
+  _opendds_get_generated_output(${target} "${target}_export.h"
+    GENERATED INCLUDE_BASE "${arg_INCLUDE_BASE}"
+    MKDIR FILE_PATH_VAR export_header)
 
   string(TOUPPER "${target}" uppercase_target)
   if(NOT EXISTS ${export_header})
@@ -235,8 +237,8 @@ function(opendds_target_sources target)
   if(opendds_target_sources IN_LIST OPENDDS_CMAKE_VERBOSE)
     message(STATUS "opendds_target_sources(${target} ${ARGN}) called from ${PROJECT_NAME}")
     set(debug TRUE)
+    list(APPEND CMAKE_MESSAGE_INDENT "  ")
   endif()
-  list(APPEND CMAKE_MESSAGE_INDENT "  ")
 
   if(NOT TARGET ${target})
     message(FATAL_ERROR "Invalid target '${target}' passed into opendds_target_sources")

diff --git a/cmake/tao_idl_sources.cmake b/cmake/tao_idl_sources.cmake
@@ -19,66 +19,108 @@ function(_opendds_tao_append_runtime_lib_dir_to_path dst)
 endfunction()
 
 function(_opendds_get_generated_output_dir target output_dir_var)
-  set(no_value_options)
+  set(no_value_options MKDIR)
   set(single_value_options O_OPT)
   set(multi_value_options)
   cmake_parse_arguments(arg
     "${no_value_options}" "${single_value_options}" "${multi_value_options}" ${ARGN})
 
   # TODO base output_dir_var on target
   set(output_dir "${CMAKE_CURRENT_BINARY_DIR}/opendds_generated")
+  get_filename_component(output_dir "${output_dir}" REALPATH)
   if(arg_O_OPT)
     if(IS_ABSOLUTE "${arg_O_OPT}")
       set(output_dir "${arg_O_OPT}")
     else()
       set(output_dir "${output_dir}/${arg_O_OPT}")
     endif()
   endif()
+
+  if(arg_MKDIR)
+    file(MAKE_DIRECTORY "${output_dir}")
+  endif()
+
   set(${output_dir_var} "${output_dir}" PARENT_SCOPE)
 endfunction()
 
-function(_opendds_ensure_generated_output_dir target include_base file o_opt output_dir_var)
-  get_filename_component(abs_file "${file}" ABSOLUTE)
-  get_filename_component(abs_dir "${abs_file}" DIRECTORY)
-  _opendds_get_generated_output_dir("${target}" output_dir O_OPT "${o_opt}")
-  if(include_base AND NOT OPENDDS_FILENAME_ONLY_INCLUDES AND NOT O_OPT)
-    get_filename_component(output_dir "${output_dir}" REALPATH)
-    get_filename_component(real_abs_file "${abs_file}" REALPATH)
-    get_filename_component(real_include_base "${include_base}" REALPATH)
-    file(RELATIVE_PATH rel_to_output "${output_dir}" "${real_abs_file}")
+function(_opendds_get_generated_output target file)
+  set(no_value_options MKDIR GENERATED)
+  set(single_value_options INCLUDE_BASE O_OPT DIR_PATH_VAR FILE_PATH_VAR PREFIX_PATH_VAR FAIL_VAR)
+  set(multi_value_options)
+  cmake_parse_arguments(arg
+    "${no_value_options}" "${single_value_options}" "${multi_value_options}" ${ARGN})
+
+  if(arg_FAIL_VAR)
+    set(${arg_FAIL_VAR} FALSE PARENT_SCOPE)
+  endif()
+
+  set(dir_args)
+  if(arg_MKDIR)
+    list(APPEND dir_args MKDIR)
+  endif()
+  if(arg_O_OPT)
+    list(APPEND dir_args O_OPT "${arg_O_OPT}")
+  endif()
+  _opendds_get_generated_output_dir("${target}" output_dir ${dir_args})
+
+  if(arg_INCLUDE_BASE AND NOT OPENDDS_FILENAME_ONLY_INCLUDES AND NOT arg_O_OPT)
+    get_filename_component(real_include_base "${arg_INCLUDE_BASE}" REALPATH)
+    if(IS_ABSOLUTE "${file}")
+      get_filename_component(real_file "${file}" REALPATH)
+    else()
+      get_filename_component(real_file "${CMAKE_CURRENT_SOURCE_DIR}/${file}" REALPATH)
+    endif()
+    file(RELATIVE_PATH rel_to_output "${output_dir}" "${real_file}")
     if(rel_to_output MATCHES "^\\.\\.")
       # This should be an IDL file that is relative to include_base.
-      file(RELATIVE_PATH rel_file "${real_include_base}" "${real_abs_file}")
+      set(rel_to "${real_include_base}")
     else()
       # This should be our own generated IDL file that is relative to
-      # opendds_generated.
-      file(RELATIVE_PATH rel_file "${output_dir}" "${real_abs_file}")
+      # opendds_generated. Doing this relative to the include base in the
+      # output might or might not make sense in practice, but makes output
+      # consistant with source directory case.
+      set(rel_to "${output_dir}/${arg_INCLUDE_BASE}")
     endif()
+    file(RELATIVE_PATH rel_file "${rel_to}" "${real_file}")
     get_filename_component(rel_dir "${rel_file}" DIRECTORY)
     if(rel_file MATCHES "^\\.\\.")
-      message(FATAL_ERROR "This IDL file:\n\n  ${rel_file}\n\nis outside the INCLUDE_BASE:\n\n  ${include_base}")
-    endif()
-    if(rel_dir)
-      set(output_dir "${output_dir}/${rel_dir}")
+      if(arg_FAIL_VAR)
+        set(${arg_FAIL_VAR} TRUE PARENT_SCOPE)
+        return()
+      endif()
+      message(FATAL_ERROR
+        "  This file:\n"
+        "  \n"
+        "    ${rel_file}\n"
+        "    (${file})\n"
+        "  \n"
+        "  is outside the INCLUDE_BASE:\n"
+        "  \n"
+        "    ${arg_INCLUDE_BASE}\n"
+        "    (${real_include_base})\n")
     endif()
+    set(output_dir "${output_dir}/${rel_dir}")
   endif()
-  file(MAKE_DIRECTORY "${output_dir}")
-  set(${output_dir_var} "${output_dir}" PARENT_SCOPE)
-endfunction()
 
-function(_opendds_get_generated_file_path target include_base file output_path_var)
-  _opendds_ensure_generated_output_dir(${target} "${include_base}" "${file}" "" output_dir)
-  get_filename_component(filename ${file} NAME)
-  set(${output_path_var} "${output_dir}/${filename}" PARENT_SCOPE)
-endfunction()
+  string(REGEX REPLACE "/$" "" output_dir "${output_dir}")
 
-function(_opendds_get_generated_idl_output
-    target include_base idl_file o_opt output_prefix_var output_dir_var)
-  _opendds_ensure_generated_output_dir(
-    ${target} "${include_base}" "${idl_file}" "${o_opt}" output_dir)
-  get_filename_component(idl_filename_no_ext ${idl_file} NAME_WE)
-  set(${output_prefix_var} "${output_dir}/${idl_filename_no_ext}" PARENT_SCOPE)
-  set(${output_dir_var} "${output_dir}" PARENT_SCOPE)
+  if(arg_MKDIR)
+    file(MAKE_DIRECTORY "${output_dir}")
+  endif()
+
+  if(arg_DIR_PATH_VAR)
+    set(${arg_DIR_PATH_VAR} "${output_dir}" PARENT_SCOPE)
+  endif()
+
+  if(arg_FILE_PATH_VAR)
+    get_filename_component(filename "${file}" NAME)
+    set(${arg_FILE_PATH_VAR} "${output_dir}/${filename}" PARENT_SCOPE)
+  endif()
+
+  if(arg_PREFIX_PATH_VAR)
+    get_filename_component(filename_no_ext "${file}" NAME_WE)
+    set(${arg_PREFIX_PATH_VAR} "${output_dir}/${filename_no_ext}" PARENT_SCOPE)
+  endif()
 endfunction()
 
 function(_opendds_tao_idl target)
@@ -95,9 +137,15 @@ function(_opendds_tao_idl target)
 
   # convert all include paths to be relative to binary tree instead of to source tree
   file(RELATIVE_PATH rel_path_to_source_tree ${working_binary_dir} ${working_source_dir})
+  set(remove_next_opt FALSE)
   foreach(flag ${arg_IDL_FLAGS})
     if("${flag}" MATCHES "^-I(\\.\\..*)")
       list(APPEND converted_flags "-I${rel_path_to_source_tree}/${CMAKE_MATCH_1}")
+    elseif("${flag}" MATCHES "^-o[SA]?$")
+      # Omit orignal -o* options because of https://github.com/DOCGroup/ACE_TAO/issues/2202
+      set(remove_next_opt TRUE)
+    elseif(remove_next_opt)
+      set(remove_next_opt FALSE)
     else()
       list(APPEND converted_flags ${flag})
       # if the flag is like "-Wb,stub_export_file=filename" then set the variable
@@ -139,23 +187,24 @@ function(_opendds_tao_idl target)
 
   foreach(idl_file ${arg_IDL_FILES})
     set(added_output_args)
-    _opendds_get_generated_idl_output(
-      ${target} "${arg_INCLUDE_BASE}" "${idl_file}" "${idl_cmd_arg_-o}" output_prefix output_dir)
+    _opendds_get_generated_output(${target} "${idl_file}"
+      INCLUDE_BASE "${arg_INCLUDE_BASE}" O_OPT "${idl_cmd_arg_-o}" MKDIR
+      PREFIX_PATH_VAR output_prefix DIR_PATH_VAR output_dir)
     list(APPEND auto_includes "${output_dir}")
     list(APPEND added_output_args "-o" "${output_dir}")
     if(idl_cmd_arg_-oS)
-      _opendds_get_generated_idl_output(
-        ${target} "${arg_INCLUDE_BASE}" ${idl_file} "${idl_cmd_arg_-oS}"
-        skel_output_prefix skel_output_dir)
+      _opendds_get_generated_output(${target} "${idl_file}"
+        INCLUDE_BASE "${arg_INCLUDE_BASE}" O_OPT "${idl_cmd_arg_-oS}" MKDIR
+        PREFIX_PATH_VAR skel_output_prefix DIR_PATH_VAR skel_output_dir)
       list(APPEND auto_includes "${skel_output_dir}")
       list(APPEND added_output_args "-oS" "${skel_output_dir}")
     else()
       set(skel_output_prefix "${output_prefix}")
     endif()
     if(idl_cmd_arg_-oA)
-      _opendds_get_generated_idl_output(
-        ${target} "${arg_INCLUDE_BASE}" "${idl_file}" "${idl_cmd_arg_-oA}"
-        anyop_output_prefix anyop_output_dir)
+      _opendds_get_generated_output(${target} "${idl_file}"
+      INCLUDE_BASE "${arg_INCLUDE_BASE}" O_OPT "${idl_cmd_arg_-oA}" MKDIR
+        PREFIX_PATH_VAR anyop_output_prefix DIR_PATH_VAR anyop_output_dir)
       list(APPEND auto_includes "${anyop_output_dir}")
       list(APPEND added_output_args "-oA" "${anyop_output_dir}")
     else()
@@ -221,6 +270,25 @@ function(_opendds_tao_idl target)
 
     _opendds_tao_append_runtime_lib_dir_to_path(extra_lib_dirs)
 
+    set(tao_idl "$<TARGET_FILE:TAO::tao_idl>")
+    if(CMAKE_GENERATOR STREQUAL "Ninja" AND TAO_IS_BEING_BUILT)
+      if(CMAKE_VERSION VERSION_LESS 3.24)
+        message(FATAL_ERROR "Using Ninja to build ACE/TAO requires CMake 3.24 or later. "
+         "Please build ACE/TAO separately, use a newer CMake, or a different CMake generator.")
+      else()
+        set(tao_idl "$<PATH:ABSOLUTE_PATH,${tao_idl},\${cmake_ninja_workdir}>")
+        set(gperf_location "$<PATH:ABSOLUTE_PATH,${gperf_location},\${cmake_ninja_workdir}>")
+      endif()
+    endif()
+    set(tao_idl_args
+      -g ${gperf_location} ${feature_flags} -Sg
+      -Wb,pre_include=ace/pre.h -Wb,post_include=ace/post.h
+      --idl-version 4 -as --unknown-annotations ignore
+      -I${TAO_INCLUDE_DIR} -I${working_source_dir}
+      ${converted_flags}
+      ${added_output_args}
+      ${idl_file_path}
+    )
     set(generated_files
       ${stub_header_files}
       ${skel_header_files}
@@ -230,37 +298,20 @@ function(_opendds_tao_idl target)
       ${anyop_cpp_files}
     )
     if(debug)
+      message(STATUS "tao_idl ${tao_idl_args}")
       foreach(generated_file ${generated_files})
         string(REPLACE "${output_dir}/" "" generated_file "${generated_file}")
         string(REPLACE "${skel_output_dir}/" "" generated_file "${generated_file}")
         string(REPLACE "${anyop_output_dir}/" "" generated_file "${generated_file}")
-        message(STATUS "tao_idl: ${generated_file}")
+        message(STATUS "${generated_file}")
       endforeach()
     endif()
-
-    set(tao_idl "$<TARGET_FILE:TAO::tao_idl>")
-    if(CMAKE_GENERATOR STREQUAL "Ninja" AND TAO_IS_BEING_BUILT)
-      if(CMAKE_VERSION VERSION_LESS 3.24)
-        message(FATAL_ERROR "Using Ninja to build ACE/TAO requires CMake 3.24 or later. "
-         "Please build ACE/TAO separately, use a newer CMake, or a different CMake generator.")
-      else()
-        set(tao_idl "$<PATH:ABSOLUTE_PATH,${tao_idl},\${cmake_ninja_workdir}>")
-        set(gperf_location "$<PATH:ABSOLUTE_PATH,${gperf_location},\${cmake_ninja_workdir}>")
-      endif()
-    endif()
     add_custom_command(
       OUTPUT ${generated_files}
       DEPENDS TAO::tao_idl ${tao_idl_shared_libs} ACE::ace_gperf
       MAIN_DEPENDENCY ${idl_file_path}
       COMMAND ${CMAKE_COMMAND} -E env "DDS_ROOT=${DDS_ROOT}" "TAO_ROOT=${TAO_INCLUDE_DIR}"
-        "${extra_lib_dirs}"
-        "${tao_idl}" -g ${gperf_location} ${feature_flags} -Sg
-        -Wb,pre_include=ace/pre.h -Wb,post_include=ace/post.h
-        --idl-version 4 -as --unknown-annotations ignore
-        -I${TAO_INCLUDE_DIR} -I${working_source_dir}
-        ${converted_flags}
-        ${added_output_args}
-        ${idl_file_path}
+        "${extra_lib_dirs}" "${tao_idl}" ${tao_idl_args}
     )
 
     set_property(SOURCE ${idl_file_path} APPEND PROPERTY