Skip to content

Fix/launch template unsafe quoting, and performance improvements#542

Open
tcutts wants to merge 4 commits intoExaWorks:mainfrom
tcutts:fix/launch-template-quoting
Open

Fix/launch template unsafe quoting, and performance improvements#542
tcutts wants to merge 4 commits intoExaWorks:mainfrom
tcutts:fix/launch-template-quoting

Conversation

@tcutts
Copy link
Copy Markdown

@tcutts tcutts commented Mar 26, 2026

Fix unsafe shell variable expansions and quoting in launcher scripts

All launcher scripts (aprun_launch.sh, jsrun_launch.sh, mpi_launch.sh, multi_launch.sh, single_launch.sh, srun_launch.sh) and the test launcher.sh had several quoting issues that would cause failures when paths contain spaces. This PR fixes them all:

  1. Replaced source $(dirname "$0")/launcher_lib.sh with source "${0%/*}/launcher_lib.sh" in all scripts, using shell parameter expansion to avoid both the unquoted command substitution and the nested quoting problem that would result from quoting it.
  2. Quoted all uses of $_PSI_J_STDOUT, $_PSI_J_STDERR, $_PSI_J_STDIN, $_PSI_J_PROCESS_COUNT, and $_PSI_J_EC in redirections and command arguments throughout all scripts.

In launcher_lib.sh:

  1. Added a _psi_j_abspath() helper that resolves all incoming path arguments to absolute paths at startup, preventing misresolution if the working directory changes during execution.
  2. Replaced backtick command substitution in ts() with $(...).
  3. On bash 4.2+, ts() now uses printf '%(%Y-%m-%d %H:%M:%S)T' — a zero-fork builtin — instead of spawning a date subprocess for every line of output. The original date-based implementation is retained as a fallback for older bash.

For 10,000 lines of output this saves about 18 seconds of execution time (benchmarked on an M1 Macbook)

@hategan
Copy link
Copy Markdown
Collaborator

hategan commented Mar 27, 2026

Thank you @tcutts!

I'll copy the PR to our repo to let our distributed tests run and merge this once that happens.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.01%. Comparing base (de87be0) to head (03a249d).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #542      +/-   ##
==========================================
+ Coverage   72.80%   75.01%   +2.20%     
==========================================
  Files          89       91       +2     
  Lines        4012     4567     +555     
==========================================
+ Hits         2921     3426     +505     
- Misses       1091     1141      +50     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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