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.

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

Elliottc/cmake support #164

Merged
merged 45 commits into from
Jan 3, 2023
Merged

Elliottc/cmake support #164

merged 45 commits into from
Jan 3, 2023

Conversation

ocielliottc
Copy link
Contributor

Limited support for CMake.

Custom commands are not inserted into the generated CMakeLists.txt files. It is expected that the user will provide a package or module that will handle generated files in a CMake fashion.

… have been modified to replace environment variables.
… regular expressions for splitting the string work properly.
…s. Also, added output_option to custom_commands.
…notlast, default template variable values, and scope function parameters. Utilize this to correct processing errors in the cmake template.
… and out of the target property. Having it in the target property caused CMake to lose all of the project dependencies.
templates/cmake.mpd Outdated Show resolved Hide resolved
@jwillemsen
Copy link
Member

Any documentation additions needed, like what are the restrictions, how to add find_package(), etc?

@jwillemsen
Copy link
Member

I will do another review early next week, do you plan to add an example to ace/tao/opendds, would be nice when we would build/test tese examples as part of the github actions

…ated to projects that use the generated project.
…he user to set the variable at the CMake level, instead of in the environment.
@ocielliottc ocielliottc merged commit 60b0c90 into master Jan 3, 2023
@ocielliottc ocielliottc deleted the elliottc/cmake-support branch January 3, 2023 13:37
@alexchandel
Copy link

What's the recommended way to invoke this?

@alexchandel
Copy link

Naively using it gives an error:

~/opt/MPC/mpc.pl -type cmake ace/ace.mpc
Generating 'cmake' output using ace/ace.mpc
ace.mpc: line 2:
ERROR: Unable to locate parent: support_ostream
ERROR: Unable to process: ace.mpc

@mitza-oci
Copy link
Member

@ocielliottc
Copy link
Contributor Author

The main thing is that if you're building ACE or ACE related projects, you're going to need to run MPC out of $ACE_ROOT/bin/ instead of $MPC_ROOT.

@alexchandel
Copy link

I cloned latest MPC and tried with ACE 7.0.11, by setting MPC_ROOT but still error:

# created config.h and platform_macros.GNU
$ export MPC_ROOT=$HOME/Git/MPC
$ export ACE_ROOT=$HOME/Downloads/ACE_wrappers
$ cd $ACE_ROOT/bin
$ $MPC_ROOT/mwc.pl -type cmake $ACE_ROOT/ACE.mwc
Generating 'cmake' output using /Users/dev/Downloads/ACE_wrappers/ACE.mwc
XML.mpc: line 1:
ERROR: Unable to locate parent: ace_output
ACE.mwc: line 28:
ERROR: Unable to process ace/XML_Utils/XML.mpc
ERROR: Unable to process: ACE.mwc

However deleting and replacing $ACE_ROOT/MPC with the clone and perl ./mwc.pl -type cmake ../ACE.mwc works. However the TAO command still errors like above, this time for pidl_install:

perl ./mwc.pl -type cmake ../TAO/TAO_ACE.mwc 
MPC_ROOT was set to /Users/dev/Downloads/ACE_wrappers/MPC.
Using .../Downloads/ACE_wrappers/bin/MakeProjectCreator/config/MPC.cfg
Generating 'cmake' output using /Users/dev/Downloads/ACE_wrappers/TAO/TAO_ACE.mwc
Skipping ACE_XML_Utils (XML.mpc); it requires xerces.
Skipping ACE_FlReactor (ace_flreactor.mpc); it requires fl.
Skipping ACE_FOR_TAO (ace_for_tao.mpc); it requires ace_for_tao.
Skipping ACE_Qt5Reactor_moc (ace_qt5reactor.mpc); it requires qt5.
Skipping ACE_Qt5Reactor (ace_qt5reactor.mpc); it requires qt5.
Skipping ACE_Qt4Reactor_moc (ace_qt4reactor.mpc); it requires qt4.
Skipping ACE_Qt4Reactor (ace_qt4reactor.mpc); it requires qt4.
Skipping ACE_XtReactor (ace_xtreactor.mpc); it requires xt.
Skipping ACE_TkReactor (ace_tkreactor.mpc); it requires tk.
Skipping QoS (qos.mpc); it requires qos.
Skipping ace_svcconf_gen (svcconfgen.mpc); it requires ace_svcconf_gen.
Skipping ACE_FoxReactor (ace_foxreactor.mpc); it requires fox.
Skipping SSL (ssl.mpc); it requires ssl.
Skipping SSL_FOR_TAO (ssl_for_tao.mpc); it requires ssl.
Skipping INet_SSL (inet_ssl.mpc); it requires ssl.
PortableServer.mpc: line 1:
ERROR: Unable to locate parent: pidl_install
TAO_ACE.mwc: line 22:
ERROR: Unable to process tao/PortableServer/PortableServer.mpc
ERROR: Unable to process: TAO_ACE.mwc

