Skip to content
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

Fix script interpreter name. #1

Closed
wants to merge 2 commits into from
Closed

Fix script interpreter name. #1

wants to merge 2 commits into from

Conversation

MG2R
Copy link

@MG2R MG2R commented Jun 28, 2016

Fixes what is probably just a simple typo.

@MG2R
Copy link
Author

MG2R commented Jun 28, 2016

Seems I didn't have my coffee yet in that first commit.

@fuhry
Copy link
Owner

fuhry commented Jun 28, 2016

Hey, thanks for the PR. I cross-checked with arch's cryptsetup package and that hook also uses #!/usr/bin/ash as the shebang. This would seem to make sense, because:

  • initcpio uses busybox, and busybox's interpreter is typically named ash
  • Arch has moved all binaries from /sbin, /bin and /usr/sbin into /usr/bin, and it would make sense if they did this for initcpio as well

So I think this shebang is actually correct. I'm not aware of any specific recommendations, however, and I don't think it matters anyway. Since all the script does is declare a function, it's clear that it's intended to be sourced rather than executed directly, and the shebang is probably just there as a hint to editors' syntax highlighting.

Your thoughts?

fuhry pushed a commit that referenced this pull request Jun 29, 2016
Fixes issue #1

Also updated out of date help text in the install file
@MG2R
Copy link
Author

MG2R commented Jun 30, 2016

Yeah, you're completely right. I shouldn't create pull requests/do code reviews in the morning before my coffee ;)

A quick google for '/usr/bin/ash' returned nothing but bash, so I went for it.

@MG2R MG2R closed this Jun 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants