Skip to content

Run cronic's wrapped command with the caller's nounset setting - #138

Closed
mernst wants to merge 3 commits into
plume-lib:masterfrom
mernst:cronic-shellopts
Closed

mernst wants to merge 3 commits into
plume-lib:masterfrom
mernst:cronic-shellopts

Conversation

@mernst

@mernst mernst commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

When the caller exports SHELLOPTS, cronic's set -u was inherited by the wrapped command. A bash script that the command runs then failed on its first unset variable. For example, Daikon's Rocky Linux CI fails in cronic makeinfo --pdf with:

/usr/bin/texi2dvi: line 83: COMSPEC: unbound variable

(texi2dvi is a #!/bin/sh script, and /bin/sh is bash on Rocky Linux.)

Now cronic records whether the caller had nounset set, and runs the wrapped command with the caller's setting. The new tests in tests/cronic-test/cronic-test.sh cover callers that export SHELLOPTS with and without set -u.

When the caller exports SHELLOPTS, cronic's `set -u` was inherited by the
wrapped command, so a bash script that it ran (such as texi2dvi, via
`makeinfo --pdf`, where /bin/sh is bash) failed on an unset variable.
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: plume-lib/plume-scripts/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 99842d91-6ed5-42a4-86bc-91a927a0ce62

📥 Commits

Reviewing files that changed from the base of the PR and between b5ec97e and 85f61c4.


📒 Files selected for processing (2)
  • cronic
  • tests/cronic-test/cronic-test.sh

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ing errexit

Save the caller's options with `set +o` and restore them in a subshell
that runs the wrapped command.  Document the limitation for builtins such
as `source` and `eval`.  Use one test wrapper script instead of two.
@mernst

mernst commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #135, which also handles errexit and allexport and has tests for errexit. The comments from this pull request about || and about source/eval have been copied into #135.

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