(Note that I need this to work on older ACE+TAO (6.3.3 or 6.5.11), not just latest.)

@iguessthislldo
Copy link
Member

Try running MPC from TAO_ROOT: perl $ACE_ROOT/bin/mwc.pl -type cmake TAO_ACE.mwc . I can reproduce your original error by running from ACE_ROOT.

However I should say building TAO wasn't in scope for these changes and I know it's not going to work. It should be possible, but it just wasn't a priority for us. @ocielliottc could comment on what else is needed, but I know there's currently no way for CMake to generate files from IDL. I think it was mentioned somewhere earlier in this PR, but someone could take the CMake tao_idl support from OpenDDS and adapt it for TAO.

@alexchandel
Copy link

No luck, same error:

$ perl $ACE_ROOT/bin/mwc.pl -type cmake TAO_ACE.mwc
MPC_ROOT was set to /Users/dev/Downloads/ACE_wrappers/MPC.
Using .../Downloads/ACE_wrappers/bin/MakeProjectCreator/config/MPC.cfg
Generating 'cmake' output using TAO_ACE.mwc
Skipping ACE_XML_Utils (XML.mpc); it requires xerces.
Skipping ACE_FlReactor (ace_flreactor.mpc); it requires fl.
Skipping ACE_FOR_TAO (ace_for_tao.mpc); it requires ace_for_tao.
Skipping ACE_Qt5Reactor_moc (ace_qt5reactor.mpc); it requires qt5.
Skipping ACE_Qt5Reactor (ace_qt5reactor.mpc); it requires qt5.
Skipping ACE_Qt4Reactor_moc (ace_qt4reactor.mpc); it requires qt4.
Skipping ACE_Qt4Reactor (ace_qt4reactor.mpc); it requires qt4.
Skipping ACE_XtReactor (ace_xtreactor.mpc); it requires xt.
Skipping ACE_TkReactor (ace_tkreactor.mpc); it requires tk.
Skipping QoS (qos.mpc); it requires qos.
Skipping ace_svcconf_gen (svcconfgen.mpc); it requires ace_svcconf_gen.
Skipping ACE_FoxReactor (ace_foxreactor.mpc); it requires fox.
Skipping SSL (ssl.mpc); it requires ssl.
Skipping SSL_FOR_TAO (ssl_for_tao.mpc); it requires ssl.
Skipping INet_SSL (inet_ssl.mpc); it requires ssl.
PortableServer.mpc: line 1:
ERROR: Unable to locate parent: pidl_install
TAO_ACE.mwc: line 22:
ERROR: Unable to process tao/PortableServer/PortableServer.mpc
ERROR: Unable to process: TAO_ACE.mwc

Deleting $ACE_ROOT/TAO/MPC and replacing it with the git clone doesn't fix it. It seems like pidl_install is being referenced from TAO but isn't provided anywhere.

However I should say building TAO wasn't in scope for these changes and I know it's not going to work. It should be possible, but it just wasn't a priority for us. @ocielliottc could comment on what else is needed, but I know there's currently no way for CMake to generate files from IDL.

My primary need, greater than building ACE/TAO via CMake, is just generating CMake lists that export the ACE/TAO packages (built by whatever means), so other CMake projects can use them.

@ocielliottc
Copy link
Contributor Author

It appears that you do not have the TAO_ROOT environment variable set. That could explain why MPC can't find pidl_install.

At any rate, as @iguessthislldo said, cmake support for TAO isn't available at this time. ACE, gperf, and the tests should build with cmake.

@mitza-oci
Copy link
Member

My primary need, greater than building ACE/TAO via CMake, is just generating CMake lists that export the ACE/TAO packages (built by whatever means), so other CMake projects can use them.

This is what I was referring to when I answered the comment here DOCGroup/ACE_TAO#266 (comment) the other day.

What you're asking for already exists. Here's an example, it may help to download and build this code locally to learn more about it. Everything required is open source and all the build steps are running in GitHub Actions on multiple platforms.

Our "downstream" application in this example is https://github.com/OpenDDS/opendds-monitor
This application has no MPC build files in it, only CMake. It builds libraries and applications that link to the ACE+TAO libraries and use tao_idl for code generation.

