Skip to content

Conversation

zenyanle
Copy link
Member

This commit addresses two issues in the bash probe logic:

  1. Unreliable Bash Path Discovery: The probe previously relied solely on the $SHELL environment variable. This could fail on systems where $SHELL is not set to bash (e.g., using zsh or fish) or is not set at all.

  2. Incorrect Probe Attachment: When bash was dynamically linked with libreadline.so, the probe incorrectly used the path to libreadline.so for all function hooks. This failed for functions like execute_command, exec_builtin, and exit_builtin which exist only in the main bash executable, not in the readline library.

This patch introduces the following fixes:

  • Adds a fallback to check for /bin/bash if a valid bash path cannot be determined from the $SHELL variable.
  • Refactors the configuration and probe logic to distinguish between the path to the bash executable and the path to libreadline.so.
  • Ensures that probes for readline target either libreadline.so (if present) or the bash binary, while probes for other bash-internal functions always target the bash binary itself.

This change significantly improves the reliability and accuracy of bash command capturing across different system configurations.

This commit addresses two issues in the bash probe logic:

1.  **Unreliable Bash Path Discovery:** The probe previously relied solely on the `$SHELL` environment variable. This could fail on systems where `$SHELL` is not set to bash (e.g., using zsh or fish) or is not set at all.

2.  **Incorrect Probe Attachment:** When bash was dynamically linked with `libreadline.so`, the probe incorrectly used the path to `libreadline.so` for all function hooks. This failed for functions like `execute_command`, `exec_builtin`, and `exit_builtin` which exist only in the main bash executable, not in the readline library.

This patch introduces the following fixes:

-   Adds a fallback to check for `/bin/bash` if a valid bash path cannot be determined from the `$SHELL` variable.
-   Refactors the configuration and probe logic to distinguish between the path to the `bash` executable and the path to `libreadline.so`.
-   Ensures that probes for `readline` target either `libreadline.so` (if present) or the `bash` binary, while probes for other bash-internal functions always target the `bash` binary itself.

This change significantly improves the reliability and accuracy of bash command capturing across different system configurations.
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 26, 2025
@zenyanle
Copy link
Member Author

Two conditions was tested in my local environment

Copy link
Member

@cfc4n cfc4n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LTGM.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 28, 2025
@cfc4n cfc4n merged commit b438a1f into gojue:master Jun 28, 2025
6 checks passed
@zenyanle zenyanle deleted the fix/bash-probe branch June 29, 2025 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants