Run more quietly#758
Conversation
📝 WalkthroughWalkthroughThis pull request modifies the Daikon project's build and CI infrastructure across four files. It adds environment variable sanitization (COMSPEC, ComSpec, BIBINPUTS) to CI job invocations in CircleCI configuration files. In the documentation build system, it introduces a CRONIC wrapper for makeinfo and HTML generation commands and updates dependencies to include plume-scripts. Additionally, it refactors the test-misc.sh script to separate javadoc and documentation building into distinct steps with explicit error handling for the doc-all target. Possibly related PRs
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @doc/Makefile:
- Around line 71-73: The rule `${CRONIC}: ../.utils/plume-scripts` is a regular
file dependency, not an order-only prerequisite; either update the Makefile to
make the directory an explicit order-only dependency (`${CRONIC}: |
../.utils/plume-scripts`) if you intend order-only semantics, or keep the
current rule and add a clarifying comment above it explaining that this is a
regular file dependency that ensures the `../.utils/plume-scripts` target
(`update-plume-scripts-in-utils`) runs first; reference `${CRONIC}`,
`../.utils/plume-scripts`, and the `update-plume-scripts-in-utils` MAKE target
so the intent is unambiguous.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.circleci/config.yml.circleci/defs.m4doc/Makefilescripts/test-misc.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: codespecs.daikon (typecheck_latest_part1_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_latest_part2_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_latest_part3_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_bundled_part1_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_bundled_part2_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_bundled_part3_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
🔇 Additional comments (10)
.circleci/config.yml (2)
331-333: LGTM! Consistent environment sanitization across Ubuntu misc jobs.The environment variable exports (
COMSPEC,ComSpec,BIBINPUTS) are correctly applied before invokingtest-misc.sh. This ensures a clean environment for documentation builds, avoiding interference from inherited variables (particularly in Windows-derived CI environments or custom BibTeX paths).Also applies to: 353-355, 375-377, 397-399, 419-421
941-943: LGTM! Rocky Linux misc jobs follow the same sanitization pattern.Also applies to: 963-965
.circleci/defs.m4 (1)
53-60: LGTM! Template correctly defines the environment sanitization.The
misc_jobmacro properly chains the environment exports before invoking the script. This ensures all generated misc jobs inconfig.ymlconsistently apply the sanitization.scripts/test-misc.sh (2)
5-6: LGTM! Helpful clarifying comment.
46-52: Sound workaround for Texinfo'sset -eincompatibility.The pattern correctly:
- Runs
javadocseparately (still underset -e)- Disables errexit for
doc-allwhich invokes Texinfo- Manually checks the exit status and fails appropriately
- Re-enables errexit for subsequent code
This preserves proper error detection while avoiding false positives from Texinfo's interaction with errexit.
doc/Makefile (5)
7-8: Good addition of CRONIC for suppressing verbose output.The
cronicwrapper is a well-established tool that only shows output when the wrapped command fails, which aligns well with the PR's goal to "run more quietly."
93-102: LGTM! CRONIC wrapping for chapter HTML generation.The
daikon/index.htmltarget correctly depends on plume-scripts and wraps the TEXI2HTML invocation with CRONIC to suppress verbose output on success.
104-112: LGTM! Consistent CRONIC wrapping for developer HTML.
116-120: LGTM! PDF generation wrapped with CRONIC.Both
daikon.pdfanddeveloper.pdfcorrectly declare the CRONIC dependency and wrap theirmakeinfo --pdfinvocations.
75-87: The single-file HTML targetsdaikon-htmlanddeveloper.htmlare invoked in CI but lack CRONIC wrapping—unlike split-chapter and PDF targets.Both targets are included in the
all-except-pdftarget (via thehtmldependency), which is invoked during CI builds viamake doc-all. The split-chapter targets (daikon/index.html,developer/index.html) and PDF targets all consistently use${CRONIC}to suppress verbose output on success, but the single-file HTML targets do not. This inconsistency should be intentional or addressed.
No description provided.