Skip to content

Conversation

@LecrisUT
Copy link
Contributor

Cherry-picked from airsim/rmol#2

@LecrisUT
Copy link
Contributor Author

The stdair.pc.in seems to be out-of-sync?

@da115115
Copy link
Member

Integrated in 2423ed8 and ebc9b33

@da115115 da115115 closed this Mar 24, 2025
@LecrisUT
Copy link
Contributor Author

Cool, let me know if you encounter any fallouts

@da115115
Copy link
Member

da115115 commented Mar 24, 2025

The change in the stdair-config.cmake.in CMake support file now specifies relative paths, rather than absolute paths and, at least on Fedora, it makes the other packages depending on it, e.g. RMOL, failing.

Example of error produced by the cmake command on RMOL:

-- Requires StdAir-1.00.0
CMake Error at /usr/lib64/cmake/stdair/stdair-config.cmake:19 (include):
  include could not find requested file:

    lib64/cmake/stdair/stdair-library-depends.cmake
Call Stack (most recent call first):
  config/project_config_embeddable.cmake:1104 (find_package)
  config/project_config_embeddable.cmake:379 (get_stdair)
  CMakeLists.txt:51 (get_external_libs)

And the content of the /usr/lib64/cmake/stdair/stdair-config.cmake file is:

...
set (STDAIR_LIBRARY_DIRS "lib64")
set (STDAIR_SAMPLE_DIR "share/stdair/samples")

# Our library dependencies (contains definitions for IMPORTED targets)
include ("lib64/cmake/stdair/stdair-library-depends.cmake")

As a comparison, the /usr/share/airrac/CMake/airrac-config.cmake file is:

...
set (AIRRAC_LIBRARY_DIRS "/usr/lib64")

# Library dependencies for AirRAC (contains definitions for the AirRAC IMPORTED
# targets)
include ("/usr/share/airrac/CMake/airrac-library-depends.cmake")

The paths are absolute here, and CMake can therefore find the support files.

Was it intentional to migrate to relative paths in the CMake support file, and if yes, do you know how to fix the error reported by packages depending on it?

@da115115 da115115 reopened this Mar 24, 2025
@LecrisUT
Copy link
Contributor Author

And the content of the /usr/lib64/cmake/stdair/stdair-config.cmake file is:

That is weird, it should be prefixing the paths with PACKAGE_PREFIX_DIR defined in the @PACKAGE_INIT@ section. The final paths should be absolute constructed from PACKAGE_PREFIX_DIR. Let me check the commit again to see if I left anything out or there is a name-clash somewhere

@da115115
Copy link
Member

And the content of the /usr/lib64/cmake/stdair/stdair-config.cmake file is:

That is weird, it should be prefixing the paths with PACKAGE_PREFIX_DIR defined in the @PACKAGE_INIT@ section. The final paths should be absolute constructed from PACKAGE_PREFIX_DIR. Let me check the commit again to see if I left anything out or there is a name-clash somewhere

Note that I had forgotten the @PACKAGE_INIT@ in the 1.00.18 release. But still, it seemed to generate relative paths rather than absolute paths.

With the 1.00.19, it seems to fix the issue:

set (STDAIR_LIBRARY_DIRS "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@")

@LecrisUT
Copy link
Contributor Author

With the 1.00.19, it seems to fix the issue:

That's not ideal because it makes it non-relocatable, which is needed for some environments like PyPI or conda. I will investigate tomorrow when I'll be able to spin up a container and see what's going on. I checked for name clashes and didn't find any, so my only other guess is that maybe there's an absolute path expansion that tricks it up. Here is the documentation page, and you could try adding INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX}, but afaiu that is already the default.

@da115115
Copy link
Member

  • Fedora (Rawhide) update with the 1.00.19 release: https://bodhi.fedoraproject.org/updates/FEDORA-2025-f41f352464

  • I'll let you investigate what the best way to fix the issue is.

  • On my side, I'll propagate the CMake-related changes to the other components, beginning with the ones failing to build on Fedora Rawhide (because I upgraded SOCI, which introduced a breaking so-name change).

@da115115
Copy link
Member

da115115 commented Mar 24, 2025

I just found out that there is another issue, this time seemingly due to the relocation of the SOCI libraries into a dedicated sub-directory, namely /usr/lib64/soci (where as these libraries were before simply located in the /usr/lib64 standard directory). For some reason, Fedora does not seem to detect the issue. But we see it clearly when displaying the library dependencies:

ldd -d -r /usr/lib64/libstdair.so
...
	libboost_filesystem.so.1.83.0 => /lib64/libboost_filesystem.so.1.83.0 (0x0000736eb429b000)
	libsoci_core.so.4 => not found
	libsoci_mysql.so.4 => not found
