Skip to content

shebangs: improve portability - #352

Open
emhamm wants to merge 1 commit into
brendangregg:masterfrom
emhamm:improve-portability
Open

emhamm wants to merge 1 commit into
brendangregg:masterfrom
emhamm:improve-portability

Conversation

@emhamm

@emhamm emhamm commented Apr 9, 2025

Copy link
Copy Markdown

First of all: Thank you for this cool project!

Motiviation:
This change should improve portability, e.g. when perl is installed to /opt/bin/.
I am using this patch and thought this might be helpful for others, too.

@BenBE

BenBE commented Apr 9, 2025

Copy link
Copy Markdown

Overall patch LGTM.

Would you mind to split the shebang changes and the whitespace fixes into separate commits? It's somewhat good practice to avoid mixing such changes whenever possible.

@brendangregg

Copy link
Copy Markdown
Owner

Thanks; what are the drawbacks to using env?

I know we discussed it for another repo years ago as someone was proposing a similar change, but I don't have the reference handy.

@emhamm

emhamm commented Apr 14, 2025

Copy link
Copy Markdown
Author

@BenBE

Would you mind to split the shebang changes and the whitespace fixes into separate commits?

Whoops, missed that. Yes I will spilt the commit.

@brendangregg

what are the drawbacks to using env?

Thats a good question. Of course there are drawbacks, but I think they do not outweigh the portability for most of the users in my opionion.
If you want to call e.g. /usr/bin/env -S perl -w (with the split string option) you have to use gnu env variant from at least coreutils 8.30 (2018-07-01) because it did not feature the -S before. So users of ancient Linux distros might/will be affected by this change.
FreeBSD env has the -S option, but I did not find since when.

Today I learned that OpenBSD env does not have this option, same for Solaris/Illumos as far as I read the man pages.
Other *nix systems I did not look into.

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.

3 participants