Skip to content

cronic: do not trace cronic's own command -- ... line when the caller's xtrace is on - #143

Merged
mernst merged 32 commits into
plume-lib:masterfrom
mernst:cronic-no-command-trace
Oct 11, 2026
Merged

mernst merged 32 commits into
plume-lib:masterfrom
mernst:cronic-no-command-trace

Conversation

@mernst

@mernst mernst commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

When the caller's xtrace is on (bash -x cronic ..., or xtrace in an exported SHELLOPTS), the report's TRACE OUTPUT contained + command -- CMD, which is cronic's line, not the command's.

Now, with bash 4.1 or later, cronic removes the first line of the trace file when the caller's xtrace is on. That line is always the trace of the command -- ... line, because the set command before it is not traced. If PS4 contains a newline, then only the first line of that trace is removed; cronic --help documents that limitation.

Bash before 4.1 (no BASH_XTRACEFD) is unchanged.

Tests:

  • The TRACE OUTPUT sections that the existing tests check (for exported xtrace and for bash -x cronic) no longer contain the command -- line, so those tests no longer need to compute its expected trace.
  • A new test checks the TRACE OUTPUT of a builtin (eval) that sets an EXIT trap.

The PR also separates paragraphs in long comments with #-only lines.

mernst added 12 commits October 10, 2026 10:03
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.
When the caller exports SHELLOPTS with xtrace or verbose, cronic traced or
echoed itself to stderr, so cron sent email on every run.  Also keep cronic's
setup of the command out of the command's trace output.
…ions

# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
… tests

Save the caller's SHELLOPTS rather than a hand-maintained list of option
letters, so that every set -o option is restored for the command.
Document that verbose and BASH_ENV can still make cronic write to stderr.
In the tests, unset PS4, skip trace-section checks when bash lacks
BASH_XTRACEFD, and share the PASS/FAIL reporting.
# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
@coderabbitai

coderabbitai Bot commented Oct 11, 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 38 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: e90ab0ab-5a52-41c0-94a2-887d15d7b3f7

📥 Commits

Reviewing files that changed from the base of the PR and between 959abaa and 1cb7f3b.


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

mernst and others added 17 commits October 11, 2026 12:01
…trace level

Swap the trace descriptor only when the caller's xtrace is on and bash is 4.1
or later; run the command directly otherwise.  Put the restoring redirection
in a function rather than an eval, so that a builtin's trace is not nested an
extra level.  Test a builtin under xtrace, and check every xtrace case for
cronic's own trace.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…he command; tests: generate every SHELLOPTS wrapper with one function, which unsets BASH_ENV
# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
…e comment; tests: run the verbose test through `check`
… same-shell re-exec, reuse BASH_XTRACEFD probe, clarify first-command comment
…e-scripts into cronic-no-command-trace

# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
…c-no-command-trace

# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
…he traced path as bash does, clarify help text, show diffs on test failure
…, clarify BASH_ENV help text, inline compare_streams, regroup header comments
…nts, drop the SHELLOPTS re-exec and two helper scripts from the tests
…c-no-command-trace

# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
…nto cronic-no-command-trace

# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
@mernst
mernst merged commit 7d21142 into plume-lib:master Oct 11, 2026
8 checks passed
@mernst
mernst deleted the cronic-no-command-trace branch October 11, 2026 20:21
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