Skip to content

Guard against re-fetching an ecosystem dep that's already the top-level project - #79

Merged
ryanmrichard merged 1 commit into
masterfrom
fix/self-reference-guard
Aug 12, 2026
Merged

Guard against re-fetching an ecosystem dep that's already the top-level project#79
ryanmrichard merged 1 commit into
masterfrom
fix/self-reference-guard

Conversation

@ryanmrichard

Copy link
Copy Markdown
Member

Summary

  • `nwx_ecosystem_dependency()` only skipped re-fetching a dependency when the `nwx::` alias target already existed (the case where the dep was resolved via `find_package()` against an installed wheel).
  • It missed the case where `` is already a bare target because this repo is that dependency's top-level project — built directly via `nwx_library()`, which never defines the `nwx::` alias — and is also, transitively, its own ecosystem dependency. E.g. SCF built with `-DINTEGRATION_TESTING=ON` pulls in the `nwchemex` superproject, which itself depends on `scf`. FetchContent would then re-fetch and redefine the same target, and CMake would fail:
    add_library cannot create target "scf" because another target with the
    same name already exists.
    
    This broke `test_cmake_build` on every platform for NWChemEx/SCF#70 — unrelated to that PR's actual (and successful) macOS pybind11 segfault fix.
  • Fix: also short-circuit on `TARGET ${ned_name}` (the bare name), same as the existing `nwx::${ned_name}` branch.

This affects every ecosystem repo that routes through this shared macro (`chemcache`, `chemist`, `integrals`, `parallelzone`, `pluginplay`, `nux`, `nwchemex`, `scf`, `simde`, `tensorwrapper`, `utilities`) whenever it's both a dependency of another ecosystem repo and built with `INTEGRATION_TESTING=ON`.

Test plan

  • Minimal standalone CMake repro mirroring `get_dependencies.cmake`'s `FetchContent_MakeAvailable` call:
    • With a pre-existing bare target and the old guard: falls through to `FetchContent`, fails trying to clone a bogus URL (reproduces the bug).
    • With the fix: short-circuits before ever reaching `FetchContent` — no network attempt, clean configure.
    • Regression check: with no pre-existing target (normal, first-time ecosystem fetch), the new branch does not fire — falls through to `FetchContent` exactly as before.
  • Once merged, SCF PR Fix broken FetchContent: point chemcache at master #70's `test_cmake_build` should pass on all legs (it FetchContents NWXCMake unpinned from `master`, so this picks up automatically).

🤖 Generated with Claude Code

…el project

nwx_ecosystem_dependency() only short-circuited on the nwx::<name> alias
target (set when a dep is resolved via find_package() against an installed
wheel). It missed the case where <name> is already a bare target because
this repo IS <name>'s top-level project (built directly via nwx_library(),
which never defines the nwx:: alias) and is also, transitively, its own
ecosystem dependency -- e.g. SCF built with -DINTEGRATION_TESTING=ON pulls
in the nwchemex superproject, which itself depends on scf. FetchContent
would then redefine the same target and CMake would fail with a
duplicate-target error ("add_library cannot create target 'scf' because
another target with the same name already exists").

Verified with a minimal standalone repro mirroring get_dependencies.cmake's
FetchContent_MakeAvailable call: before this fix, a pre-existing bare target
still fell through to FetchContent and failed trying to clone a bogus URL;
after this fix, it short-circuits before ever reaching FetchContent. A
second repro with no pre-existing target confirms normal, non-self-referencing
fetches are unaffected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ryanmrichard
ryanmrichard merged commit d7330f9 into master Aug 12, 2026
8 checks passed
@ryanmrichard
ryanmrichard deleted the fix/self-reference-guard branch August 12, 2026 03:11
@github-actions

Copy link
Copy Markdown

🚀 [bumpr] Bumped!
New version:v0.0.19
Changes:v0.0.18...v0.0.19

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.

1 participant