Fix has_local to not search PATH for external 'local' command - #5018
Fix has_local to not search PATH for external 'local' command#5018harriiinnii wants to merge 2 commits into
Conversation
On systems where /bin/sh has no builtin `local`, `has_local` would fall through and search $PATH for a command named `local`. If such a command existed there, it would be incorrectly used as the alias target. Fix by setting PATH= before the `local` call so that $PATH is not consulted for the fallback, matching the behavior on shells that have a builtin `local`. Fixes rust-lang#5009
|
@harriiinnii Oops, it seems like your change is not a pure refactoring step? Would you mind doing some more digging? |
| has_local() { | ||
| # shellcheck disable=SC2034 # deliberately unused | ||
| local _has_local | ||
| PATH= local _has_local |
There was a problem hiding this comment.
Maybe try (PATH= local _has_local)?
|
Thanks for the review, @rami3l. A couple of things I want to address: The Accidental file mode change: I noticed this commit also changed git checkout --theirs rustup-init.sh # restore mode from main
git add rustup-init.sh
git commit --amend --no-edit
git push --force-with-leaseOr I can re-open the PR from a clean branch if you prefer. Let me know how you'd like to proceed. |
rustup-init.sh was accidentally committed as 100644 (non-executable); restore it to 100755. shellcheck flags 'PATH= local _has_local' as SC1007 (space after =), but the space is intentional: PATH= is a POSIX command-prefix assignment, not a variable assignment, so suppress the warning.
|
@harriiinnii You are not supposed to be copy-pasting from an LLM chat which is in direct conflict with our contribution guidelines. |
|
Conflicting with the guidelines is one thing. Offering a correct fix is the other. IMHO the latter weighs more! Not clear why rami3l suggests suggests adding parens, without any motivation. Is it just a misunderstanding of this Shell syntax? Or are there Shells that have problems with this common syntax? |
Problem
has_local()inrustup-init.shis used to detect whether the shell has a builtinlocalkeyword. On shells that lack it, the function falls through and the script aliaseslocaltotypeset.However, if the shell has no builtin
localand an executable namedlocalhappens to exist somewhere in$PATH, that external command would be found and used instead — makinghas_localreturn success incorrectly. Any subsequentlocalusage would then invoke that external command rather than thetypesetalias.Fix
Set
PATH=before thelocalcall insidehas_local. This prevents the shell from searching$PATHfor alocalexecutable: on shells with a builtinlocalthe call succeeds as before, and on shells without it the call correctly fails even if an externallocalexists in$PATH.Fixes #5009