...
ls -lFh /usr/lib64/soci/libsoci_core.so.4*
lrwxrwxrwx 1 root root   21 Mar 24 01:00 /usr/lib64/soci/libsoci_core.so.4 -> libsoci_core.so.4.1.0*
-rwxr-xr-x 1 root root 446K Mar 24 01:00 /usr/lib64/soci/libsoci_core.so.4.1.0*
ls -lFh /usr/lib64/soci/libsoci_mysql.so.4*
lrwxrwxrwx 1 root root   22 Mar 24 01:00 /usr/lib64/soci/libsoci_mysql.so.4 -> libsoci_mysql.so.4.1.0*
-rwxr-xr-x 1 root root 127K Mar 24 01:00 /usr/lib64/soci/libsoci_mysql.so.4.1.0*

Of course, I could set the LD_LIBRARY_PATH on my own environment, but any other installation will fail.
I do not understand why this issue arises, as the following line in the CMake file adds the specific SOCI libraries to the dependencies/RPATH:

    # Update the list of dependencies for the project
    list (APPEND PROJ_DEP_LIBS_FOR_LIB ${SOCI_LIBRARIES} ${SOCIMYSQL_LIBRARIES})

If you understand/see where the problem could be, do not hesitate.

I will also check on the SOCI side => SOCI/soci#1222

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Mar 24, 2025

Rpaths are stripped by default at installation, but you can use /etc/ld.so.conf.d/ if you want to keep it in a subdirectory. https://docs.fedoraproject.org/en-US/packaging-guidelines/#alternatives-to-rpath

It depends pretty much on the environment, spack uses Lmod to populate LD_LIBRARY_PATH, PyPI uses RPATH, distro can use /etc/ld.so.conf.d/, users can use INSTALL_RPATH + CMAKE_INSTALL_RPATH_USE_LINK_PATH

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Mar 25, 2025

@da115115 Can you share with me more on your setup, I have tried in a container with CMake 3.31 and 3.15 1, and in both cases it expands the PACKAGE_* variable appropriately

Screenshot_20250325_091037

I'll check up on why PACKAGE_STDAIR_SAMPLE_DIR is seemingly failing, and fix up some other failures I happened to notice.

Edit: It seems that ${PACKAGE_NAME}_SAMPLE_DIR was not added to install_dev_helper_files. Do you want it added by default (expanding to empty), or make it configurable so that you pass it as a function variable. For proper design I would need to convert it to function, but I would be making the assumption that ${PACKAGE_NAME}_* vars are not used outside of that function.

Edit2: Oh, I'm blind there was a set(${PACKAGE_NAME}_SAMPLE_DIR) just below that

Footnotes

  1. PS you added a cmake_path command which is only available in CMake>=3.20

