Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed OPENDDS_FILENAME_ONLY_INCLUDES setting which is deprecated. #24

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Removed OPENDDS_FILENAME_ONLY_INCLUDES setting which is deprecated. #24

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
534bda0
Removed OPENDDS_FILENAME_ONLY_INCLUDES setting which is deprecated.
Feb 23, 2024
6674277
Remove unnecessary option.
Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions idl2library.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ function(idl2library)
option(OPENDDS_CPP11_IDL_MAPPING "Use C++11 IDL mapping" OFF)
option(OPENDDS_CMAKE_VERBOSE "Print verbose output when loading the OpenDDS Config Package" ON)

#This is an option, but we set it as a forced cache var here because otherwise if you include OpenDDS before OpenDDSManager it will default off..
set(OPENDDS_FILENAME_ONLY_INCLUDES ON CACHE BOOL "No directory info in generated #includes." FORCE)

find_package(OpenDDS REQUIRED)

if(NOT IDL_WISHLIST)
Expand Down Expand Up @@ -185,6 +182,7 @@ function(idl2library)
${${current_idl_target}_RELPATH}
OPENDDS_IDL_OPTIONS ${current_idl_include_opts}
TAO_IDL_OPTIONS ${current_idl_include_opts}
INCLUDE_BASE ${${current_idl_target}_ABSDIR}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand what you are saying is the problem. But what do you propose we do to solve it? Are you going to make another opendds change to fix issue 2?

No, 2 is a decision that has to be made in OpenDDW here:

Suggested change
INCLUDE_BASE ${${current_idl_target}_ABSDIR}
INCLUDE_BASE "${CMAKE_CURRENT_SOURCE_DIR}"

That's the change that was needed for my example, but what this needs to be depends on how the IDL is organized. My IDL was in different directories because that's how I test INCLUDE_BASE, but INCLUDE_BASE ${${current_idl_target}_ABSDIR} would only work if all the IDL is all in the same directory. I guess what really matters is does INCLUDE_BASE ${${current_idl_target}_ABSDIR} work for all your IDL? If so then how this is now is probably okay, and we can merge this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I think I understand better. Lets look at this from a bit different angle: The goal of this PR is to have it work the same as it did before without the warning messages. Have we achieved that? We can look into making it better in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will probably work the same, but that really depends on the IDL. Have you tested with all your IDL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I have tested it with both our project and our examples.

)
target_link_libraries(${current_idl_target}
${IDL_TARGET_DEPENDENCIES}
Expand Down
1 change: 0 additions & 1 deletion openddw-config.in.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ include(CMakeFindDependencyMacro)
# Find OpenDDS
#This allows DDS_ROOT to be set off OpenDDSConfig.cmake location
option(OPENDDS_ALLOW_ENV_CHANGE "Allow multiple find_package(opendds) calls." ON)
doug1234 marked this conversation as resolved.
Show resolved Hide resolved
option(OPENDDS_FILENAME_ONLY_INCLUDES "No directory info in generated #includes." ON)
find_package(OpenDDS REQUIRED)

#Include idl2dll.cmake so it can be used if/when needed
Expand Down