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: use #!/usr/bin/env bash #315

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JarvisCraft
Copy link

Description

This replaces /bin/bash shebang with a more universal /usr/bin/env bash.

This is required for easier packaging in systems which don't provide the former such as NixOS.

@schwabe
Copy link
Contributor

schwabe commented Jul 8, 2024

All the scripts that you are modifying are deprecated. The cmake build system should be used instead.

@JarvisCraft
Copy link
Author

JarvisCraft commented Jul 8, 2024

@schwabe, thanks for your response!

AFAIKS, openvpn3-linux v22_dev calls some of them while building (namely, version), which happens to be an issue.
This may be due to it using v3.8.5 tag instead of master, but I guess that it would still be better to have these scripts more portable until they are fully eliminated.

@schwabe
Copy link
Contributor

schwabe commented Jul 8, 2024

Might be the version one. I would rather like to to see to use #!/bin/sh and make it portable than to use the env hack.

@dsommers
Copy link
Member

dsommers commented Jul 8, 2024

I'm also willing to consider other approaches in openvpn3-linux to extract the version information from OpenVPN 3 Core, including not depending on any scripts from the Core library. I'm also not a huge fan of the env approach as well.

@JarvisCraft
Copy link
Author

I would rather like to to see to use #!/bin/sh and make it portable than to use the env hack.

I like this idea! While I don't consider env a hack (IMO, it is the de facto standard way to write bash-shebangs), I support the idea of migrating to pure sh.

@JarvisCraft
Copy link
Author

I've created #317 as an attempt to make all scripts sh-compatible, please take a look at it

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