Skip to content

fix(openemr-cmd): use portable realpath for macOS/BSD#783

Merged
kojiromike merged 2 commits into
openemr:masterfrom
kojiromike:openemr-cmd-realpath-macos
Jun 6, 2026
Merged

fix(openemr-cmd): use portable realpath for macOS/BSD#783
kojiromike merged 2 commits into
openemr:masterfrom
kojiromike:openemr-cmd-realpath-macos

Conversation

@kojiromike
Copy link
Copy Markdown
Member

Summary

The openemr-cmd worktree subcommands fail on macOS with realpath: illegal option -- e. They pass GNU coreutils flags (-e, -m) to realpath, but the BSD realpath shipped on macOS doesn't accept them. The git worktree gets half-created on disk, but state registration (compose override + .env + .worktrees.json entry) never completes.

Background: GNU vs BSD realpath

GNU realpath has three modes:

  • default — all path components except the leaf must exist
  • -e — all components including the leaf must exist, else fail
  • -m — nothing need exist

BSD realpath (macOS) has a single mode equivalent to GNU -e: it resolves symlinks and errors on a missing leaf. Every -m call site in this script resolves a path that already exists (mkdir -p runs first, or the path was just reported by git), so bare BSD realpath is behaviorally equivalent at all 17 sites.

Fix

Detect the platform once via realpath --version (GNU prints a version; BSD errors out) and build command arrays carrying the correct flags:

if realpath --version >/dev/null 2>&1; then
    OE_REALPATH_E=(realpath -e)
    OE_REALPATH_M=(realpath -m)
else
    OE_REALPATH_E=(realpath)
    OE_REALPATH_M=(realpath)
fi

Using command arrays rather than a wrapper function keeps realpath an external command, which avoids shellcheck SC2310 (a function invoked in an || condition disables set -e) under the repo's enable=all config. Symlink resolution — the security property feeding the path-containment checks (e.g. blocking docker/library -> /etc escapes) — is preserved natively on both platforms.

Zero behavior change on GNU/Linux: the flags pass straight through. No new runtime dependency.

Test plan

  • shellcheck clean under the repo's .shellcheckrc (enable=all, --check-sourced --external-sources)
  • bash -n passes
  • On macOS (BSD realpath, bash 3.2): openemr-cmd worktree add <branch> -b completes without realpath: illegal option — compose override + .env written, .worktrees.json entry registered
  • Verified array idiom works on macOS bash 3.2 (arrays always non-empty under set -u)

The worktree subcommands passed GNU coreutils flags (-e, -m) to realpath,
which the BSD realpath shipped on macOS rejects with "realpath: illegal
option -- e". The git worktree got half-created but state registration
(compose override + .env + .worktrees.json) never completed.

GNU realpath has three modes: default (all but leaf must exist), -e (all
incl. leaf must exist, else fail), and -m (nothing need exist). BSD
realpath has a single mode equivalent to GNU -e: it resolves symlinks and
errors on a missing leaf. Every -m call site here resolves a path that
already exists (mkdir -p runs first, or git just reported it), so bare
BSD realpath is equivalent at all sites.

Detect the platform once via 'realpath --version' (GNU prints it; BSD
errors) and build command arrays with the right flags. Using arrays
rather than a wrapper function keeps realpath an external command, which
avoids shellcheck SC2310 (function in an || condition disables set -e)
under the repo's enable=all config. Symlink resolution -- the security
property feeding the path-containment checks -- is preserved natively on
both platforms.

Assisted-by: Claude Code
Copilot AI review requested due to automatic review settings June 5, 2026 19:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes openemr-cmd worktree portability on macOS/BSD by removing reliance on GNU realpath flags (-e, -m) that are not supported by the BSD realpath shipped with macOS, while preserving the script’s path canonicalization/symlink-resolution behavior used for safety checks.

Changes:

  • Added a one-time platform capability check (realpath --version) and introduced OE_REALPATH_E / OE_REALPATH_M command arrays to select GNU flags when available and fall back to bare realpath on BSD/macOS.
  • Replaced all realpath -e / realpath -m call sites in worktree-related logic with the new command arrays.
  • Bumped script version from 1.0.38 to 1.0.39.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The realpath platform-detection block added 14 lines above the function
definitions, shifting the end-of-functions boundary from 1446 to 1460.
The test harness slices the script at OC_SCRIPT_FUNCS_END to source only
the function defs; the stale 1446 cut landed mid-if, breaking every
sourced-function test with "unexpected end of file".

Assisted-by: Claude Code
@kojiromike kojiromike merged commit 62fa2a0 into openemr:master Jun 6, 2026
7 checks passed
@kojiromike kojiromike deleted the openemr-cmd-realpath-macos branch June 6, 2026 00:57
kojiromike added a commit that referenced this pull request Jun 6, 2026
…#784)

## Summary

