Skip to content

Resolve a get_* install path that does not exist yet - #862

Open
egpbos wants to merge 2 commits into
mainfrom
fix/portable-realpath-missing-leaf
Open

egpbos wants to merge 2 commits into
mainfrom
fix/portable-realpath-missing-leaf

Conversation

@egpbos

@egpbos egpbos commented Sep 9, 2026

Copy link
Copy Markdown
Member

Description

tools/get_socrates.sh takes an optional install-path positional argument (the
same interface tools/get_agni.sh documents), but resolved it with
portable_realpath, which rejects a path that does not exist yet: BSD
realpath (so every macOS host) refuses a missing leaf, GNU realpath a
missing parent. set -euo pipefail is enabled only further down the script, so
the empty result travelled on and surfaced much later as a confusing

fatal: could not create work tree dir '': No such file or directory

from git clone. Passing a fresh destination therefore required mkdir -p
first.

Before:

$ bash tools/get_socrates.sh /tmp/some-path-that-does-not-exist
realpath: /tmp/some-path-that-does-not-exist: No such file or directory
...
fatal: could not create work tree dir '': No such file or directory

After (git clone stubbed out so the run stops at the build step):

$ bash tools/get_socrates.sh /tmp/some-path-that-does-not-exist
git clone https://github.com/FormingWorlds/SOCRATES.git /private/tmp/some-path-that-does-not-exist

Changes:

  • portable_realpath 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, intermediate directories
    included, so no mkdir -p is needed anywhere.
  • The helper is duplicated across all nine tools/get_*.sh scripts that carry
    it, and every copy gets the same change. Only get_socrates.sh was reachable
    with a non-existent path today (get_petsc.sh already does mkdir -p before
    resolving, get_spider.sh resolves an existing PETSc tree), but the copies
    are 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.sh stops with a named error if the resolution is empty for any
    other reason, instead of handing an empty string to git clone.
  • get_socrates.sh gained a usage header listing the positional argument and
    --force, matching get_agni.sh.

The manual invocation is not the only trigger: proteus get socrates was
broken on macOS by the same defect. get_socrates() in
src/proteus/utils/data.py returns early when socrates/ already exists, and
otherwise passes that not-yet-existing absolute path to the script without
creating it, which is exactly the argument portable_realpath refused. The
route the docs and proteus install-all use was unaffected, because it invokes
the script with no argument and takes the branch that never resolves an install
path.

Verified by calling proteus.utils.data.get_socrates() with git replaced by
a stub that logs its arguments. Against the script on main:

socrates/ exists before call: False
realpath: /var/folders/.../T/fakeproteus-l2h5sgxv/socrates: No such file or directory
git clone https://github.com/FormingWorlds/SOCRATES.git
socrates/ created by the run: False

and with this branch:

socrates/ exists before call: False
git clone https://github.com/FormingWorlds/SOCRATES.git /private/var/folders/.../T/fakeproteus-eal09lky/socrates
socrates/ created by the run: True

Both runs then fail at the build step, since the stubbed clone leaves no source
tree. No change is needed in data.py itself.

Resolution of existing paths is unchanged, symlinks included: the realpath
branch 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 realpath at /bin/realpath.

  • Reproduced the reported failure on the unmodified script, with git replaced
    by 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.
  • Confirmed the proteus get socrates route (proteus.utils.data.get_socrates,
    wired at cli.py:558) hits the same failure before this change and passes the
    correct destination after it, with git stubbed as described above.
  • Confirmed git clone creates a nested destination (git clone <src> a/b/c
    with no a/), which is why no mkdir -p was added.
  • Checked the patched helper directly: missing leaf, missing nested path,
    relative missing path, existing directory, and a symlinked directory (still
    followed to its target).
  • Added five tests to tests/tools/test_install_scripts.py, all of which fail
    against the pre-fix script and pass after it: missing-path resolution, the
    cross-script identical-helper invariant, install-path resolution before the
    checkout exists, --force with and without a path, and the empty-resolution
    error path.
  • The test file previously carried its own inline copy of portable_realpath;
    it now lifts the shipped definition out of get_socrates.sh, so the cases
    run 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 23
    fail on this branch without this change (all in
    tests/interior_energetics/test_aragog*.py, unrelated to the shell scripts).
  • ruff check src/ tests/ and ruff format --check src/ tests/ : clean.
    bash tools/validate_test_structure.sh : complete.
    python tools/check_test_quality.py --check reports no violations in the
    touched file (its pre-existing regressions are in other files).
  • bash -n on all tools/get_*.sh : clean. shellcheck is not installed on
    this host, so the scripts were not linted with it.

Not verified: the behaviour of GNU realpath on a missing parent was not
exercised 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

  • I have followed the contributing guidelines
  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings or errors
  • I have checked that the tests still pass on my computer
  • I have updated the docs, as appropriate (usage header in the script; the
    installation docs do not document the positional argument)
  • I have added tests for these changes, as appropriate
  • I have checked that all dependencies have been updated, as required (none
    changed; python3 was already the helper's fallback)

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.
@egpbos
egpbos requested a review from a team as a code owner September 9, 2026 10:34
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.46%. Comparing base (24a7040) to head (6fd24a3).

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           
Flag Coverage Δ
unit-tests 87.25% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@egpbos
egpbos marked this pull request as draft September 9, 2026 11:44
@egpbos
egpbos marked this pull request as ready for review September 9, 2026 12:01
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