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

[close #990] Warn on bad shebang line #1014

Merged
merged 1 commit into from
Jun 11, 2020
Merged

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Jun 9, 2020

This article explains binstubs and "shebang" lines: https://devcenter.heroku.com/articles/bad-ruby-binstub-shebang. A relatively common problem when generating a binstub is for it to contain a bad shebang line. Here's an example:

#!/usr/bin/env ruby2.5

If you've got a binstub with that shebang line in your project and are running on Heroku 18 then your app will be forced to use ruby2.5 binary no matter what version of Ruby you've specified in the Gemfile. This causes very odd behavior and weird, difficult to debug bundler errors.

This PR does not fix the problem, but will at least warn anyone with a version number in their ruby shebang line.

This behavior was originally:

It was rolled back because it would falsely claim that #!/usr/bin/env bash was incorrect also it only protected against a limited set of binstubs.

Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if it's also common for users to hardcode system ruby paths in their shebangs (ie not using /usr/bin/env)?

Separately, I wonder if longer term we need metrics for usage of some of the warning features (such as this or the bundler warnings) to see when the issues are no longer occurring (eg due to ecosystem changes) - allowing older buildpack functionality to be gradually retired/simplified?

@schneems
Copy link
Contributor Author

Do we know if it's also common for users to hardcode system ruby paths in their shebangs (ie not using /usr/bin/env)?

In all the tickets i've seen about this, I've never found one that's not /usr/bin/env. These files are generated programatically and not by the user, because of this the types of bugs are pretty consistent. I.e. even the binary names are pretty common ruby2.5 for example.

I wonder if longer term we need metrics for usage of some of the warning features (such as this or the bundler warnings) to see when the issues are no longer occurring (eg due to ecosystem changes) - allowing older buildpack functionality to be gradually retired/simplified?

The "right" thing to do here would be to throw all warnings into honeycomb via bin/report. It's been on my TODO for ages and i'm having a hard time prioritizing it.

@avdi
Copy link

avdi commented Jun 10, 2020

I could swear I read a great page on either Bundler.io or RubyGems.org the other day that explained exactly why this happens (short version: history). But for the life of me I can't find it anymore.

This article explains binstubs and "shebang" lines: https://devcenter.heroku.com/articles/bad-ruby-binstub-shebang. A relatively common problem when generating a binstub is for it to contain a bad shebang line. Here's an example:

```
#!/usr/bin/env ruby2.5
```

If you've got a binstub with that shebang line in your project and are running on Heroku 18 then your app will be forced to use `ruby2.5` binary no matter what version of Ruby you've specified in the `Gemfile`. This causes very odd behavior and weird, difficult to debug bundler errors. 

This PR does not fix the problem, but will at least warn anyone with a version number in their ruby shebang line.

This behavior was originally:


-    Introduced: #586
-    Rolled back: #623
 

It was rolled back because it would falsely claim that `#!/usr/bin/env bash` was incorrect also it only protected against a limited set of binstubs.
@schneems schneems force-pushed the schneems/fu-binstubs branch from 6a1ebc4 to 6499ce8 Compare June 10, 2020 19:21
@schneems
Copy link
Contributor Author

I could swear I read a great page on either Bundler.io

I've seen this one https://bundler.io/man/bundle-binstubs.1.html. Not sure if there was another you're referencing. Binstubs are great for solving the bundle exec problem...provided they don't have bugs in them.

@avdi
Copy link

avdi commented Jun 10, 2020 via email

@schneems schneems merged commit aeed0ce into master Jun 11, 2020
@schneems schneems deleted the schneems/fu-binstubs branch June 11, 2020 16:21
schneems added a commit that referenced this pull request Jun 11, 2020
This reverts commit aeed0ce, reversing
changes made to 30184b6.

Due to ticket https://heroku.support/885880

Error is:

```
!
! invalid byte sequence in UTF-8
!
/app/tmp/buildpacks/dir/lib/language_pack/helpers/binstub_check.rb:27:in match?': invalid byte sequence in UTF-8 (ArgumentError) from /app/tmp/buildpacks
```
schneems added a commit that referenced this pull request Jun 11, 2020
Revert "Merge pull request #1014 from heroku/schneems/fu-binstubs"
@schneems
Copy link
Contributor Author

I had to revert this in #1020

@schneems schneems restored the schneems/fu-binstubs branch June 11, 2020 18:00
krisrang added a commit to skyltmax/heroku-buildpack-ruby that referenced this pull request Oct 2, 2020
* upstream/master:
  [changelog skip] Bring back rake task (heroku#1038)
  CNB: make the gems layer accessible to subsequent buildpacks heroku#1033  (heroku#1037)
  [close heroku#934] Skip rake task if it does not exist (heroku#1036)
  [changelog skip] Move unreleased changelogs (heroku#1035)
  v218 for Monday release (heroku#1034)
  [close heroku#1029] Remove default bin/rake binstub (heroku#1031)
  Document v217 release in master (heroku#1030)
  [close heroku#1027][changelog skip] Fix frozen string error (heroku#1028)
  Handle binary binstubs (heroku#1021)
  v216 release for monday (heroku#1023)
  Revert "Merge pull request heroku#1014 from heroku/schneems/fu-binstubs"
  [close heroku#990] Warn on bad shebang line
  [close heroku#818] Disable spring
  [changelog skip][close heroku#977] Recommend recent Ruby in warning
  [changelog skip][close heroku#977] Recommend recent Ruby in warning
  [close heroku#1001] Put Yarn first on the path
  Update HEREDOC to 2.5 syntax to support indenting
  Switch to using /usr/bin/env bash
  [changelog skip] Fix CNB tests
  Allow Nolockfile to get to compile phase
@edmorley edmorley deleted the schneems/fu-binstubs branch March 1, 2021 11:05
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