@alexchandel
Copy link

I'm trying to work out how to extract OpenDDS's ACE+TAO CMake bits. Is it necessary to run $DDS_ROOT/configure to generate enough of the CMake interface for ACE+TAO?

@alexchandel
Copy link

@ocielliottc That was sufficient.

export ACE_ROOT=$HOME/Downloads/ACE_wrappers
export TAO_ROOT=$ACE_ROOT/TAO
export MPC_ROOT=$ACE_ROOT/MPC

rm -r $ACE_ROOT/MPC
git clone https://github.com/DOCGroup/MPC.git $ACE_ROOT/MPC

cd $ACE_ROOT/bin
perl ./mwc.pl -type cmake ../ACE.mwc
cd $ACE_ROOT/TAO
perl $ACE_ROOT/bin/mwc.pl -type cmake TAO_ACE.mwc

If ACE+TAO is built with MPC before or after this, will the generated CMakeLists export those build artifacts correctly, or try to rebuild them?

@ocielliottc
Copy link
Contributor Author

That wouldn't work. CMake would still want to build everything because the object files and libraries wouldn't be where it would expect them to be.

It might work if you generate all of the code from the IDL and then try to build using CMake. But, I really don't know if this would work. I've never tried it.

@mitza-oci
Copy link
Member

I'm trying to work out how to extract OpenDDS's ACE+TAO CMake bits. Is it necessary to run $DDS_ROOT/configure to generate enough of the CMake interface for ACE+TAO?

It's not necessary. If configure is not run, create the file cmake/config.cmake with a few set statements for variables that the other CMake files will need. It should at least include the locations of ACE, TAO, and MPC. It could also set other variables. Descriptions of these variables and how the system works overall are available at $DDS_ROOT/docs/cmake.md.

set(OPENDDS_MPC $ENV{MPC_ROOT})
set(OPENDDS_ACE $ENV{ACE_ROOT})
set(OPENDDS_TAO $ENV{TAO_ROOT})

@iguessthislldo
Copy link
Member

I'm trying to work out how to extract OpenDDS's ACE+TAO CMake bits. Is it necessary to run $DDS_ROOT/configure to generate enough of the CMake interface for ACE+TAO?

Technically no, all the CMake module needs is the config.cmake file, but it might be helpful to run configure to see what that looks like.

If ACE+TAO is built with MPC before or after this, will the generated CMakeLists export those build artifacts correctly, or try to rebuild them?

I don't know if this has been said yet, but just to make sure there's no misunderstanding, the changes in this PR do not allow the generated CMake to export anything, only build what the mpc files are describing. The way MPC is designed separates build (mpc files) and linking information (mpb files), and so it's not easy to combine those into one CMake project.

@alexchandel
Copy link

I don't know if this has been said yet, but just to make sure there's no misunderstanding, the changes in this PR do not allow the generated CMake to export anything, only build what the mpc files are describing.

@iguessthislldo That is good to know, I need to link against ACE+TAO from a downstream CMake project, not just build them. This limitation doesn't apply to the OpenDDS CMake stuff, right? opendds-monitor links against things that CMake exports from OpenDDS, ACE, and TAO?

@iguessthislldo
Copy link
Member

iguessthislldo commented Feb 3, 2023

This limitation doesn't apply to the OpenDDS CMake stuff, right? opendds-monitor links against things that CMake exports from OpenDDS, ACE, and TAO?

Yes, that's correct. The CMake support in OpenDDS was created to allow linking with MPC-built OpenDDS and ACE/TAO. If that's all you need then a version with the OpenDDS support stripped out should work for you.

@alexchandel
Copy link

It's quite difficult to figure out which bits of OpenDDS/cmake are just necessary for ACE+TAO …

Is it necessary to configure and build OpenDDS and its dependencies without CMake, as the opendds-monitor action appears to do, before using it from a downstream CMake project?

iguessthislldo added a commit to iguessthislldo/OpenDDS that referenced this pull request Feb 5, 2023
@iguessthislldo
Copy link
Member

It's quite difficult to figure out which bits of OpenDDS/cmake are just necessary for ACE+TAO …

