draft: 255 organize odeint as stand alone library by GeorgGrassler · Pull Request #41 · itpplasma/rabe

GeorgGrassler · 2026-03-05T15:55:51Z

sister pull request to itpplasma/libneo#256

The intend of this change is to only configure and build subpackages of libneo. If libneo was already fetched, it is only fetched once for all its subpackes. This approach however fails and makes also no sense, if the full package libneo is already imported/build anyway in the project (see build failure in CI).

CMake Error at build/_deps/libneo-src/src/odeint/CMakeLists.txt:1 (add_library):
  add_library cannot create target "odeint" because another target with the
  same name already exists.  The existing target is a static library created
  in source directory
  "/home/runner/work/rabe/rabe/build/_deps/libneo-src/src/odeint".  See
  documentation for policy CMP0002 for more details.


-- Found OpenMP_Fortran: -fopenmp (found version "4.5")
CMake Error at build/_deps/libneo-src/src/odeint/CMakeLists.txt:15 (add_library):
  add_library cannot create ALIAS target "LIBNEO::odeint" because another
  target with the same name already exists.

This pr will stay open until e.g. VMEC spline functionalities in libneo have been also refactored into an independant subpackage. As long as rabe needs to import libneo as a whole, subpackage management is not applicable.

qodo-code-review · 2026-03-05T15:56:04Z

Review Summary by Qodo

Organize odeint as standalone sublibrary with CMake support

✨ Enhancement

Walkthroughs

Description

• Add support for subdirectory specification in CMake dependency resolution
• Enable odeint library as standalone sublibrary within libneo
• Modify build paths to support nested library structures
• Link odeint library to rabe_lib as private dependency

Diagram

flowchart LR
  A["CMake Util Functions"] -->|"Add SUBDIR parameter"| B["find_or_fetch Function"]
  B -->|"Pass SUBDIR to fetch"| C["fetch Function"]
  C -->|"Append SUBDIR to paths"| D["Nested Build Paths"]
  E["src/CMakeLists.txt"] -->|"Include Util module"| F["find_or_fetch libneo"]
  F -->|"Fetch with SUBDIR src/odeint"| G["odeint Library"]
  G -->|"Link as PRIVATE"| H["rabe_lib Target"]

File Changes

1. cmake/Util.cmake ✨ Enhancement +8/-5

Add subdirectory parameter support to CMake utilities

• Add SUBDIR optional parameter to find_or_fetch() function using cmake_parse_arguments()
• Append OPTIONAL_SUBDIR to source directory paths in both local and fetched dependency cases
• Update fetch() function to accept and propagate SUBDIR parameter
• Modify binary directory paths to include subdirectory structure for nested builds

cmake/Util.cmake

2. src/CMakeLists.txt ✨ Enhancement +4/-0

Integrate odeint as standalone sublibrary dependency

• Include the Util CMake module for dependency management
• Fetch libneo with SUBDIR src/odeint to organize odeint as standalone sublibrary
• Link odeint library to rabe_lib as a PRIVATE dependency

src/CMakeLists.txt

qodo-code-review · 2026-03-05T15:56:05Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Malformed fetch() call 🐞 Bug ✓ Correctness

Description

find_or_fetch() always calls fetch(... SUBDIR ${OPTIONAL_SUBDIR}); when the caller did not
provide SUBDIR, this can become fetch(<dep> SOURCE_DIR SUBDIR) (missing value), potentially
breaking argument parsing and causing configure failures. This is a backward-incompatible regression
for existing callers like find_or_fetch(quadpack).

Code

cmake/Util.cmake[10]

+        fetch(${DEPENDENCY} SOURCE_DIR SUBDIR ${OPTIONAL_SUBDIR})

Evidence

find_or_fetch() unconditionally appends the SUBDIR keyword when calling fetch(). Existing call
sites invoke find_or_fetch() with no SUBDIR (e.g., quadpack), so the expansion can produce a
fetch() invocation where SUBDIR has no value, which can mis-parse or error during
cmake_parse_arguments() inside fetch().

cmake/Util.cmake[3-16]
src/utils/CMakeLists.txt[9-12]
cmake/Util.cmake[19-22]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`find_or_fetch()` currently always calls `fetch(${DEPENDENCY} SOURCE_DIR SUBDIR ${OPTIONAL_SUBDIR})`. When no SUBDIR was provided by the caller, this can turn into `fetch(&lt;dep&gt; SOURCE_DIR SUBDIR)` which may break argument parsing and regress existing call sites (e.g. quadpack).

## Issue Context
The repo already uses `find_or_fetch(quadpack)` without any SUBDIR. This needs to keep working.