- Add the ``${PACKAGE_NAME}_SAMPLE_DIR` to `PATH_VARS`
- fix the `include` in stdair-config.cmake

Signed-off-by: Cristian Le <[email protected]>
@da115115
Copy link
Member

da115115 commented Mar 25, 2025

@da115115 Can you share with me more on your setup, I have tried in a container with CMake 3.31 and 3.15 1, and in both cases it expands the PACKAGE_* variable appropriately

My configuration is a Fedora Rawhide, with CMake 3.31.
I do get the same expansion results for the CMake variables as the ones you show in the screenshot.

Note that the following (latest) configuration seems to work well:

set (STDAIR_SAMPLE_DIR "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/@PROJECT_NAME@/samples")

  • For the SAMPLE variable, yes, it would be nice to have it point to @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/@PROJECT_NAME@/samples, which is translating to /usr/share/stdair/samples on Fedora. Note that this folder contains (CSV) sample data for the simulator applications (the other travel simulator-related packages, like RMOL, AirRAC, TraDemGen, TvlSim, etc)
  • By "working well", I mean that depending packages, like AirRAC for instance, will build correctly, and more specifically, the CMake find_library() function of depending packages will find the StdAir libraries without any issue. With the initial version of the stdair-config.cmake.in file, the depending packages fail to find StdAir libraries and report the error that the stdair-library-depends.cmake file cannot be found. If I understand well, that is due to the fact that @STDAIR_CMAKE_DIR@ expands to a relative path, not to an absolute one. That is why in the working version, I have added @CMAKE_INSTALL_PREFIX@/, yielding to: @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/cmake/@PROJECT_NAME@/@[email protected]

Note: this is not ideal because
- a bundled `FindBoost.cmake` is used instead of the CMake provided one, and it can break due to the policy set
- the variable `BOOST_REQUIRED_COMPONENTS` is defined in an undefined scope

Signed-off-by: Cristian Le <[email protected]>
@da115115
Copy link
Member

With respect to the cmake_path() function, you are absolutely right. It was a way to get rid of the warning for some policy about normalization of install paths. I took inspiration from jrouwe/JoltPhysics#1309

@LecrisUT
Copy link
Contributor Author

Note that the following (latest) configuration seems to work well:

set (STDAIR_SAMPLE_DIR "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/@PROJECT_NAME@/samples")

Not quite, if you run cmake --install ./build_dir --prefix some_other_path then it will still have used the same CMAKE_INSTALL_PREFIX instead of ${PACKAGE_PREFIX_DIR}. The ${PACKAGE_PREFIX_DIR} ensures it is relocatable.

Screenshot_20250325_100543

(I have edited STDAIR_SAMPLE_DIR like you did and did the cmake --install --prefix)

* For the `SAMPLE` variable, yes, it would be nice to have it point to `@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/@PROJECT_NAME@/samples`, which is translating to `/usr/share/stdair/samples` on Fedora. Note that this folder contains [(CSV) sample data](https://github.com/airsim/stdair/tree/main/samples) for the simulator applications (the other travel simulator-related packages, like RMOL, AirRAC, TraDemGen, TvlSim, etc)

Isn't it INSTALL_SAMPLE_DIR and already defined in the previous steps?

the depending packages fail to find StdAir libraries and report the error that the stdair-library-depends.cmake file cannot be found.

Yes, I found that issue as well and fixed it in 8fce616. ${CMAKE_CURRENT_LIST_DIR} is the recommended approach to create paths relative to the current file.

If I understand well, that is due to the fact that @STDAIR_CMAKE_DIR@ expands to a relative path

STDAIR_CMAKE_DIR should only be used in the configure_package_config_file() and install() commands, not in any other expansions.

@da115115
Copy link
Member

Note that the following (latest) configuration seems to work well:

set (STDAIR_SAMPLE_DIR "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/@PROJECT_NAME@/samples")

Not quite, if you run cmake --install ./build_dir --prefix some_other_path then it will still have used the same CMAKE_INSTALL_PREFIX instead of ${PACKAGE_PREFIX_DIR}. The ${PACKAGE_PREFIX_DIR} ensures it is relocatable.

Ok, thanks for the detailed explanations and examples of use cases, I now get it better!

For the sample directory, yes, it is certainly specified before, along with the other directories. So, I guess we should also use the same prefix variable.

@da115115
Copy link
Member

Could you apply 8fce616 to the main branch?
In my case, GitHub states that it is a commit outside of the repository, and I cannot apply it (e.g., by cherry-picking)

@LecrisUT
Copy link
Contributor Author

Could you apply 8fce616 to the main branch? In my case, GitHub states that it is a commit outside of the repository, and I cannot apply it (e.g., by cherry-picking)

Done, I've also reverted the hard-coded CMAKE_INSTALL_PREFIX in that commit: #4

@da115115
Copy link
Member

@da115115 da115115 closed this Mar 25, 2025
@LecrisUT
Copy link
Contributor Author

How did these changes go so far? Any hitches? I was looking at the CMake change proposal progress and I think these ones are resolved? Can you share me a list of packages that I need to update (you should also have access to do those updates if you have the time)?

@da115115
Copy link
Member

Thanks Cristian!

I've just upgraded SOCI to the latest version (4.1.2): https://bodhi.fedoraproject.org/updates/FEDORA-2025-796235f430
As a matter of fact, SOCI was bringing along mysql-devel, and since all my packages depend on SOCI, I had to start with it. I'll then upgrade the other packages (AirSim simulator and OpenTREP).

@da115115
Copy link
Member

da115115 commented Jun 2, 2025

All my packages depending on the MySQL client library have just been rebuilt on Fedora Rawhide. More specifically:

  1. Airline Simulator:
  1. OpenTREP:

Just for reference, the dependency graph is derived from the MetaSim's Rake build specification.

Many thanks for your valuable contribution and support throughout that migration!

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Jun 2, 2025

Thanks, I'll try to check it early this week.

Btw, I can help you setup a packit workflow for all of these packages which would build (and test) each commit of the packages against any distro available on copr. For reference it would roughly look like this project with:

  • latest/nightly: build each project with the latest main branch commits. Could be considered as experimental builds that are allowed to fail transitively
  • stable/release: build each project with the latest tagged versions
  • PR builds are also supported which would pull from either setup

The main benefit over docker containers is that it does not require to rebuild the container every time the dependencies change, and will allow you to keep up to date with Fedora rawhide. You can also pull in side-tag builds and other in progress change proposals and make upstream-downstream smoother. You can also offer these for early testers

@da115115
Copy link
Member

da115115 commented Jun 2, 2025

Btw, I can help you setup a packit workflow for all of these packages which would build (and test) each commit of the packages against any distro available on copr. For reference it would roughly look like this project with:

Yes, that would be lovely!
For the CI/CD, I used to have all the pipelines on Travis-CI (there are still a lot of them there), but with their licensing/business model changes, it has become impractical. Packit seems much better in 2025 :)

Thanks for the proposal!

Note that I cannot spend much time on it, as my daily job keeps me very busy these days. But I appreciate your help and will strive to make that migration happen.

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Jun 2, 2025

My pleasure. I will make PRs for stdair and airrac initially as proof-of-concept. One thing that would be necessary is installing packit app and iirc it will ask you the admin for a FAS account (although I have often discussed with packit folks to override it to me as the packager when the organization owner(s) don't have a FAS account). I don't remember if the app still requires a billing address (do let me know if it still reports it so I can ping the team about it), but there is no actual billing and a bogus address will also be fine.

Other than that I can work on it asynchronously when I have a few minutes of procrastination :)

@LecrisUT LecrisUT mentioned this pull request Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants