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

Replace bash shebang with '#!/usr/bin/env bash" for portability #34543

Merged
merged 1 commit into from Sep 20, 2017

Conversation

Projects
None yet
6 participants
@ultimaweapon
Contributor

ultimaweapon commented Sep 17, 2017

The example system that does not put bash in a /bin directory is FreeBSD. Instead, they put it inside /usr/local/bin.

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Sep 17, 2017

CLA assistant check
All CLA requirements met.

msftclas commented Sep 17, 2017

CLA assistant check
All CLA requirements met.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 18, 2017

Member

There is no Electron for FreeBSD.

Did you hit some actual issue?

Member

joaomoreno commented Sep 18, 2017

There is no Electron for FreeBSD.

Did you hit some actual issue?

@ultimaweapon

This comment has been minimized.

Show comment
Hide comment
@ultimaweapon

ultimaweapon Sep 18, 2017

Contributor

I'm trying to port VS Code to FreeBSD. So, this PR is a first step.

Contributor

ultimaweapon commented Sep 18, 2017

I'm trying to port VS Code to FreeBSD. So, this PR is a first step.

@Tyriar

Tyriar approved these changes Sep 18, 2017

This does seem to be the best for portability 👍 https://stackoverflow.com/a/10383546/1156119

@Tyriar Tyriar removed their assignment Sep 18, 2017

@joaomoreno joaomoreno merged commit ff8954e into Microsoft:master Sep 20, 2017

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
license/cla All CLA requirements met.
Details
@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 20, 2017

Member

Thanks!

Member

joaomoreno commented Sep 20, 2017

Thanks!

@joaomoreno joaomoreno added this to the September 2017 milestone Sep 20, 2017

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 25, 2017

Member

@Tyriar This broke your publish scripts:

2017-09-25T05:22:58.0228590Z *****************************************************************************
2017-09-25T05:22:58.0248040Z Start: Publish to repositories
2017-09-25T05:22:58.0267650Z *****************************************************************************
2017-09-25T05:22:58.0295740Z /usr/bin/env: bash -e: No such file or directory

This is why I don't like PRs which don't fix an actual problem. Reverted that file: d99c7d8

Member

joaomoreno commented Sep 25, 2017

@Tyriar This broke your publish scripts:

2017-09-25T05:22:58.0228590Z *****************************************************************************
2017-09-25T05:22:58.0248040Z Start: Publish to repositories
2017-09-25T05:22:58.0267650Z *****************************************************************************
2017-09-25T05:22:58.0295740Z /usr/bin/env: bash -e: No such file or directory

This is why I don't like PRs which don't fix an actual problem. Reverted that file: d99c7d8

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Oct 6, 2017

Member

I reverted the remainder of this commit in e29c517, it caused issues with Lintian on the deb package: #35638

Member

Tyriar commented Oct 6, 2017

I reverted the remainder of this commit in e29c517, it caused issues with Lintian on the deb package: #35638

@ultimaweapon

This comment has been minimized.

Show comment
Hide comment
@ultimaweapon

ultimaweapon Oct 10, 2017

Contributor

Sorry for the problems. I will investigate these problems later when I have free time.

Contributor

ultimaweapon commented Oct 10, 2017

Sorry for the problems. I will investigate these problems later when I have free time.

@jshap70

This comment has been minimized.

Show comment
Hide comment
@jshap70

jshap70 Oct 10, 2017

not sure if you figured this out already, but #!/usr/bin/env has issues when you try to give the commands arguments like your bash -e from above. I've run into it before trying to do something like #!/usr/bin/env awk -f. It likes to try to parse all of the arguments together as one input to env, almost like it's making a call similar to env "$@" and mashing everything together. This is at least the case with GNU coreutils' env, it's reported to work on BSD, not that that's much help here though.
It looks like you already caught it in d99c7d8

jshap70 commented Oct 10, 2017

not sure if you figured this out already, but #!/usr/bin/env has issues when you try to give the commands arguments like your bash -e from above. I've run into it before trying to do something like #!/usr/bin/env awk -f. It likes to try to parse all of the arguments together as one input to env, almost like it's making a call similar to env "$@" and mashing everything together. This is at least the case with GNU coreutils' env, it's reported to work on BSD, not that that's much help here though.
It looks like you already caught it in d99c7d8

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Oct 11, 2017

Member

@jshap70 that was the first problem that was hit

Member

Tyriar commented Oct 11, 2017

@jshap70 that was the first problem that was hit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment