Skip to content

Now finds libaec with find_package #73

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.

Sign up for GitHub

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

Closed
Ozaq wants to merge 1 commit into from
Closed

Now finds libaec with find_package #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/ci-hpc-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ build:
env:
- ECCODES_SAMPLES_PATH=$ECCODES_DIR/share/eccodes/samples
- ECCODES_DEFINITION_PATH=$ECCODES_DIR/share/eccodes/definitions
- libaec_ROOT=$AEC_DIR
15 changes: 2 additions & 13 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

########################################################################################################################
### dependencies and options

set( PERSISTENT_NAMESPACE "eckit" CACHE INTERNAL "" ) # needed for generating .b files for persistent support
ecbuild_find_package( NAME eckit VERSION 1.28.3 REQUIRED )
ecbuild_find_package( NAME metkit VERSION 1.11.22 REQUIRED )
Expand All @@ -27,19 +26,10 @@ if (HAVE_GRIBJUMP_LOCAL_EXTRACT)

ecbuild_find_package( NAME eccodes VERSION 2.32.1 REQUIRED )

### AEC
# Override eccodes' aec with our own: we need a newer version.
# Can be removed once we rely on next eckit version
Copy link
Copy Markdown
Member

@ChrisspyB ChrisspyB Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it going to be compulsory to build eckit/eccodes with aec 1.1.1+?

Copy link
Copy Markdown
Member

@ChrisspyB ChrisspyB Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik, it is currently not compulsory to build those libraries with aec at all.

Copy link
Copy Markdown
Member Author

@Ozaq Ozaq Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this comment is about

unset(AEC_INCLUDE_DIRS CACHE)
unset(AEC_LIBRARIES CACHE)

still being in the code, this was left in for the transitionary period when eckit did not yet remove the use of FindAEC.cmake

The minimum eckit version should be hiked to the next version (current develop) to ensure eckit/eccodes/gribjump use all ecbuild_find_pacakge(libaec) (aka use libaec-config.cmake instead of FindAEC.cmake)

unset(AEC_INCLUDE_DIRS CACHE)
unset(AEC_LIBRARIES CACHE)
ecbuild_find_package( NAME AEC VERSION 1.1.1 REQUIRED )

# ecbuild_find_package's version checking does not work if the version is not specified in the package.
if (NOT AEC_VERSION)
message(FATAL_ERROR "AEC version is too old (version unspecified). Minimum supported version is 1.1.1.")
endif()
if (AEC_VERSION VERSION_LESS 1.1.1)
message(FATAL_ERROR "AEC version is too old. Minimum supported version is 1.1.1. Found version: ${AEC_VERSION}")
endif()
Comment on lines -34 to -42
Copy link
Copy Markdown
Member

@ChrisspyB ChrisspyB Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would quite like to continue failing at config time if the aec version is too old. Unless there has been an update to ecbuild, "VERSION 1.1.1 REQUIRED" has proven insufficient to ensure this.

Copy link
Copy Markdown
Member

@ChrisspyB ChrisspyB Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct solution would be for ecbuild to properly fail if it picks it only picks up an aec older than 1.1.1. I believe the issue may be due to older aec's not having a discernable version number, so ecbuild fails to validate the version and keeps marching on...

Copy link
Copy Markdown
Member Author

@Ozaq Ozaq Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please notice that we are not using the old FindAEC.cmake here any longer, but we are searching for a dependency 'libaec'. Libaec starting with 1.0.6 supplies version information in its libaec-config.cmake (I checked all distros we use). Further this is now the minimum accepted version across the stack. So this should error out reliably if any other ecbuild_find_package has picked up an earlier libaec. Internally this falls back to a find_package in config mode with modern targets and version numbers.

ecbuild_find_package( NAME libaec VERSION 1.1.1 REQUIRED )

# Optional dependency: dhskit
ecbuild_find_package( NAME dhskit VERSION 0.8.6 )
Expand All @@ -62,7 +52,6 @@ include(find_python_module)
set( gribjump_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/src ${CMAKE_CURRENT_BINARY_DIR}/src )

include_directories(
${AEC_INCLUDE_DIRS}
${gribjump_INCLUDE_DIRS}
${eckit_INCLUDE_DIRS}
)
Expand Down
61 changes: 0 additions & 61 deletions cmake/FindAEC.cmake
View file
Open in desktop

This file was deleted.

12 changes: 4 additions & 8 deletions src/gribjump/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,8 @@ if( HAVE_GRIBJUMP_LOCAL_EXTRACT )

FDBPlugin.cc
FDBPlugin.h
)

set( SERVER_LIBS fdb5 ${AEC_LIBRARIES} )

endif()
)

if (GRIBJUMP_HAVE_DHSKIT)
list( APPEND SERVER_LIBS dhskit )
endif()

ecbuild_add_library(
Expand All @@ -160,7 +154,9 @@ ecbuild_add_library(
metkit
eckit
eckit_option
${SERVER_LIBS}
$<${HAVE_GRIBJUMP_LOCAL_EXTRACT}:libaec::aec>
$<${HAVE_GRIBJUMP_LOCAL_EXTRACT}:fdb5>
$<${GRIBJUMP_HAVE_DHSKIT}:dhskit>
)

############################################################################################
Expand Down
2 changes: 1 addition & 1 deletion src/tools/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@ if ( HAVE_GRIBJUMP_LOCAL_EXTRACT)
LIBS gribjump
)

endif()
endif()
4 changes: 2 additions & 2 deletions tests/tools/callback_vs_scan.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export LD_LIBRARY_PATH="$binary/lib:$binary/lib64:$LD_LIBRARY_PATH"
export LOAD_PLUGINS="int.ecmwf.gribjump-plugin"

# LD PRELOAD AEC to prevent downstream-CI issues where eckit is linked against an incompatible version (GRIBJUMP-39).
aecpath="@AEC_LIBRARY@"
aecpath="@libaec_LIBRARY@"
if [ -e "$aecpath" ]; then
export LD_PRELOAD="$aecpath"
fi
Expand Down Expand Up @@ -181,4 +181,4 @@ for i in $(seq 1 $NROOTS); do
rm -rf ${bindir}/root${i}
done

echo "Test finished"
echo "Test finished"
Loading