Yes I can imagine. I can give you a starting point. I started making a list of what would be the bare minimum to remove, but I realized that it would be easier if I just did it and pushed it to a branch. It also allowed me to test that it works. Some notes on it:

  • I added an example in no-opendds-cmake-example and was able to build it. Steps to build:
    • ACE/TAO has to be built.
    • I configured OpenDDS with the existing ACE/TAO, but did not build OpenDDS just to get the config.cmake file generated. This was just because this was easiest for me, but as mentioned before you can write this by hand instead.
    • After that it's basically a normal CMake build: cd no-opendds-cmake-example, cmake -S . -B build, cmake --build build.
      • If CMake can't find OpenDDS module, it will probably need to be added to CMAKE_MODULE_PATH.
  • I'm not sure we have all the TAO libraries defined, so some might have to be added to _tao_libs if you need to use them.
  • The module handles dependencies for OpenDDS libraries so, for example a user of OpenDDS library X needs to include library Y headers then it will. This didn't appear to be done with any of TAO, so there might be cases where that causes a problem and the dependencies have to be added. See the *_DEPS variables in OpenDDSConfig.cmake if you need examples of that. I did make it so TAO is dependent on ACE, so ACE can be included in tao_idl generated code without the user explicitly linking ACE::ACE.

As I said this is the bare minimum and is basically just skipping the critical OpenDDS parts. It would take a decent amount more work to turn into a proper thing that doesn't mention OpenDDS, but I hope it helps.

Is it necessary to configure and build OpenDDS and its dependencies without CMake, as the opendds-monitor action appears to do, before using it from a downstream CMake project?

I'm not sure if this answer is needed anymore, but yes, the CMake module needs to be able to find the built libraries and application from OpenDDS and ACE/TAO.

@alexchandel
Copy link

alexchandel commented Feb 7, 2023

Thank you, I've been chewing through this.

I'm not sure if this answer is needed anymore, but yes, the CMake module needs to be able to find the built libraries and application from OpenDDS and ACE/TAO.

Just making sure I wasn't missing anything; OpenDDS's CMake module just exports (not builds), while this PR can build.

I'm not sure we have all the TAO libraries defined, so some might have to be added to _tao_libs if you need to use them.

That's ok. These correspond to the MPC project sharedname?

I'm also trying to understand, what's left before this PR's generated CMake is usable? MPC appears to spit out CMakeLists for every target, even the many TAO/orbsvcs/orbsvcs projects (e.g. TAO/orbsvcs/orbsvcs/CMakeLists.CosNaming and TAO/orbsvcs/orbsvcs/CMakeLists.CosNaming_*). But the _idl projects don't declare a target, and they call IDL_FILES_TARGET_SOURCES implemented here with tao_idl_command and opendds_get_generated_idl_output. What's the latter supposed to do? Is this just missing the right target?

@alexchandel
Copy link

alexchandel commented Feb 8, 2023

@ocielliottc So, tweaking the CMake template to emit custom targets for IDL files has gotten me very close to a complete CMake build generated from MPC. (cmake.mpd)

Because the consumer projects (e.g. AnyTypeCode) reference generated IDL files directly (e.g. BoundsC.cpp) rather than by IDL file, I had to modify the CMake scripts from here to perform in-tree IDL generation. (idl_files.cmake, tao_idl_sources.cmake)

The only two targets I'm stuck on are TAO_AnyTypeCode and TAO_PortableServer. Both are in subfolders of /TAO/tao/, and uniquely, these two targets reference sources in their subfolders that are generated from IDL files in parent directories (files like *SeqA.cpp and PolicyS.cpp, generated from /TAO/tao/*Seq.pidl and /TAO/tao/Policy.pidl). Log messages say these file targets are created in the proper subdirectories, but these two consumers don't see them.

@alexchandel
Copy link

alexchandel commented Feb 9, 2023

I pushed a self-contained example here, using this fork of MPC.

If you clone and run it, you'll get these two errors (and some spurious ones after, to ignore for now):

$ cd build && cmake ..
-- The CXX compiler identification is AppleClang 14.0.0.14000029
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
CMake Error at ACE_wrappers/TAO/tao/AnyTypeCode/CMakeLists.AnyTypeCode:32 (add_library):
  Cannot find source file:
BooleanSeqA.cpp

Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
.hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc
Call Stack (most recent call first):
ACE_wrappers/TAO/tao/AnyTypeCode/CMakeLists.txt:12 (include)

CMake Error at ACE_wrappers/TAO/tao/PortableServer/CMakeLists.PortableServer:32 (add_library):
Cannot find source file:

PolicyS.cpp

Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
.hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc
Call Stack (most recent call first):
ACE_wrappers/TAO/tao/PortableServer/CMakeLists.txt:12 (include)

@alexchandel
Copy link

I now have a working CMake build of ACE+TAO with IDL generation and no hard-coding. See here with this MPC fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants