Skip to content

pub's binstubs should set #!/bin/bash explicitly #1211

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
DartBot opened this issue Jun 5, 2015 · 8 comments
Closed

pub's binstubs should set #!/bin/bash explicitly #1211

DartBot opened this issue Jun 5, 2015 · 8 comments
Assignees
Labels
type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="96" height="96"hspace="10"> Issue by seaneagan
Originally opened as dart-lang/sdk#21854


From seaneagan/den#7:

This seems to be the culprit http://stackoverflow.com/a/17753098/217408 (/bin/sh -> dash) and dash seems not to support [[ according to the SO answer.

Do you consider changing the first line in /home/myuser/.pub-cache/bin/den to #!/bin/bash, which fixes the issue for me?

[[ is bash specific and according to http://lwn.net/Articles/343924/ the script file should set #!/bin/bash explicitly in this case. Contains some other interesting background I wasn't aware of.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd


Not everyone uses bash, though?


Removed Type-Defect label.
Added Type-Enhancement, Area-Pub, Triaged labels.

@DartBot DartBot added type-enhancement A request for a change that isn't a bug Fixed labels Jun 5, 2015
@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


We could also just use [ ] rather than [[ ]], right?

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="48" height="48"hspace="10"> Comment by seaneagan


They don't have to use it though, just have to have it installed, so that
the binstubs can use it.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


Set owner to @nex3.
Added Started label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


Fixed in dart-lang/sdk@86de424.


Added Fixed label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/405837?v=3" align="left" width="48" height="48"hspace="10"> Comment by zoechi


Great! Works fine now.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="48" height="48"hspace="10"> Comment by seaneagan


Thanks! Looks like the pub executable also uses [[, but it has #!/bin/bash. Should that be changed to be consistent with the binstubs? Not sure if the pub executable uses any other bash-specific features.

Cygwin is also not properly supported by the binstubs and the pub executable, see issue #1120. This means you have to call pub.bat or e.g. den.bat instead of just pub or den, and also that it's impossible to write shell scripts that call these executables and work cross-platform (cygwin and linux/mac).

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


Thanks! Looks like the pub executable also uses [[, but it has #!/bin/bash. Should that be changed to be consistent with the binstubs? Not sure if the pub executable uses any other bash-specific features.

Feel free to file an issue for this, although it's much less urgent since it's not actually breaking anyone.

Cygwin is also not properly supported by the binstubs and the pub executable, see issue #1120.

I believe Cygwin is not an officially supported platform right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants