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

Update shebang to use env #46

Merged
merged 7 commits into from
Feb 13, 2018

Conversation

acmcarther
Copy link
Collaborator

@acmcarther acmcarther commented May 31, 2017

This PR replaces "/bin/bash" with the more general "/usr/bin/env bash". This is necessary on platforms that don't have bash in /bin, namely BSD and esoteric linux distributions such as Guix and NixOS.

FWIW: This is a pervasive problem is Bazel. My own build rules for Bazel apply a multi thousand line patch just to let Bazel build on NixOS. I'm OK with this PR being closed if you feel this is too much of a niche problem.

acmcarther added a commit to acmcarther/rules_rust that referenced this pull request Jun 1, 2017
@mfarrugi
Copy link
Collaborator

mfarrugi commented Nov 3, 2017

I would consider this strictly more portable, is this blocked for any other reasons?

@lilianmoraru
Copy link

The main maintainers are clearly inactive(a year or over an year).

@acmcarther If you are willing to become a maintainer, can you please ask to be added to the group( bazel-discuss@googlegroups.com )?
It would be really nice to see some of the pull request to get in...

@davidzchen
Copy link
Member

Sorry for the wait. I have been the sole maintainer of this during my spare time, and due to commitments from my regular work and family commitments that have come up, I have not had much time to commit to this.

I have reached out to seek new maintainers, and @acmcarther, @mhlopko, and two others have graciously agreed to take up maintainership of rules_rust. I will help with getting the existing PR's committed and will then take on a more advisory role, focusing on helping the new maintainers get up to speed on the project and answering any questions they may have.

@acmcarther This change looks good to me. Can you rebase and then go ahead and merge? Thanks!

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@acmcarther
Copy link
Collaborator Author

Error was caused by a bad rebase that scooped in changes.

I undid the rebase, re authored the commit, and force pushed. Reviewing test failure now.

@acmcarther
Copy link
Collaborator Author

Test failure was elsewhere. Will merge with dzc's blessing in
#46 (comment)

@acmcarther acmcarther merged commit 790bb36 into bazelbuild:master Feb 13, 2018
@acmcarther acmcarther deleted the acm-make-shebang-use-env branch February 13, 2018 21:17
@hlopko
Copy link
Member

hlopko commented Apr 5, 2018

Sorry to be late in the party, I think this could be rewritten to use https://docs.bazel.build/versions/master/skylark/lib/actions.html#run_shell and get the same bash location as other rules in bazel are using.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants