smoke: fix running grout in a separate terminal

[rjarry]

When grout is started manually in a separate terminal, pgrep -g0 grout returns nothing and exits with an error status. Indeed, there is no grout process started as a child of the shell script.

Only check for the PID if grout was started by the script.

Fixes: 6a7e9a4 ("smoke: print stack traces on crashes")

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where the system attempted to retrieve a process identifier unconditionally, even when the process wasn't configured to run. The retrieval now correctly occurs only when the process is actually started.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds a conditional guard to the grout PID retrieval in smoke/_init.sh. The PID capture via pgrep -g0 grout now executes only when run_grout is true, preventing unnecessary PID lookup attempts when grout is not started.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Single-file change with straightforward conditional logic wrapping an existing command. No complex branching, state management, or side effects to verify.

Possibly related PRs

• smoke: print stack traces on crashes #341: Modifies grout PID capture behavior in smoke/_init.sh for cleanup and diagnostics purposes.

Suggested reviewers

• david-marchand

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "smoke: fix running grout in a separate terminal" directly addresses the primary change in this PR. The objectives clarify that the issue is grout started manually in a separate terminal causing pgrep -g0 grout to fail, and the fix conditionally checks for PID only when the script itself started grout. The title accurately captures the problem being solved and is concise, clear, and specific enough for someone reviewing history to understand the intent.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23666dc and cd56d4d.

📒 Files selected for processing (1)

• smoke/_init.sh (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.sh

⚙️ CodeRabbit configuration file

**/*.sh: - Don't bother about unquoted shell variables.

Files:

• smoke/_init.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). (3)

• GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
• GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
• GitHub Check: build-cross-aarch64

🔇 Additional comments (1)

smoke/_init.sh (1)

143-145: Good fix. Guarding the PID retrieval with run_grout prevents pgrep -g0 grout from failing when grout is started externally. The cleanup function already guards its usage of $grout_pid, so this is safe and correct.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[aharivel]

Why don't we get the process ID of the grout process directly when we create it ?
We put the process in the background with "&" just above with "taskset ..." but then we can use :
grout_pid=$! to capture it no ?
($!) Expands to the process ID of the job most recently placed into the background,
https://www.gnu.org/software/bash/manual/html_node/Special-Parameters.html

[rjarry]

$! returns the PID of the last background "job" that was created. Which isn't necessarily the grout process itself (there may be a taskset parent, or in case of a pipeline, the PID may point to grep or awk). I wanted to make sure we get the correct process ID.

[aharivel]

Yeah ok if we are sure that there is no other grout process running this will work properly - otherwise it will output a list of pid like '12324 34456'

[rjarry]

that is the point of the -g0 argument:

-g, --pgroup pgrp,...
Only match processes in the process group IDs listed. Process group 0 is translated into pgrep's own process group.