`openemr-cmd worktree list` and `worktree remove` can fail on setups
with many git worktrees: `list` shows `(no worktrees)` even when entries
exist, and `remove` aborts with `Path '...' is not a registered git
worktree`.

## Root cause

`wt_validate_dir` and `wt_validate_dir_safe` validate a path by piping
git's worktree listing into `grep`:

```bash
git -C "${OPENEMR_ROOT}" worktree list --porcelain 2>/dev/null \
    | grep -Fqx "worktree ${dir_real}"
```

The script runs under `set -o pipefail`. `grep -q` exits as soon as it
finds a match and closes the read end of the pipe. If git is still
writing (its output exceeds the ~64KB pipe buffer, which happens once
you have many worktrees), git receives `SIGPIPE` and exits 141. Under
`pipefail` the pipeline's exit status becomes that 141, so the `if !`
branch fires and a path that *is* a registered worktree gets rejected.

This stays latent on typical setups where git's output fits in the pipe
buffer and git finishes writing before `grep` exits — the SIGPIPE never
happens.

## Fix

Capture the listing into a variable first, then match with a herestring.
No pipe means no SIGPIPE:

```bash
local worktrees
worktrees=$(git -C "${OPENEMR_ROOT}" worktree list --porcelain 2>/dev/null)
if ! grep -Fqx "worktree ${dir_real}" <<< "${worktrees}"; then
    ...
fi
```

## Test plan

- [x] `shellcheck` clean under the repo's `.shellcheckrc` (`enable=all`)
- [x] `bash -n` passes
- [x] Reproduced the failure on a machine with ~150 worktrees: the old
pipe form fails under `pipefail` (pipeline exits 141); the herestring
form returns 0
- [x] End-to-end on macOS: `worktree list` shows the entry and `worktree
remove` completes cleanly (both previously broken)

## Note

Found while verifying #783 (the macOS realpath fix). That fix let
execution finally reach this validation code, surfacing this separate,
platform-independent bug. The two are independent; this PR stands alone.
kojiromike added a commit to kojiromike/openemr-devops that referenced this pull request Jun 6, 2026
openemr-cmd is a host CLI users run on their Macs, but BATS only ran on
ubuntu-22.04 — so macOS portability bugs (e.g. the BSD realpath issue in
PR openemr#783) reached users with zero CI coverage. Add a macos-14 leg to the
BATS matrix, where the script runs under system bash 3.2.

That surfaced two real bash 3.2 incompatibilities:

- openemr-cmd used ${RESP,,} (bash 4+ case-modification), which throws
  "bad substitution" under macOS bash 3.2. Replace with a [Yy] glob
  match, preserving the [Y/n] default-yes semantics.
- The test harness sourced the script via process substitution
  (source <(head -n ...)), which silently defines no functions under
  macOS bash 3.2. Switch the three sites to eval "$(head -n ...)".

The repo is public, so GitHub-hosted macOS minutes are free. The macOS
leg is advisory for now (fail-fast: false, not branch-protection
required). Suite passes 39/39 under both bash 3.2 and bash 5.

Assisted-by: Claude Code
kojiromike added a commit that referenced this pull request Jun 6, 2026
)

## Summary

`openemr-cmd` is a host CLI developers run on their Macs, but its BATS
workflow only ran on `ubuntu-22.04`. So macOS-portability bugs reached
users with zero CI coverage — most recently the BSD `realpath` failure
fixed manually in #783. This adds a `macos-14` leg to the BATS matrix,
where the script runs under macOS system **bash 3.2**, the sharpest
portability risk.

Adding that leg immediately surfaced two real bash 3.2
incompatibilities, both fixed here:

- **`openemr-cmd`** used `${RESP,,}` (bash 4+ case-modification), which
throws `bad substitution` under bash 3.2. Replaced with a `[Yy]` glob
match in `[[ ]]`, preserving the `[Y/n]` default-yes semantics.
(Verified identical behavior on bash 3.2 and 5.)
- **Test harness** sourced the script via process substitution (`source
<(head -n …)`), which silently defines no functions under macOS bash
3.2. Switched the three sites (`helpers.bash`, `helpers_pure.bats`,
`state.bats`) to `eval "$(head -n …)"`.

The repo is public, so GitHub-hosted macOS minutes are free. The macOS
leg is **advisory** for now (`fail-fast: false`, not
branch-protection-required) and can be promoted to required later.

## Test plan

- [x] Full openemr-cmd BATS suite passes **39/39 under system bash 3.2**
(macOS-runner equivalent)
- [x] Full suite passes **39/39 under bash 5** (Linux-runner equivalent)
- [x] `${RESP,,}` → `[Yy]` glob verified semantically identical across
both shells (empty→pull, y/Y→pull, else→skip)
- [x] Workflow YAML validates; `helpers.bash` and `openemr-cmd`
shellcheck-clean
- [ ] CI: confirm both `ubuntu-22.04` and `macos-14` legs go green
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.

2 participants