Conversation
tools/get_socrates.sh resolves its optional install-path argument through portable_realpath, which rejects a path with a missing leaf (BSD realpath, so every macOS host) or a missing parent (GNU realpath). set -euo pipefail is enabled only further down the script, so the empty result travelled on and surfaced as "could not create work tree dir ''" from git clone. Creating the directory beforehand was the only way to pass a new destination. The helper now falls through to its python3 resolver whenever realpath refuses the path, so a destination that does not exist yet resolves lexically; git clone then creates it, parents included. The helper is duplicated across the nine get_*.sh scripts and every copy carries the same change, pinned by a test that compares the copies as text. get_socrates.sh additionally stops with a named error if the resolution is empty, instead of handing an empty string to git, and its usage header now lists the positional argument.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #862 +/- ##
=======================================
Coverage 93.45% 93.46%
=======================================
Files 113 113
Lines 16797 16797
Branches 2992 2992
=======================================
+ Hits 15698 15699 +1
+ Misses 1093 1092 -1
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
egpbos
marked this pull request as draft
September 9, 2026 11:44
egpbos
marked this pull request as ready for review
September 9, 2026 12:01
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
tools/get_socrates.shtakes an optional install-path positional argument (thesame interface
tools/get_agni.shdocuments), but resolved it withportable_realpath, which rejects a path that does not exist yet: BSDrealpath(so every macOS host) refuses a missing leaf, GNUrealpathamissing parent.
set -euo pipefailis enabled only further down the script, sothe empty result travelled on and surfaced much later as a confusing
from
git clone. Passing a fresh destination therefore requiredmkdir -pfirst.
Before:
After (
git clonestubbed out so the run stops at the build step):Changes:
portable_realpathnow falls through to itspython3resolver wheneverrealpathrefuses the path, so a destination that does not exist yetresolves lexically.
git clonethen creates it, intermediate directoriesincluded, so no
mkdir -pis needed anywhere.tools/get_*.shscripts that carryit, and every copy gets the same change. Only
get_socrates.shwas reachablewith a non-existent path today (
get_petsc.shalready doesmkdir -pbeforeresolving,
get_spider.shresolves an existing PETSc tree), but the copiesare kept identical so a future caller does not reintroduce the failure. A new
test compares the copies as text, in the spirit of the existing
"keep this guard in sync across the get_* scripts" convention.
get_socrates.shstops with a named error if the resolution is empty for anyother reason, instead of handing an empty string to
git clone.get_socrates.shgained a usage header listing the positional argument and--force, matchingget_agni.sh.The manual invocation is not the only trigger:
proteus get socrateswasbroken on macOS by the same defect.
get_socrates()insrc/proteus/utils/data.pyreturns early whensocrates/already exists, andotherwise passes that not-yet-existing absolute path to the script without
creating it, which is exactly the argument
portable_realpathrefused. Theroute the docs and
proteus install-alluse was unaffected, because it invokesthe script with no argument and takes the branch that never resolves an install
path.
Verified by calling
proteus.utils.data.get_socrates()withgitreplaced bya stub that logs its arguments. Against the script on main:
and with this branch:
Both runs then fail at the build step, since the stubbed clone leaves no source
tree. No change is needed in
data.pyitself.Resolution of existing paths is unchanged, symlinks included: the
realpathbranch still runs first and only its failure reaches
python3.No related issue is open for this; the report came in directly.
Validation of changes
Test configuration: macOS 26.6.2 (arm64), Python 3.12, system bash 3.2.57
with BSD
realpathat/bin/realpath.gitreplacedby a stub that reports its clone destination: the clone received an empty
destination and the stub refused it. The same run on the fixed script passes
the full absolute path, including through two levels of not-yet-existing
parents.
proteus get socratesroute (proteus.utils.data.get_socrates,wired at
cli.py:558) hits the same failure before this change and passes thecorrect destination after it, with
gitstubbed as described above.git clonecreates a nested destination (git clone <src> a/b/cwith no
a/), which is why nomkdir -pwas added.relative missing path, existing directory, and a symlinked directory (still
followed to its target).
tests/tools/test_install_scripts.py, all of which failagainst the pre-fix script and pass after it: missing-path resolution, the
cross-script identical-helper invariant, install-path resolution before the
checkout exists,
--forcewith and without a path, and the empty-resolutionerror path.
portable_realpath;it now lifts the shipped definition out of
get_socrates.sh, so the casesrun against the text that ships.
pytest tests/tools/: 226 passed.pytest -m "unit and not skip and not slow and not integration" --ignore=tests/examples: 2965 passed, 45 skipped, 23 failed. The same 23fail on this branch without this change (all in
tests/interior_energetics/test_aragog*.py, unrelated to the shell scripts).ruff check src/ tests/andruff format --check src/ tests/: clean.bash tools/validate_test_structure.sh: complete.python tools/check_test_quality.py --checkreports no violations in thetouched file (its pre-existing regressions are in other files).
bash -non alltools/get_*.sh: clean.shellcheckis not installed onthis host, so the scripts were not linted with it.
Not verified: the behaviour of GNU
realpathon a missing parent was notexercised on a Linux host, only its documented default. The fix does not depend
on which of the two refuses, since any refusal now falls through to
python3.A full SOCRATES clone and build with a custom install path was not run; the
clone step was stubbed.
Checklist
installation docs do not document the positional argument)
changed;
python3was already the helper's fallback)