Removed OPENDDS_FILENAME_ONLY_INCLUDES setting which is deprecated.

[doug1234]

No description provided.

[doug1234]

Any chance this review can get moved along?

[iguessthislldo]

Tried this out, but ran into issues with the test I came up with that I'm trying to resolve in OpenDDS/OpenDDS#4489.

[iguessthislldo]

It is gone now. Did that fix your test?

OPENDDS_ALLOW_ENV_CHANGE doesn't do anything, it was just an observation I made last week.

The test I've been using is the client-server one, but I changed it so it has two IDL files:

example.idl
test_data/test_data.idl

idl2library(IDLS example.idl test_data/test_data.idl)

This is to test that INCLUDE_BASE can work properly in this situation. It tries to properly handle the directory structure of the IDL files as opposed to OPENDDS_FILENAME_ONLY_INCLUDES which worked around the issue completely.

I ran into 2 issues:

1. Logic for dealing with INCLUDE_BASE and the generated export header for shared libraries broke because of how they interacted wasn't fully fleshed out. I have fixed that in Make Generated Output in CMake More Robust OpenDDS#4489. I also was able to improve the overall logic of INCLUDE_BASE.

2. As I guessed INCLUDE_BASE ${${current_idl_target}_ABSDIR} assumes a flat directory structure that causes the build to fail:

   /home/fred/work/OpenDDW/OpenDDW/build/example/opendds_generated/exampleC.h:68:10: fatal error: test_data/test_dataC.h: No such file or directory
      68 | #include "test_data/test_dataC.h"
         |          ^~~~~~~~~~~~~~~~~~~~~~~~
   

   Changing INCLUDE_BASE to ${CMAKE_CURRENT_SOURCE_DIR} does fix the issue.

[doug1234]

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?

[iguessthislldo]

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.

[doug1234]

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.

[iguessthislldo]

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

[doug1234]

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