The Wayback Machine - https://web.archive.org/web/20201226153324/https://github.com/github/homebrew-bootstrap/pull/44
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

Fix ERB interpolation in brew-setup-nginx-conf #44

Merged
merged 1 commit into from Dec 1, 2017

Conversation

@rufo
Copy link
Contributor

@rufo rufo commented Dec 1, 2017

This fixes a problem that #43 seems to have triggered for myself and @phantomdata, whereby no project that uses brew setup-nginx-conf in its bootstrap can successfully interpolate the ERB file. I tested this with three separate projects (halp, githubber.tv and docsotron), and all three returned similar errors:

Error: undefined local variable or method `root' for main:Object
(erb):2:in `<main>'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/lib/ruby/2.3.0/erb.rb:864:in `eval'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/lib/ruby/2.3.0/erb.rb:864:in `result'
/usr/local/Homebrew/Library/Taps/github/homebrew-bootstrap/cmd/brew-setup-nginx-conf.rb:29:in `<top (required)>'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
/usr/local/Homebrew/Library/Homebrew/utils.rb:20:in `require?'
/usr/local/Homebrew/Library/Homebrew/brew.rb:106:in `<main>'

The variable changes, but the stack trace is the same. The problem seems to be that ERB doesn't have access to the local variables set up in the script, and so the binding needs to be passed in to access them.

This fixes all three projects... but what I can't figure out is why it worked in the first place, and why the change in #43 would've broken it. Most of the documentation/howtos I looked up suggested that passing in the current binding to result was required if you needed access to local variables, and I haven't seen anything just yet that suggests the binding wouldn't be necessary in any circumstance. Only thing I can think of is some sort of not-particularly visible setting or library version change, but I haven't narrowed that down yet.

So that's a bit of a stumper, but this does fix the problem, so going to go ahead and get some 👀 (and maybe other 💭) on it 😅

cc: @MikeMcQuaid for review

@MikeMcQuaid
Copy link
Contributor

@MikeMcQuaid MikeMcQuaid commented Dec 1, 2017

So that's a bit of a stumper, but this does fix the problem, so going to go ahead and get some 👀 (and maybe other 💭) on it 😅

If it fixes it for you: :shipit: and thanks!

@MikeMcQuaid MikeMcQuaid merged commit 8958f91 into master Dec 1, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@MikeMcQuaid MikeMcQuaid deleted the fix-setup-nginx-conf-erb branch Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.
Morty Proxy This is a proxified and sanitized view of the page, visit original site.