JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
9 stars 4 forks source link

Unable to find ioda-engines from ioda-converters when in a bundle. #315

Closed aerorahul closed 4 years ago

aerorahul commented 4 years ago

jedi and ioda_engines from ioda-engines are required libraries for mapping raw data into IODA objects. In the ioda-converters, we are trying to find those libraries, while building in an ioda-bundle.

The ioda-bundle branch is: feature/ioda-engines-ioda-integration The ioda-converters branch is: feature/bufr-converter

find_package(jedi REQUIRED) and find_package(ioda_engines REQUIRED) fail with the following error:

Make Error at ioda-converters/CMakeLists.txt:90 (find_package):
  By not providing "Findjedi.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "jedi", but
  CMake did not find one.

  Could not find a package configuration file provided by "jedi" with any of
  the following names:

    jediConfig.cmake
    jedi-config.cmake

  Add the installation prefix of "jedi" to CMAKE_PREFIX_PATH or set
  "jedi_DIR" to a directory containing one of the above files.  If "jedi"
  provides a separate development package or SDK, be sure it has been
  installed.

We are building ioda-engines as part of the ioda-bundle.

Not sure how to proceed.

aerorahul commented 4 years ago

I am guessing the issue at the core is ioda-engines is vanilla cmake and not ecbuild_project and thus the other components of the bundle (in this case ioda-converters) does not know about where to look for ioda-engines.

srherbener commented 4 years ago

@aerorahul, not sure what's going wrong. I'm finding jedi and ioda-engines okay with my work for ioda.

I'm using the feature/ioda-engines-ioda-integration branch in ioda-bundle, and the develop branch for ioda-engines.

Here's what I have in my ioda CMakeLists.txt file:

target_link_libraries( ${PROJECT_NAME} PUBLIC ioda-engines )
aerorahul commented 4 years ago

@srherbener where in ioda are you using ioda-engines? where is the find_package(ioda_engines) line in ioda?

aerorahul commented 4 years ago

Ok. I see.
So there is no need for find_package(ioda_engines) I will try that.

srherbener commented 4 years ago

I believe that that you don't need the find_package command when using an ecbuild_bundle command.

aerorahul commented 4 years ago

ioda does not need to do find_package(ioda_engines)? and yet you can do target_link_libraries( ${PROJECT_NAME} PUBLIC ioda-engines )

There is no check if that package is present or not. What's to say if the targets were not created/configured correctly?

aerorahul commented 4 years ago

Also, if I want to build ioda-converters out of the bundle I will need the find_package. How do I keep that line and skip it (because I don't need to check for it) when I am in a bundle? This implies I will always have to be in the bundle.

srherbener commented 4 years ago

I don't think it hurts to have the find_package command when using the ecbuild_bundle method. And as you say, including the find_package makes it so you can switch back and forth.

I wonder if the issue is that the find_package is creating a different name for referencing ioda-engines than whatever ecbuild_bundle is doing. We've probably got some inconsistency somewhere in this regard.

srherbener commented 4 years ago

I just looked at ecbuild_bundle source code and what it does is point the find_package search to the build area:

 # Point CMake to the packages in the bundle when using find_package
  set( CMAKE_PREFIX_PATH ${CMAKE_BINARY_DIR} ${CMAKE_PREFIX_PATH} )
srherbener commented 4 years ago

Do you have a ioda-engines package installed somewhere where find_package could locate it? Eg, in /usr/local or you have ioda_engines_ROOT set to where it is installed. If this is true, perhaps the ecbuild_bundle trick just confuses find_package or some other part of the cmake configuration. Maybe you need to unset the variable or uninstall ioda-engines if this is the case.

aerorahul commented 4 years ago

@srherbener No, ioda-engines is being built as part of the bundle. I don't set any variables. I added:

list(APPEND INGESTER_LIBS_DEP
  Eigen3::Eigen
  eckit
  oops
  ioda-engines
  bufr::bufr_4_DA)

ecbuild_add_library( TARGET   ingester
                     SOURCES  ${_ingester_srcs}
                     LIBS     ${INGESTER_LIBS_DEP}
                     INSTALL_HEADERS LISTED
                     HEADER_DESTINATION ${INSTALL_INCLUDE_DIR}/iodaconv
                     LINKER_LANGUAGE ${IODACONV_LINKER_LANGUAGE}
                    )

And that works, although I don't have code in place for ioda-engines.

I had to remove the line find_package(ioda-engines) because it throws this error during configuration with ecbuild:

CMake Error at ioda-converters/CMakeLists.txt:75 (find_package):
  By not providing "Findioda-engines.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "ioda-engines", but CMake did not find one.

  Could not find a package configuration file provided by "ioda-engines" with
  any of the following names:

    ioda-enginesConfig.cmake
    ioda-engines-config.cmake

  Add the installation prefix of "ioda-engines" to CMAKE_PREFIX_PATH or set
  "ioda-engines_DIR" to a directory containing one of the above files.  If
  "ioda-engines" provides a separate development package or SDK, be sure it
  has been installed.

-- Configuring incomplete, errors occurred!

ecbuild or not, there should be a line in the top-level CMakeLists.txt that find's packages that the current project is going to need.

aerorahul commented 4 years ago

Since I can link ioda-converters with ioda-engines in the bundle, I will close this issue. Thanks @srherbener

srherbener commented 4 years ago

I agree, you shouldn't have to edit a CMakeLists.txt file when switching between ecbuild_bundle and using an installed library.

I wonder if we should leave this open to remind us to address this later on.

I'm glad you are up and working now.

aerorahul commented 4 years ago

I think I will leave this issue closed. @markjolah said he will refactor the feature/bufr-converter branch with removing ecbuild from it as he did in develop. I think he will do it properly (and have a proper answer).

srherbener commented 4 years ago

Sounds good. Thanks

rmclaren commented 4 years ago

Unfortunately the prescribed changes don't really seem to produce a working build... I've updated the ioda-converter feature/bufr-converter branch with my latest changes. If you look at src/bufr/CMakeLists.txt you will notice we're I manually specified a bunch of things in order to get make things work... There seem to bee several issues:

1) Its unable to find the header files for the project ioda-engines. 2) Its unable to find the library files for the project ioda-engines. 3) Looking for a library file with the wrong name (looking for ioda-engines.dylib but libioda-engines.dylib is what it is actually called).

Anyways...

aerorahul commented 4 years ago

Unfortunately the prescribed changes don't really seem to produce a working build... I've updated the ioda-converter feature/bufr-converter branch with my latest changes. If you look at src/bufr/CMakeLists.txt you will notice we're I manually specified a bunch of things in order to get make things work... There seem to bee several issues:

1) Its unable to find the header files for the project ioda-engines.

2) Its unable to find the library files for the project ioda-engines.

3) Looking for a library file with the wrong name (looking for ioda-engines.dylib but libioda-engines.dylib is what it is actually called).

Anyways...

I am going to let @markjolah handle that when he gets to it. This seems rather messy.

srherbener commented 4 years ago

@rmclaren if the feature/bufr-converter branch CMake configuration is working for you now I would leave it as is until we get our c++14 compatibility issues straightened out. @markjolah has a set of PRs that will allow the usage of bufrlib or NCEPLIBS-bufr in ioda-converters, but these are being held up with the c++14 work. Once we get the c++14 issues resolved (which should be soon) and @markjolah's PRs merged in, then I would suggest syncing the feature/bufr-converter branch with develop. At this point you should be able to create a much cleaner CMake configuration.

rmclaren commented 4 years ago

Sounds good, Thanks!