Install RtpsRelay Library #2864
Conversation
Also avoid installing headers for OpenDDS Util
iguessthislldo
changed the title
| @@ -6,7 +6,7 @@ project: acelib, dds_macros, install { | |||
| libout = $(DDS_ROOT)/lib | |||
| pch_header = | |||
| pch_source = | |||
| macros += NOTAO | |||
| macros += OPENDDS_NO_TAO | |||
There was a problem hiding this comment.
I think we originally named it that way since the macro doesn't appear in headers. Not that I mind changing it.
There was a problem hiding this comment.
Yeah, it's probably not as important that it starts with OpenDDS as one that's defined normally since that would leak into a user's code, but it still a macro we're using in a header, so it'd be safer if it's prefixed. It's possible, though unlikely, that it could be defined somewhere else by the user or another library.
There was a problem hiding this comment.
Scratch that I missed the whole "not in the headers" part.
There was a problem hiding this comment.
So I think I will keep it, because it's a bit safer, but I'm not sure I would have changed it if I realized it's only for the Serializer.cpp. I think it being in the mpb file threw me off.
MPC/config/opendds_util.mpb
Outdated
| @@ -3,6 +3,5 @@ project: acelib, dds_macros { | |||
| libs += OpenDDS_Util | |||
| includes += $(DDS_ROOT) | |||
| libpaths += $(DDS_ROOT)/lib | |||
| macros += NOTAO | |||
|
|
|||
| macros += OPENDDS_NO_TAO | |||
There was a problem hiding this comment.
Should this line be removed instead of edited? (see comment on the corresponding .mpc file)
There was a problem hiding this comment.
Yes, this can go away.
dds/DCPS/OpenDDS_Util.mpc
Outdated
| @@ -18,6 +18,10 @@ project: acelib, dds_macros, install { | |||
| XTypes/TypeObject.h | |||
| } | |||
|
|
|||
| specific { | |||
| install_dir = dds/DCPS | |||
There was a problem hiding this comment.
This project doesn't need to install headers since DdsDcps will -- but maybe we don't have a way to do that?
There was a problem hiding this comment.
We do, I actually had it like that previously. I changed it because I didn't know if there was going to point where we might have headers in Util that won't be in Dcps. If you don't thinks that's likely though I don't have a problem with reverting to what I had before.
There was a problem hiding this comment.
Yes, I'd prefer not to have headers from this project installed since they're a subset of the headers already installed
| @@ -1,10 +1,10 @@ | |||
| #ifdef OPENDDS_HAS_CXX11 | |||
|
|
|||
| #include <gtest/gtest.h> | |||
| #include <OpenDDS_RtpsRelayLib/Name.h> | |||
There was a problem hiding this comment.
We don't have this style of directory name anywhere else in the repo. Since $(DDS_ROOT)/tools is on the include path, how about making this include dds/rtpsrelaylib/Name.h with the actual file being tools/dds/rtpsrelaylib/Name.h? Then we have the same top-level directory after $prefix/include as the one that's already in use.
There was a problem hiding this comment.
We don't have this style of directory name anywhere else in the repo.
No, I based it on the library filenames but I'd like us to eventually transition to something where the include path starts with some form of OpenDDS. This is related to #2160.
Since $(DDS_ROOT)/tools is on the include path, how about making this include dds/rtpsrelaylib/Name.h with the actual file being tools/dds/rtpsrelaylib/Name.h?
Maybe... but I don't know about using dds because of the reason above and I'm not sure it should be the same name as the main directory that it's not part of. It sounds a bit confusing.
There was a problem hiding this comment.
Here's an example from ACE https://github.com/DOCGroup/ACE_TAO/tree/master/ACE/protocols/ace
We can certainly eventually transition to something else, but I'd rather not do that for different libraries at different times
There was a problem hiding this comment.
The fact that ACE does it doesn't make me feel any better about it.
There was a problem hiding this comment.
Yes, I was assuming that was the case when I wrote the last comment.
However I do think we need to be consistent across different libraries in the same version. A hypothetical change to a new scheme should come later and apply to all libraries.
OpenDDS_RtpsRelayso the header names are proper when installedtoolsto avoid putting the files intools/rtpsrelayon the include pathOpenDDS_RtpsRelayand the other installed headers to the same lint checks as theddsinstalled headers and deal with the issues arising from that.