Skip to content

Fix shebang #72

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

Closed
wants to merge 1 commit into from
Closed

Fix shebang #72

wants to merge 1 commit into from

Conversation

Freed-Wu
Copy link
Contributor

NixOS doesn't have /bin/bash. Replace it by /usr/bin/env bash.

@Freed-Wu
Copy link
Contributor Author

https://github.com/infertux/bashcov/blob/master/lib/bashcov.rb#L79

Why not get bash path from env bash?

infertux added a commit that referenced this pull request Jul 12, 2023
On most OS, shebangs can only specify a single parameter.
infertux added a commit that referenced this pull request Jul 12, 2023
On most OS, shebangs can only specify a single parameter.
@infertux
Copy link
Owner

https://github.com/infertux/bashcov/blob/master/lib/bashcov.rb#L79

Why not get bash path from env bash?

The main issue with using env is that you cannot pass arguments to bash that way, e.g. /usr/bin/env bash -eu doesn't work. You need to call set -eu as a separate command. This would also fail here.

I started working on it in this commit however the build is still failing. I don't have more time for this at the moment so feel free to continue the work in the pr-72 branch, either here or on GitLab to run tests with GitLab CI. You can also run tests locally with the ./test.sh script in the repo.

@Freed-Wu
Copy link
Contributor Author

Freed-Wu commented Jul 12, 2023

I file a package request for NixOS here. If we want to use bashcov in NixOS, we should fix those shebangs. I am not faimilar with ruby, and anyone who is familiar can discuss on the issue

@Freed-Wu
Copy link
Contributor Author

The main issue with using env is that you cannot pass arguments to bash that way, e.g. /usr/bin/env bash -eu doesn't work. You need to call set -eu as a separate command. This would also fail here.

I think it should be /usr/bin/env -S bash -eu.

NixOS doesn't have /bin/bash. Replace it by `/usr/bin/env bash`.
@infertux
Copy link
Owner

Superseded by #74

@infertux infertux closed this Jul 14, 2023
@infertux
Copy link
Owner

Bashcov 3.0.3 has been released, please give it a try.

@tomeon
Copy link
Collaborator

tomeon commented Jul 15, 2023

@infertux -- the changes in this PR are still needed, I think.

@infertux
Copy link
Owner

@tomeon It's only touching test files so we only need it if we want to run the test suite on NixOS. I assumed we wouldn't but now that we have GitHub Actions set up, is there a way to leverage it and run the tests under NixOS as well?

tomeon pushed a commit that referenced this pull request Jul 15, 2023
On most OS, shebangs can only specify a single parameter.
tomeon pushed a commit that referenced this pull request Jul 15, 2023
On most OS, shebangs can only specify a single parameter.
@tomeon
Copy link
Collaborator

tomeon commented Jul 15, 2023

[I]s there a way to leverage [GitHub Actions] and run the tests under NixOS as well?

There sure is :). I will see about adding it to the nix-support branch.

FWIW, NixOS is my distro of choice these days, so the updated shebangs are helpful for my development workflow. Anyway, better portability is better portability :).

However! Switching from #!/bin/bash to #!/usr/bin/env bash in the test suite is neither necessary nor sufficient to solve "file not found" errors when running the test suite in the Nix build sandbox while constructing a Nix package of Bashcov. The build sandbox provides neither /bin/bash nor /usr/bin/env. Instead, I've use the patchShebangs hook function from the Nixpkgs standard environment to fix up the test scripts' shebangs prior to running RSpec and Cucumber tests.

@Freed-Wu -- if you are a Nix flakes user, I'd be much obliged if you could check out the nix-support branch.

@Freed-Wu
Copy link
Contributor Author

check out the nix-support branch.

nix build can work. 👍

tomeon pushed a commit that referenced this pull request Jul 28, 2023
On most OS, shebangs can only specify a single parameter.
tomeon pushed a commit that referenced this pull request Jul 28, 2023
On most OS, shebangs can only specify a single parameter.
tomeon pushed a commit that referenced this pull request Jul 28, 2023
On most OS, shebangs can only specify a single parameter.
tomeon pushed a commit that referenced this pull request Apr 22, 2024
On most OS, shebangs can only specify a single parameter.
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