Skip to content

Add RunProcess, a safer alternative to Exec - #6536

Open
fingolfin wants to merge 1 commit into
masterfrom
mh/RunProcess
Open

Add RunProcess, a safer alternative to Exec#6536
fingolfin wants to merge 1 commit into
masterfrom
mh/RunProcess

Conversation

@fingolfin

@fingolfin fingolfin commented Aug 29, 2026

Copy link
Copy Markdown
Member

Grew out of #5103, and hence closes #5103.

Exec hands its concatenated arguments to a shell. That makes it hard to pass arguments containing spaces or quotes, ties the behaviour to whichever shell happens to be installed, discards the exit code, and wires the child process to the user's terminal.

RunProcess takes the program and its arguments as separate strings and runs it directly:

RunProcess( cmd[, arg1, ..., argN][, options] )  ->  rec( status[, output] )

For example:

gap> RunProcess("echo", "GAP is great!");
rec( output := "GAP is great!\n", status := 0 )
gap> RunProcess("false").status;
1
  • Nothing needs quoting or escaping, and no shell is involved.
  • The exit code is returned rather than thrown away; a nonzero code is not an error, it is up to the caller to check.
  • Output is captured by default instead of being printed.
  • Input defaults to nothing at all, rather than to whatever the user types.

The price is that shell features such as redirections and wildcard expansion are gone, so Exec stays; its documentation now points here for new code.

What changed since #5103

The draft let the working directory and the streams appear as positional arguments in any order. Following the discussion there (@ChrisJefferson, @wilfwilson), everything beyond the command line now goes into an optional trailing options record with the keys directory, input and output:

RunProcess("sort", rec(input := someStream, output := OutputTextUser()));

Unknown keys are rejected rather than ignored. That is the point of the record: GAP currently cannot capture a child process's standard error at all (#4657), and this leaves room to add an error key later without breaking any caller — and without older GAP versions silently dropping it.

The name is the other thing #5103 stalled on. RunProcess sits next to the existing Process operation, reads as a verb, and matches Mathematica's RunProcess and Python's subprocess.run.

Arguments must be strings or integers; anything else is an error. Deliberately not String-ing arbitrary objects, as that would close off future extensions.

Co-authored-by: Claude Opus 5 noreply@anthropic.com

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library labels Aug 29, 2026
@fingolfin fingolfin added this to the GAP 4.17.0 milestone Aug 29, 2026
@fingolfin
fingolfin marked this pull request as ready for review August 29, 2026 13:14
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.96774% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.99%. Comparing base (989cc35) to head (3eb8de7).

Files with missing lines Patch % Lines
lib/helpview.gi 0.00% 20 Missing ⚠️
lib/streams.gi 0.00% 5 Missing ⚠️
lib/process.gi 97.01% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6536      +/-   ##
==========================================
+ Coverage   78.98%   78.99%   +0.01%     
==========================================
  Files         684      684              
  Lines      294205   294271      +66     
  Branches     8647     8670      +23     
==========================================
+ Hits       232369   232466      +97     
+ Misses      60028    59986      -42     
- Partials     1808     1819      +11     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@fingolfin fingolfin added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Aug 29, 2026
`Exec` hands its concatenated arguments to a shell. That makes it hard
to pass arguments containing spaces or quotes, ties the behaviour to
whichever shell is installed, discards the exit code, and wires the
child process to the user's terminal.

`RunProcess` takes the program and its arguments as separate strings and
runs it directly, so nothing needs quoting and no shell is involved. It
returns a record holding the exit code and, by default, the captured
output. Anything beyond the command line is passed in an optional
trailing options record with the keys `directory`, `input` and `output`;
unknown keys are rejected, so that `error` can be added once GAP is able
to capture the standard error stream of a child process (see #4657).

The price is that shell features such as redirections and wildcard
expansion are no longer available, so `Exec` stays.

Convert the callers in `helpview.gi` and `streams.gi`. The terminal
browsers keep the user's terminal explicitly, as they are interactive.

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library

1 participant