Skip to content

cronic: run the command with the caller's shell options - #135

Closed
mernst wants to merge 6 commits into
plume-lib:masterfrom
mernst:cronic-caller-options
Closed

mernst wants to merge 6 commits into
plume-lib:masterfrom
mernst:cronic-caller-options

Conversation

@mernst

@mernst mernst commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

When the caller exports SHELLOPTS, the wrapped command inherited cronic's own -e and -u, so a bash script that reads an unset variable failed. 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.)

cronic now runs the command with the caller's options. It also does not export its own variables to the command when the caller's exported SHELLOPTS contains allexport.

This supersedes #138.

When the caller exports SHELLOPTS, the command inherited cronic's own -e and -u,
so a bash script that reads an unset variable (such as texi2dvi) failed.
Running the command in a subshell also keeps the exit and exec builtins from
ending cronic before it reports.
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 9 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: 78816992-ee0c-4bb2-a894-f511fdac29a9

📥 Commits

Reviewing files that changed from the base of the PR and between ad31cec and ca053e2.


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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: plume-lib/plume-scripts/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 97e902da-c44e-4418-add7-71597f8a8083


📥 Commits

Reviewing files that changed from the base of the PR and between 0347a30 and ad31cec.



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


Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

cronic saves the caller’s enabled allexport, errexit, and nounset flags before applying its own settings. It runs the wrapped command in a subshell with the saved flags and continues to capture output and record command status. Tests cover unset-variable reads and wrapped commands with default or explicitly exported shell options.



Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ad31c

No actionable merge-blocking issue remains from the reviewed changes; normal checks can proceed.

Pre-merge checks | Passed 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.



✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR



  • 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.

@mernst mernst changed the title cronic: run the command with the caller's shell options, in a subshell cronic: run the command with the caller's shell options Oct 10, 2026
@mernst

mernst commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Turn off verbose and xtrace for cronic itself, and restore them for the
command, so that the report's TRACE OUTPUT is the command's.  Discard the
traces of cronic's first two commands, so that cronic is silent when the
command succeeds.  Explain why `$(set +o)` cannot save the caller's options.
@mernst

mernst commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #142, which restores the caller's options in the subshell; #136 handles verbose.

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