Skip to content

WIP: small but crucial fixes to ecbuild #139

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

Draft
mcocdawc wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

WIP: small but crucial fixes to ecbuild #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d0a2d4
small but crucial fixes to ecbuild
mcocdawc Apr 10, 2026
b465fda
Merge branch 'develop' of github.com:ecmwf/ecbuild into fix_interface…
mcocdawc Apr 10, 2026
b192f8a
ci: extract shared infrastructure into composite actions
mcocdawc Apr 13, 2026
d7f4c08
ci(use-ecbuild): clone fresh source instead of using action_path
mcocdawc Apr 13, 2026
b3b7e24
ci(use-ecbuild): move action to downstream-ci repo
mcocdawc Apr 13, 2026
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
4 changes: 2 additions & 2 deletions cmake/ecbuild_target_fortran_module_directory.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# granted to it by virtue of its status as an intergovernmental organisation
# nor does it submit to any jurisdiction.

macro( ecbuild_target_fortran_module_directory )
function( ecbuild_target_fortran_module_directory )
set( options NO_MODULE_DIRECTORY )
set( single_value_args TARGET MODULE_DIRECTORY INSTALL_MODULE_DIRECTORY )
set( multi_value_args "" )
Expand Down Expand Up @@ -35,4 +35,4 @@ macro( ecbuild_target_fortran_module_directory )
endif()
endif()

endmacro()
endfunction()
7 changes: 6 additions & 1 deletion cmake/project-config.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ if(EXISTS ${@PROJECT_NAME@_CMAKE_DIR}/@CONF_IMPORT_FILE@)
endif()

### insert definitions for IMPORTED targets
if(NOT @PROJECT_NAME@_BINARY_DIR)
# Guard: for install-tree exports (@PROJECT_NAME@_IS_BUILD_DIR_EXPORT=OFF) we must
# always load the targets file. For build-tree exports the targets are already
# defined by the build system, so we only skip when we are genuinely inside that
# build (i.e. the variable was set by ecbuild's own project() hook, not by an
# unrelated project that happens to share the same name).
if(NOT (@PROJECT_NAME@_IS_BUILD_DIR_EXPORT AND @PROJECT_NAME@_BINARY_DIR))
Comment on lines +45 to +49
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The new guard/comment says we only skip loading targets when we are “genuinely inside” the exporting build (i.e. not an unrelated project with the same name). However the condition only checks @PROJECT_NAME@_IS_BUILD_DIR_EXPORT and that @PROJECT_NAME@_BINARY_DIR is non-empty; any unrelated project named @PROJECT_NAME@ will also define @PROJECT_NAME@_BINARY_DIR, so for build-tree exports this would still skip loading the targets file. Consider either (a) tightening the condition to verify @PROJECT_NAME@_BINARY_DIR actually matches this export’s build tree (e.g., compare against @PROJECT_NAME@_BASE_DIR / PACKAGE_PREFIX_DIR), or (b) adjusting the comment to match the actual behavior.

Suggested change
# always load the targets file. For build-tree exports the targets are already
# defined by the build system, so we only skip when we are genuinely inside that
# build (i.e. the variable was set by ecbuild's own project() hook, not by an
# unrelated project that happens to share the same name).
if(NOT (@PROJECT_NAME@_IS_BUILD_DIR_EXPORT AND @PROJECT_NAME@_BINARY_DIR))
# always load the targets file. For build-tree exports the targets are already
# defined by the build system, so we only skip when we are genuinely inside that
# build tree, i.e. when ecbuild's own project() hook has set
# @PROJECT_NAME@_BINARY_DIR to the same location as this export's base directory.
set(_@PROJECT_NAME@_INSIDE_EXPORTING_BUILD FALSE)
if(@PROJECT_NAME@_IS_BUILD_DIR_EXPORT AND DEFINED @PROJECT_NAME@_BINARY_DIR AND NOT "@PROJECT_NAME@_BINARY_DIR" STREQUAL "")
get_filename_component(_@PROJECT_NAME@_EXPORT_BASE_DIR "${@PROJECT_NAME@_BASE_DIR}" REALPATH)
get_filename_component(_@PROJECT_NAME@_CURRENT_BINARY_DIR "${@PROJECT_NAME@_BINARY_DIR}" REALPATH)
if(_@PROJECT_NAME@_EXPORT_BASE_DIR STREQUAL _@PROJECT_NAME@_CURRENT_BINARY_DIR)
set(_@PROJECT_NAME@_INSIDE_EXPORTING_BUILD TRUE)
endif()
endif()
if(NOT _@PROJECT_NAME@_INSIDE_EXPORTING_BUILD)

Copilot uses AI. Check for mistakes.
find_file(@PROJECT_NAME@_TARGETS_FILE
NAMES @PROJECT_NAME@-targets.cmake
HINTS @PACKAGE_TARGETS_DIRS@
Expand Down
Loading