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: update shebangs to use bash from /usr/bin/env instead of /bin/ for better portability #694

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

MrMebelMan
Copy link
Contributor

Fixes #693

Proposed changes

As said in the issue, on some systems, bash doesn't lie in /bin/, but in other places, like /usr/local/bin/bash (FreeBSD) or even in unpredictable paths like /nix/store/y0vp7qcjz02rsgawlgmx8ilbh2wimcg5-system-path/bin/bash (NixOS).
It makes the installation impossible without modifying the Makefile.
This PR replaces all /bin/bash shebangs with /usr/bin/env bash, improving portability.

Checklist

  • I have read the contributing guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further comments

I consider /usr/bin env to be more portable, because env is more likely to be in /usr/bin than bash is to be in /bin.

@CLAassistant
Copy link

CLAassistant commented Apr 20, 2021

CLA assistant check
All committers have signed the CLA.

@MrMebelMan
Copy link
Contributor Author

Lol, since the CI tests have failed, I suppose /usr/bin env is not as portable as I thought 😅

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

😅 the CI failed because of a different reason but if you run make format it should pass!

@aeneasr
Copy link
Member

aeneasr commented Apr 23, 2021

I think this is good to merge as is

@aeneasr aeneasr merged commit e522062 into ory:master Apr 23, 2021
@MrMebelMan MrMebelMan deleted the bash-path-portability branch April 24, 2021 04:55
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.

Unable to build oathkeeper from source on systems where bash is not in /bin/
3 participants