-
Notifications
You must be signed in to change notification settings - Fork 12
fix: issue-sync-helper.sh dirname failure in stripped PATH environments #2606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -28,7 +28,7 @@ _ISSUE_SYNC_LIB_LOADED=1 | |||||
| # Source shared-constants.sh to make the library self-contained. | ||||||
| # Resolves SCRIPT_DIR from BASH_SOURCE so it works when sourced from any location. | ||||||
| if [[ -z "${SCRIPT_DIR:-}" ]]; then | ||||||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||||||
| SCRIPT_DIR="$(cd "${BASH_SOURCE[0]%/*}" && pwd)" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While
This introduces a regression that can break the script in some invocation scenarios. A more robust approach would be to handle this edge case.
Suggested change
|
||||||
| fi | ||||||
| source "${SCRIPT_DIR}/shared-constants.sh" | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While
${BASH_SOURCE[0]%/*}is a good replacement fordirnameto avoid an external command dependency, it behaves differently when${BASH_SOURCE[0]}does not contain a/. For example, if the script is in the current directory and invoked asbash issue-sync-helper.sh,${BASH_SOURCE[0]}will beissue-sync-helper.sh.dirname "issue-sync-helper.sh"correctly returns.echo "${BASH_SOURCE[0]%/*}"returnsissue-sync-helper.sh, which causescdto fail.This introduces a regression that can break the script in some invocation scenarios. A more robust approach would be to handle this edge case.