## Fix Focus Areas
- cmake/Util.cmake[3-16]
- cmake/Util.cmake[19-39]
- src/utils/CMakeLists.txt[9-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. SUBDIR path not validated 🐞 Bug ⛯ Reliability

Description

With the new SUBDIR feature, find_or_fetch() checks only that $ENV{CODE}/<dep> exists, but then
uses $ENV{CODE}/<dep>/<SUBDIR> as SOURCE_DIR without validating it (and fetch() similarly
appends SUBDIR). A typo/missing subdir will cause a hard add_subdirectory() failure with a less
actionable error message.

Code

cmake/Util.cmake[R6-16]

    if(DEFINED ENV{CODE} AND EXISTS $ENV{CODE}/${DEPENDENCY})
-        set(SOURCE_DIR $ENV{CODE}/${DEPENDENCY})
-        message(STATUS "Using ${DEPENDENCY} in $ENV{CODE}/${DEPENDENCY}")
+        set(SOURCE_DIR $ENV{CODE}/${DEPENDENCY}/${OPTIONAL_SUBDIR})
+        message(STATUS "Using ${DEPENDENCY}/${OPTIONAL_SUBDIR} in $ENV{CODE}/${DEPENDENCY}")
    else()
-        fetch(${DEPENDENCY} SOURCE_DIR)
+        fetch(${DEPENDENCY} SOURCE_DIR SUBDIR ${OPTIONAL_SUBDIR})
    endif()

    add_subdirectory(${SOURCE_DIR}
-        ${CMAKE_CURRENT_BINARY_DIR}/${DEPENDENCY}
+        ${CMAKE_CURRENT_BINARY_DIR}/${DEPENDENCY}/${OPTIONAL_SUBDIR}
        EXCLUDE_FROM_ALL
    )

Evidence

The existence check for local deps does not include the requested SUBDIR, but add_subdirectory()
is invoked on the SUBDIR-expanded path. The PR introduces a new call site using SUBDIR src/odeint,
so a missing/incorrect subdir (either in a local $ENV{CODE} checkout or in the fetched content)
will fail at configure time without a targeted diagnostic.

cmake/Util.cmake[5-16]
src/CMakeLists.txt[26-28]
cmake/Util.cmake[35-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When SUBDIR is used, `find_or_fetch()` and `fetch()` build a subdirectory path but do not validate it. If the subdir is missing/typo, `add_subdirectory()` will fail with a generic error.

## Issue Context
This PR adds a new `find_or_fetch(libneo SUBDIR src/odeint)` call site. Both local `$ENV{CODE}` usage and FetchContent usage should validate `SOURCE_DIR` (and ideally ensure it contains a `CMakeLists.txt`).

## Fix Focus Areas
- cmake/Util.cmake[3-16]
- cmake/Util.cmake[19-39]
- src/CMakeLists.txt[26-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

qodo-code-review · 2026-03-05T16:00:03Z

+        message(STATUS "Using ${DEPENDENCY}/${OPTIONAL_SUBDIR} in $ENV{CODE}/${DEPENDENCY}")
    else()
-        fetch(${DEPENDENCY} SOURCE_DIR)
+        fetch(${DEPENDENCY} SOURCE_DIR SUBDIR ${OPTIONAL_SUBDIR})


1. Malformed fetch() call 🐞 Bug ✓ Correctness

find_or_fetch() always calls fetch(... SUBDIR ${OPTIONAL_SUBDIR}); when the caller did not provide SUBDIR, this can become fetch(<dep> SOURCE_DIR SUBDIR) (missing value), potentially breaking argument parsing and causing configure failures. This is a backward-incompatible regression for existing callers like find_or_fetch(quadpack).

Agent Prompt

## Issue description `find_or_fetch()` currently always calls `fetch(${DEPENDENCY} SOURCE_DIR SUBDIR ${OPTIONAL_SUBDIR})`. When no SUBDIR was provided by the caller, this can turn into `fetch(<dep> SOURCE_DIR SUBDIR)` which may break argument parsing and regress existing call sites (e.g. quadpack). ## Issue Context The repo already uses `find_or_fetch(quadpack)` without any SUBDIR. This needs to keep working. ## Fix Focus Areas - cmake/Util.cmake[3-16] - cmake/Util.cmake[19-39] - src/utils/CMakeLists.txt[9-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-04-06T11:49:04Z

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: test
Failed stage: Build project [❌]
Failed test name: ""
Failure summary: The action failed during CMake configuration while fetching/building the dependency `libneo`. - CMake hit a fatal error in `FetchContent.cmake` stating `Content libneo already populated` in `/home/runner/work/rabe/rabe/build/_deps/libneo-src`, which indicates `FetchContent_Populate(libneo)` (called via `cmake/Util.cmake:33` -> `cmake/Util.cmake:10` -> `src/CMakeLists.txt:29`) was invoked after `libneo` had already been populated in the same build directory. - Because of this `FetchContent` double-population, configuration was aborted (`Configuring incomplete, errors occurred!`), and `make` failed with `Error 1`, causing the job to exit with code `2`.
Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 913: -- Python numpy include dir: /opt/hostedtoolcache/Python/3.12.13/x64/lib/python3.12/site-packages/numpy/_core/include 914: -- Python binary output dir: /home/runner/work/rabe/rabe/build/src/vmec/libneo 915: -- Branch 255-organize-odeint-as-stand-alone-library exists: 916: -- Fetch libneo branch main from https://github.com/itpplasma/libneo.git 917: CMake Warning (dev) at /usr/local/share/cmake-3.31/Modules/FetchContent.cmake:1953 (message): 918: Calling FetchContent_Populate(libneo) is deprecated, call 919: FetchContent_MakeAvailable(libneo) instead. Policy CMP0169 can be set to 920: OLD to allow FetchContent_Populate(libneo) to be called directly for now, 921: but the ability to call it with declared details will be removed completely 922: in a future version. 923: Call Stack (most recent call first): 924: cmake/Util.cmake:33 (FetchContent_Populate) 925: cmake/Util.cmake:10 (fetch) 926: src/CMakeLists.txt:29 (find_or_fetch) 927: This warning is for project developers. Use -Wno-dev to suppress it. 928: CMake Error at /usr/local/share/cmake-3.31/Modules/FetchContent.cmake:2028 (message): 929: Content libneo already populated in 930: /home/runner/work/rabe/rabe/build/_deps/libneo-src 931: Call Stack (most recent call first): 932: /usr/local/share/cmake-3.31/Modules/FetchContent.cmake:1978:EVAL:1 (__FetchContent_Populate) 933: /usr/local/share/cmake-3.31/Modules/FetchContent.cmake:1978 (cmake_language) 934: cmake/Util.cmake:33 (FetchContent_Populate) 935: cmake/Util.cmake:10 (fetch) 936: src/CMakeLists.txt:29 (find_or_fetch) 937: -- Configuring incomplete, errors occurred! 938: make: *** [Makefile:8: build/CMakeCache.txt] Error 1 939: ##[error]Process completed with exit code 2. 940: Post job cleanup.

…ze-odeint-as-stand-alone-library

krystophny · 2026-07-02T12:34:08Z

Merged feat/precession, dropped the second libneo fetch in favor of linking LIBNEO::odeint from the already included libneo, and wrapped the over-length reference lines in test_trace_till_bounce.f90. Build and 38/39 tests now pass in CI; TestPrecessionAnalytic still fails with identical values locally and in CI (flux_mode 2/3 off by factors ~3.2/~2.3 against the analytic expectation, numerical mode 3 at 1.45e-6 vs 1e-6 tolerance), so that comparison needs a look from the derivation side.

qodo-code-review Bot reviewed Mar 5, 2026

View reviewed changes

GeorgGrassler changed the base branch from main to feat/precession April 6, 2026 11:41

GeorgGrassler force-pushed the 255-organize-odeint-as-stand-alone-library branch from 7abb2dc to 16feba3 Compare April 6, 2026 11:47

GeorgGrassler added 3 commits April 6, 2026 14:28

include: sublibrary of libneo

f258288

change: build path

ace99e8

guard: against repeated fetch

38b898f

GeorgGrassler force-pushed the 255-organize-odeint-as-stand-alone-library branch from 8493b6d to 38b898f Compare April 6, 2026 12:28

GeorgGrassler marked this pull request as draft April 6, 2026 12:32

GeorgGrassler mentioned this pull request Apr 6, 2026

build: with only the necessary libneo dependencies #55

Closed

GeorgGrassler changed the title ~~255 organize odeint as stand alone library~~ draft: 255 organize odeint as stand alone library Apr 6, 2026

krystophny added 3 commits July 2, 2026 14:13

Merge remote-tracking branch 'origin/feat/precession' into 255-organi…

d9467b3

…ze-odeint-as-stand-alone-library

fix: link odeint from already fetched libneo

8092be3

fix: wrap reference values exceeding line limit

b6564e8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

draft: 255 organize odeint as stand alone library - #41

draft: 255 organize odeint as stand alone library#41
GeorgGrassler wants to merge 6 commits into
feat/precessionfrom
255-organize-odeint-as-stand-alone-library

GeorgGrassler commented Mar 5, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Mar 5, 2026

Uh oh!

qodo-code-review Bot commented Mar 5, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot Mar 5, 2026

Uh oh!

qodo-code-review Bot commented Apr 6, 2026

Uh oh!

krystophny commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

GeorgGrassler commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Mar 5, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review Bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-code-review Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Apr 6, 2026

CI Feedback 🧐

Uh oh!

krystophny commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GeorgGrassler commented Mar 5, 2026 •

edited

Loading

qodo-code-review Bot commented Mar 5, 2026 •

edited

Loading