Skip to content

Fix/conditional dependency build#250

Merged
krystophny merged 2 commits intomainfrom
fix/conditional-dependency-build
Jan 13, 2026
Merged

Fix/conditional dependency build#250
krystophny merged 2 commits intomainfrom
fix/conditional-dependency-build

Conversation

@krystophny
Copy link
Copy Markdown
Member

@krystophny krystophny commented Jan 13, 2026

PR Type

Bug fix


Description

  • Only build external dependencies that are actually needed

    • Wrap build_hdf5(), build_netcdf_c(), build_netcdf_fortran(), and build_fftw() calls with conditional checks for NEED_BUILD_* variables
    • Prevents CMake errors when system libraries are available but only some dependencies need building
  • Add robustness guards to HDF5 imported target creation

    • Wrap each add_library() call with if(NOT TARGET) checks to prevent duplicate target errors
  • Add DOWNLOAD_EXTRACT_TIMESTAMP to external project downloads

    • Silence CMP0135 policy warnings in HDF5 and FFTW ExternalProject_Add calls

Diagram Walkthrough

flowchart LR
  A["build_all_external_dependencies"] --> B["Check NEED_BUILD_HDF5"]
  B -->|TRUE| C["build_hdf5"]
  B -->|FALSE| D["Skip HDF5"]
  A --> E["Check NEED_BUILD_NETCDF"]
  E -->|TRUE| F["build_netcdf_c + build_netcdf_fortran"]
  E -->|FALSE| G["Skip NetCDF"]
  A --> H["Check NEED_BUILD_FFTW"]
  H -->|TRUE| I["build_fftw"]
  H -->|FALSE| J["Skip FFTW"]
  C --> K["if NOT TARGET guards on add_library"]
  F --> K
  I --> K
Loading

File Walkthrough

Relevant files
Bug fix
BuildExternalDependencies.cmake
Conditional dependency building with target creation guards

cmake/BuildExternalDependencies.cmake

  • Wrap build_hdf5(), build_netcdf_c(), build_netcdf_fortran(), and
    build_fftw() function calls with conditional checks for NEED_BUILD_*
    variables to only build dependencies that are actually needed
  • Add if(NOT TARGET) guards around all six HDF5 imported target creation
    calls to prevent duplicate target errors when find_package already
    created them
  • Add DOWNLOAD_EXTRACT_TIMESTAMP TRUE parameter to HDF5 and FFTW
    ExternalProject_Add calls to silence CMP0135 policy warnings
  • Update function comment to clarify that only needed dependencies are
    built
+32/-11 

krystophny and others added 2 commits January 12, 2026 22:36
Previously, build_all_external_dependencies() unconditionally called
all build functions (build_hdf5, build_netcdf_c, etc.) regardless of
which dependencies were actually needed. This caused CMake errors like
"add_library cannot create imported target 'hdf5::hdf5' because another
target with the same name already exists" when system HDF5/NetCDF were
usable but only FFTW needed to be built from source.

Now each build function is only called if its corresponding
NEED_BUILD_* variable is TRUE.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add DOWNLOAD_EXTRACT_TIMESTAMP TRUE to HDF5 and FFTW ExternalProject_Add
  calls to silence CMP0135 policy warning
- Add if(NOT TARGET) guards when creating HDF5 imported targets for extra
  robustness in case find_package already created them

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Decouple NetCDF C/Fortran build flags

Replace the single NEED_BUILD_NETCDF flag with separate NEED_BUILD_NETCDF_C and
NEED_BUILD_NETCDF_FORTRAN flags for more granular build control.

cmake/BuildExternalDependencies.cmake [279-282]

-if(NEED_BUILD_NETCDF)
+if(NEED_BUILD_NETCDF_C)
     build_netcdf_c()
+endif()
+if(NEED_BUILD_NETCDF_FORTRAN)
     build_netcdf_fortran()
 endif()
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This suggestion improves the PR's logic by decoupling the NetCDF C and Fortran library builds, which aligns with the PR's goal of only building necessary dependencies and provides more granular control.

Medium
General
Use a loop to guard imported targets

Refactor the repetitive if(NOT TARGET) blocks for HDF5 imported targets into a
single foreach loop to reduce code duplication.

cmake/BuildExternalDependencies.cmake [73-90]

-if(NOT TARGET hdf5::hdf5)
-    add_library(hdf5::hdf5 STATIC IMPORTED GLOBAL)
-endif()
-if(NOT TARGET hdf5::hdf5_hl)
-    add_library(hdf5::hdf5_hl STATIC IMPORTED GLOBAL)
-endif()
-if(NOT TARGET hdf5::hdf5_f90cstub)
-    add_library(hdf5::hdf5_f90cstub STATIC IMPORTED GLOBAL)
-endif()
-if(NOT TARGET hdf5::hdf5_hl_f90cstub)
-    add_library(hdf5::hdf5_hl_f90cstub STATIC IMPORTED GLOBAL)
-endif()
-if(NOT TARGET hdf5::hdf5_fortran)
-    add_library(hdf5::hdf5_fortran STATIC IMPORTED GLOBAL)
-endif()
-if(NOT TARGET hdf5::hdf5_hl_fortran)
-    add_library(hdf5::hdf5_hl_fortran STATIC IMPORTED GLOBAL)
-endif()
+foreach(tgt IN ITEMS
+    hdf5::hdf5
+    hdf5::hdf5_hl
+    hdf5::hdf5_f90cstub
+    hdf5::hdf5_hl_f90cstub
+    hdf5::hdf5_fortran
+    hdf5::hdf5_hl_fortran
+)
+  if(NOT TARGET ${tgt})
+    add_library(${tgt} STATIC IMPORTED GLOBAL)
+  endif()
+endforeach()
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies repetitive code introduced in the PR and proposes using a foreach loop to refactor it, which improves code readability and maintainability.

Low
  • More

@krystophny krystophny merged commit 4abdcf7 into main Jan 13, 2026
2 checks passed
@krystophny krystophny deleted the fix/conditional-dependency-build branch January 13, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant