Relocate infra to $CODE/infra; keep $CODE as the workspace root#54
Relocate infra to $CODE/infra; keep $CODE as the workspace root#54krystophny wants to merge 8 commits into
Conversation
The repository was the workspace root, so its .git tracked every sibling code checkout as untracked noise and carried two accidental gitlinks (SIMPLE, libneo) with no .gitmodules. Make it the infra directory of a workspace. activate.sh, activate.fish and activate export INFRA (this repo) and derive CODE as its parent. Infra-owned paths (scripts, .venv, external, modules, images, local, lib, bin) move to $INFRA. $CODE stays the workspace root, so the codes keep resolving dependencies as $CODE/<name> via find_or_fetch. Sibling references ($CODE/libneo/build) and workspace navigation (cdcode, vscode) stay on $CODE; set_branch reads the infra repo via $INFRA. Drop the accidental SIMPLE and libneo gitlinks. Update setup.sh, the devcontainer build and postCreateCommand.sh, the release CI in main.yml, the STELLOPT and VMEC config paths, and the README to the new layout.
main.yml downloaded a pinned release tarball whose frozen activate.sh predates the infra relocation, so $INFRA was unset and $INFRA/scripts/checkout_branch.sh resolved to /scripts/... (exit 127). Check out the branch and rsync its sources over the tarball, keeping the release's prebuilt .venv and external, and relocate the venv shebangs to the new path.
activate.sh is meant for interactive shells; its add_to_path/add_to_library_path helpers return the guard status, so under the runner's bash -e the first absent dir ($INFRA/local/bin) aborted the step before the venv activated. Wrap the source in set +e/set -e so activation completes while build commands stay strict.
The codes read prebuilt third-party libs from $CODE/external (e.g. NEO-2's build links $CODE/external/fgsl-1.6.0/.libs/libfgsl.a). external is workspace- shared like the code checkouts, not infra-private, so revert it from $INFRA to $CODE and stop tracking it in the infra repo. setup.sh creates it; CI moves the release's external to the workspace; CODE_ROOT (STELLOPT make configs) points at $CODE. setup.yml release packaging flagged for tag-time validation.
9be1ed9 to
a14b19c
Compare
main.yml built every downstream code on each push to infra and never passed (0 of 94 runs): it built the unpinned tips of six repos in one sequential job against a frozen release tarball. Remove it. Add docs/development-model.md (proposed): workspace layout, the self-describing dependency graph, the per-code + release-time reverse-dependency testing model, the <DEP>_BRANCH convention, the release-branch flow, and YY.MINOR.PATCH versioning. The genuine external -> $CODE fix from earlier commits stays; the umbrella build it was chasing is gone.
docs/templates/downstream-integration.yml: per-consumer reverse-dep validation, dispatched with a libneo ref, no commit. docs/templates/upstream-release.yml: libneo release-branch + gate + bump-PR skeleton (untested, TODOs marked). Referenced by the development-model design note.
Codes track the libneo release branch by default (committed LIBNEO_RELEASE); tags are reproducibility snapshots. Rename the override to LIBNEO_REF (branch, tag or commit), validated against the remote. Drop the $CODE local-path shortcut: first-party deps are always fetched at the resolved ref. The release gate dispatches each downstream's existing CI and opens auto-merge bump PRs to the new release branch; templates reflect reuse of existing CI and point at the canonical libneo release.yml.
There was a problem hiding this comment.
Review verdict: Request changes
Summary: This PR relocates the infra repository from the workspace root ($CODE) to a subdirectory ($CODE/infra), keeping $CODE as the workspace root so codes continue to resolve dependencies as $CODE/<name>. The change is mostly mechanical: ~30 files are updated to distinguish infra-private paths ($INFRA: scripts, .venv, modules) from workspace-level paths ($CODE: external libs, code checkouts). Two accidental gitlinks (SIMPLE, libneo) are dropped, and the umbrella CI (main.yml, 0/94 green) is retired in favor of a proposed per-code CI model documented in docs/development-model.md. The design is sound and the $CODE/$INFRA split is consistently applied across shell scripts, Dockerfiles, and the devcontainer flow. However, the fish activation script has a regression that breaks CODE resolution on older fish, and one comment actively contradicts its code.
Findings:
- [blocker]
activate.fish:20—set -gx CODE (path resolve $INFRA/..)usespath resolveunconditionally, but lines 9-19 handle three cases forINFRAwherepathmay not exist (fish < 3.5, nopathbuiltin). On those systemsINFRAis set viarealpathor acd/pwdfallback, butCODEtriespath resolve, which fails — the command substitution returns an empty string, soCODEis set to"". Every subsequent$CODEreference breaks:add_to_library_path_fish $CODE/libneo/buildis skipped (dir doesn't exist),CMAKE_INCLUDE_PATHgets a dangling$CODE/external/triangle,alias cdcode="cd $CODE"cds to$HOME. The old code onmainsetCODEthrough the samepath/realpath/cdfallback asINFRA, so this is a regression. Fix: use the same fallback forCODE, e.g.:
if type -q path
set -gx CODE (path resolve $INFRA/..)
else if type -q realpath
set -gx CODE (realpath $INFRA/..)
else
set -gx CODE (cd $INFRA/..; pwd)
end-
[major]
scripts/setup/stellopt.sh:5— Comment says "That tree lives in the infra repo, so point it at $INFRA" but the code on line 6 setsCODE_ROOT="$CODE". The external tree is at$CODE/external(workspace), not in the infra repo ($INFRA/externaldoesn't exist). The comment contradicts the code: the code is correct (STELLOPT is cloned into$CODE/external/STELLOPTand reads$(CODE_ROOT)/external/...), but a future developer following the comment would "fix" it to$INFRAand break STELLOPT. Fix: update the comment to say the tree lives in the workspace at$CODE/external. -
[minor]
.github/workflows/setup.yml:84— The TODO says "main.yml still consumes the pinned old release" butmain.ymlis deleted in this very PR (the umbrella CI being retired). The reference is stale. Fix: drop themain.ymlclause or reword to "the pinned old release is still the latest tag; validate on a real tag before cutting the next one." -
[minor] No CI —
gh pr checksreports no checks on this branch (confirmed:check-runsAPI returnstotal_count: 0). The deletedmain.ymlnever passed (0/94 runs, per the PR body), andsetup.ymlruns only on version tags — it is not exercised by PR CI. Thesetup.ymlchanges (building external deps in the workspace parent, copying into the tarball) are explicitly marked unvalidated ("Validate on a real tag before cutting the next release"). The PR body claims local verification of variable resolution and venv activation, but no automated CI exercises the changed activation, setup, or devcontainer paths. This is a process risk, not a code defect, but it means the changed paths are unproven beyond manual testing.
Verdict: request_changes — the activate.fish CODE regression (finding 1) breaks fish activation on any system without the path builtin, a configuration the script explicitly supports via the INFRA fallback. The fix is a one-liner. Finding 2 should also be fixed before merge to avoid a misleading comment that could cause a future breakage.
What
Make this repository the
infradirectory of a workspace instead of theworkspace root, and retire the umbrella integration CI that never passed.
activate.sh/.fishexportINFRA=$CODE/infraand deriveCODEas itsparent. Infra-private paths (
scripts,.venv,modules) move to$INFRA.$CODEstays the workspace root, so the codes keep resolving dependencies as$CODE/<name>(find_or_fetch) and prebuilt libraries as$CODE/external.SIMPLE,libneo) are dropped.main.ymlis removed. It built the unpinned tips of six code repos in onesequential job and never went green (0 of 94 runs). The replacement model is
in
docs/development-model.md: per-code CI at the source plus release-timereverse-dependency validation owned by the upstream.
Migration for users
Move your checkout one level down and repoint the activation line:
Verification
New activation resolves correctly. Simulated workspace with infra at
<ws>/infraand a
libneo/sibling:Before/after on the dependency-resolution contract, with infra at
<ws>/infra:Verified locally on macOS: variable resolution, venv activation, PATH and
library-path entries,
set_branch. CI is no longer the umbrella build; thatworkflow is removed rather than fixed.
Note
docs/development-model.mdis proposed, not settled. It documents the layout,the
<DEP>_BRANCHconvention, the release-branch flow, andYY.MINOR.PATCHversioning for group discussion.
setup.yml(the release-tarball builder) isleft in place but loses its only consumer with
main.ymlgone, so it needs aseparate decision.