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 patches to handle a newer version of Unicorn #22

Merged
merged 1 commit into from
Aug 30, 2011
Merged

Conversation

ericstewart
Copy link
Contributor

Unicorn has changed a bit since the original patch was created it seems.

@tra
Copy link
Owner

tra commented Aug 30, 2011

I'm not using unicorn, what problems/backtraces can you share?

@ericstewart
Copy link
Contributor Author

Sure. When running under Unicorn (in my case 3.5.0 but I also tried on 4.1.1) the existing patch code produces the following trace:

/Users/estewart/.rvm/gems/ree-1.8.7-2011.03/gems/activesupport-2.3.14/lib/active_support/dependencies.rb:131:in `const_missing': uninitialized constant Unicorn::HttpParser::REQ (NameError)
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/bundler/gems/spawn-5cf7fd30e343/lib/patches.rb:60
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/gems/activesupport-2.3.14/lib/active_support/dependencies.rb:182:in `require'
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/gems/activesupport-2.3.14/lib/active_support/dependencies.rb:182:in `require'
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/gems/activesupport-2.3.14/lib/active_support/dependencies.rb:547:in `new_constants_in'
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/gems/activesupport-2.3.14/lib/active_support/dependencies.rb:182:in `require'
    from /Users/estewart/.rvm/gems/ree-1.8.7-2011.03/bundler/gems/spawn-5cf7fd30e343/lib/spawn.rb:25:in `included'
etc...

when it is included. This appears to be because Unicorn::HttpParser::REQ (or at least Unicorn::HttpRequest::REQ which the code references) is no longer there. It looks like it was removed from Unicorn way back in defunkt/unicorn@fe94d80 and has changed since then. Before it was a hash that was being cleared but now that is gone. I'm not sure the @request I added into this patch is even necessary so I can look at that a bit further, but the patch as is at least eliminates the error and works when forking.

These adjustments would support either variant but there may be some in-between that it doesn't handle correctly either. I'm just attempting to add support in for releases near the current release.

tra added a commit that referenced this pull request Aug 30, 2011
Update patches to handle a newer version of Unicorn
@tra tra merged commit 5669dd5 into tra:edge Aug 30, 2011
@tra
Copy link
Owner

tra commented Aug 30, 2011

Looks good, thanks for the patch!

@tra
Copy link
Owner

tra commented Aug 30, 2011

Also, be sure to look at issue 21... https://github.com/tra/spawn/issues/21

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.

2 participants