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

Use /usr/bin/env bash in nvcc_wrapper and kokkos_launch_compiler for portability #6357

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

msimberg
Copy link
Contributor

This is useful e.g. on NixOS where bash is not in /bin. As far as I know there's no downside to using /usr/bin/env bash as it should be strictly more portable, but am I missing some reason for keeping /bin/bash hardcoded in kokkos_launch_compiler and nvcc_wrapper?

@msimberg msimberg self-assigned this Aug 11, 2023
Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't think of any good reason not to update the shebang to use the env utility.

@msimberg
Copy link
Contributor Author

Failures look like what is being fixed by #6354.

@cz4rs
Copy link
Contributor

cz4rs commented Aug 11, 2023

This is useful e.g. on NixOS where bash is not in /bin. As far as I know there's no downside to using /usr/bin/env bash as it should be strictly more portable, but am I missing some reason for keeping /bin/bash hardcoded in kokkos_launch_compiler and nvcc_wrapper?

Some distros complain about using /usr/bin/env - see Fedora's guideline (this is only a draft!) and rather lenghty discussion here. Main issue seems to be automated dependency tracking.
AFAIK there is no hard requirement to avoid it in Fedora and openSUSE shows rpmlint warning for such scripts.

Copy link
Contributor

@cz4rs cz4rs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Distro packages can be trivially patched, so that's still 👍 for me.
As a bonus, now all the scripts in bin directory use the same shebang.

@dalg24 dalg24 merged commit 3eb0bca into kokkos:develop Aug 11, 2023
26 of 28 checks passed
@dalg24 dalg24 mentioned this pull request Aug 10, 2023
@msimberg msimberg deleted the bin-bash-to-usr-bin-env-bash branch August 11, 2023 18:41
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.

None yet

4 participants