refactor: pre-push-hook.sh potential regression #34999

pull d0wn3d wants to merge 1 commits into bitcoin:master from d0wn3d:patch-2 changing 1 files +5 −5

d0wn3d commented at 10:07 PM on April 3, 2026: none
Issue being fixed or feature implemented

With set -- A "$LINE" (quoted), the entire input line becomes $2 as a single string, leaving $3 and $4 empty. Since empty $4 never equals "refs/heads/master", the hook always continues, and the commit-signing verification never executes in either repo.

Using the unquoted form set -- A $LINE (no quotes), which relied on word-splitting to populate $2…$5 correctly, trips ShellCheck's SC2086 rule.

What was done?

Parse the fields directly with read: The fix is both ShellCheck-compliant and functionally correct.

edit: Added # shellcheck disable=SC2034 to bypass false positive x not being used warning.

How Has This Been Tested?

Not tested. I initially PR'd this to Dash, but was recommended to PR to Bitcoin too, since it's inherited.

Breaking Changes

None.

Checklist:

Go over all the following points, and put an x in all the boxes that apply.
- I have performed a self-review of my own code
- I have commented my code, particularly in hard-to-understand areas
- I have added or updated relevant unit/integration/functional/e2e tests
- I have made corresponding changes to the documentation
- I have assigned this pull request to a milestone (for repository code-owners and collaborators only)
DrahtBot added the label Refactoring on Apr 3, 2026
DrahtBot commented at 10:08 PM on April 3, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

refactor: pre-push-hook.sh potential regression

## Issue being fixed or feature implemented

With set -- A "$LINE" (quoted), the entire input line becomes $2 as a single string, leaving $3 and $4 empty.
Since empty $4 never equals "refs/heads/master", the hook always continues, and the commit-signing verification never executes in either repo.

Using the unquoted form `set -- A $LINE` (no quotes), which relied on word-splitting to populate `$2`…`$5` correctly, trips **ShellCheck's SC2086** rule.

## What was done?

Parse the fields directly with `read:`
The fix is both ShellCheck-compliant and functionally correct.

edit: Added `# shellcheck disable=SC2034` to bypass false positive `x not being used` warning.

## How Has This Been Tested?

Not tested.

## Breaking Changes

None.

## Checklist:
  _Go over all the following points, and put an `x` in all the boxes that apply._
- [X] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

b595f05bef

d0wn3d force-pushed on Apr 3, 2026
maflcko commented at 9:35 AM on April 7, 2026: member

When you say this is a "regression", is that since 9a1ad7bc0dd8a0769738ca4dffbeb8d55438b0dc?

Concept ack on fixing this.

Though, the project may not have enough reviewers fluent in Bash, so if no one reviews this, it could make sense to re-write in Python.
d0wn3d commented at 2:13 PM on April 7, 2026: none

When you say this is a "regression", is that since 9a1ad7b?

Concept ack on fixing this.

Though, the project may not have enough reviewers fluent in Bash, so if no one reviews this, it could make sense to re-write in Python.

Yes, it was introduced when the shellcheck rule SC2086 was enabled.

Contributors

Labels

refactor: pre-push-hook.sh potential regression #34999

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Reviews