[PATCH] construct listener_fds Hash in 1.8 compatible way

[PATCH] construct listener_fds Hash in 1.8 compatible way

[Ernest W. Durbin III]

This renables the ability for Ruby 1.8 environments to perform reexecs
---
 lib/unicorn/http_server.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 2decd77..9a5795c 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -449,13 +449,14 @@ class Unicorn::HttpServer
     end
 
     self.reexec_pid = fork do
-      listener_fds = Hash[LISTENERS.map do |sock|
+      listener_fds = Hash.new
+      LISTENERS.map do |sock|
         # IO#close_on_exec= will be available on any future version of
         # Ruby that sets FD_CLOEXEC by default on new file descriptors
         # ref: http://redmine.ruby-lang.org/issues/5041
         sock.close_on_exec = false if sock.respond_to?(:close_on_exec=)
-        [ sock.fileno, sock ]
-      end]
+        listener_fds[sock.fileno] = sock
+      end
       ENV['UNICORN_FD'] = listener_fds.keys.join(',')
       Dir.chdir(START_CTX[:cwd])
       cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
-- 
1.8.4

_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Eric Wong]

"Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
> This renables the ability for Ruby 1.8 environments to perform reexecs

Is this for Ruby 1.8.6?  I've only tested on 1.8.7,
I haven't had a 1.8.6 installation in a while.

> --- a/lib/unicorn/http_server.rb
> +++ b/lib/unicorn/http_server.rb
> @@ -449,13 +449,14 @@ class Unicorn::HttpServer
>      end
>  
>      self.reexec_pid = fork do
> -      listener_fds = Hash[LISTENERS.map do |sock|
> +      listener_fds = Hash.new

         listener_fds = {}

is generally preferred unless there's a default value.

I'll apply the edited change if that's alright with you (and update
the commit message for 1.8.6 support).

Thanks!
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Ernest W. Durbin III]

On Fri, Nov 1, 2013 at 12:50 PM, Eric Wong <normalperson@yhbt.net> wrote:
> "Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
>> This renables the ability for Ruby 1.8 environments to perform reexecs
>
> Is this for Ruby 1.8.6?  I've only tested on 1.8.7,
> I haven't had a 1.8.6 installation in a while.
>

I'll admit that the reason I need it is for a Ruby 1.8.6 environment...

However, Ruby 1.8.7 does not have support for that Hash constructor
either it simply fails silently and in a bug prone manner.

```
[ernestd@ewd3do ~]$ ruby --version
ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-linux]
[ernestd@ewd3do ~]$ irb
irb(main):001:0> thing = Hash[ [ 'foo', 'bar' ], ['fizz', 'buzz'] ]
=> {["foo", "bar"]=>["fizz", "buzz"]}
irb(main):002:0> thing = Hash[ [ 'foo', 'bar' ] ]
=> {}
```

```
-bash-4.1$ ruby --version
ruby 1.8.6 (2010-09-02 patchlevel 420) [x86_64-linux]
-bash-4.1$ irb
irb(main):001:0> thing = Hash[ [ 'foo', 'bar' ], ['fizz', 'buzz'] ]
=> {["foo", "bar"]=>["fizz", "buzz"]}
irb(main):002:0> thing = Hash[ [ 'foo', 'bar' ] ]
ArgumentError: odd number of arguments for Hash
from (irb):2:in `[]'
from (irb):2
from :0
```


>> --- a/lib/unicorn/http_server.rb
>> +++ b/lib/unicorn/http_server.rb
>> @@ -449,13 +449,14 @@ class Unicorn::HttpServer
>>      end
>>
>>      self.reexec_pid = fork do
>> -      listener_fds = Hash[LISTENERS.map do |sock|
>> +      listener_fds = Hash.new
>
>          listener_fds = {}
>
> is generally preferred unless there's a default value.
>
> I'll apply the edited change if that's alright with you (and update
> the commit message for 1.8.6 support).

It's up to you how to get this merged :) Whatever is easiest.

>
> Thanks!
> _______________________________________________
> Unicorn mailing list - mongrel-unicorn@rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Hleb Valoshka]

On 11/1/13, Ernest W. Durbin III <ewdurbin@gmail.com> wrote:

> +      LISTENERS.map do |sock|

You mean LISTENERS.each, aren't you? :)

>          # IO#close_on_exec= will be available on any future version of
>          # Ruby that sets FD_CLOEXEC by default on new file descriptors
>          # ref: http://redmine.ruby-lang.org/issues/5041
>          sock.close_on_exec = false if sock.respond_to?(:close_on_exec=)
> -        [ sock.fileno, sock ]
> -      end]
> +        listener_fds[sock.fileno] = sock
> +      end
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Ernest W. Durbin III]

On Fri, Nov 1, 2013 at 1:46 PM, Hleb Valoshka <375gnu@gmail.com> wrote:
> On 11/1/13, Ernest W. Durbin III <ewdurbin@gmail.com> wrote:
>
>> +      LISTENERS.map do |sock|
>
> You mean LISTENERS.each, aren't you? :)
>

the call has been to `LISTENERS.map` as far back in the history of
lib/unicorn/http_server.rb as i could find.

>>          # IO#close_on_exec= will be available on any future version of
>>          # Ruby that sets FD_CLOEXEC by default on new file descriptors
>>          # ref: http://redmine.ruby-lang.org/issues/5041
>>          sock.close_on_exec = false if sock.respond_to?(:close_on_exec=)
>> -        [ sock.fileno, sock ]
>> -      end]
>> +        listener_fds[sock.fileno] = sock
>> +      end
> _______________________________________________
> Unicorn mailing list - mongrel-unicorn@rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Eric Wong]

"Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
> On Fri, Nov 1, 2013 at 1:46 PM, Hleb Valoshka <375gnu@gmail.com> wrote:
> > On 11/1/13, Ernest W. Durbin III <ewdurbin@gmail.com> wrote:
> >
> >> +      LISTENERS.map do |sock|
> >
> > You mean LISTENERS.each, aren't you? :)

Yes, LISTENERS.each is correct here.  Thanks for the catch!

> the call has been to `LISTENERS.map` as far back in the history of
> lib/unicorn/http_server.rb as i could find.

It used to be map because we wanted it to return an array.  Now we don't
care for the return value, so I've tweaked it to each.

Pushed out the following, thanks all.  Should have out 4.7.0 within
a few days (few more cleanups while we're at it).

>From 7e9e4c740aba24096f768f578779dc1053cb8b70 Mon Sep 17 00:00:00 2001
From: "Ernest W. Durbin III" <ewdurbin@gmail.com>
Date: Fri, 1 Nov 2013 10:12:33 -0400
Subject: [PATCH] construct listener_fds Hash in 1.8.6 compatible way

This renables the ability for Ruby 1.8.6 environments to perform reexecs

[ew: clarified this is for 1.8.6,
 favor literal {} over Hash.new,
 tweaked LISTENERS.map => LISTENERS.each, thanks to Hleb Valoshka
]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 lib/unicorn/http_server.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 2decd77..402f133 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -449,13 +449,14 @@ class Unicorn::HttpServer
     end
 
     self.reexec_pid = fork do
-      listener_fds = Hash[LISTENERS.map do |sock|
+      listener_fds = {}
+      LISTENERS.each do |sock|
         # IO#close_on_exec= will be available on any future version of
         # Ruby that sets FD_CLOEXEC by default on new file descriptors
         # ref: http://redmine.ruby-lang.org/issues/5041
         sock.close_on_exec = false if sock.respond_to?(:close_on_exec=)
-        [ sock.fileno, sock ]
-      end]
+        listener_fds[sock.fileno] = sock
+      end
       ENV['UNICORN_FD'] = listener_fds.keys.join(',')
       Dir.chdir(START_CTX[:cwd])
       cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
-- 
1.8.4.483.g7fe67e6.dirty

_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Eric Wong]

"Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
> On Fri, Nov 1, 2013 at 12:50 PM, Eric Wong <normalperson@yhbt.net> wrote:
> > "Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
> >> This renables the ability for Ruby 1.8 environments to perform reexecs
> >
> > Is this for Ruby 1.8.6?  I've only tested on 1.8.7,
> > I haven't had a 1.8.6 installation in a while.
> >
> 
> I'll admit that the reason I need it is for a Ruby 1.8.6 environment...

OK (wow!).   I've been pondering dropping 1.8 entirely, but I think I'll
keep it for now since CentOS 6.x still uses it

Anyways, was that the only 1.8.6-incompatible thing we did?

> However, Ruby 1.8.7 does not have support for that Hash constructor
> either it simply fails silently and in a bug prone manner.
> 
> ```
> [ernestd@ewd3do ~]$ ruby --version
> ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-linux]
> [ernestd@ewd3do ~]$ irb
> irb(main):001:0> thing = Hash[ [ 'foo', 'bar' ], ['fizz', 'buzz'] ]
> => {["foo", "bar"]=>["fizz", "buzz"]}

Actually, on 1.8.7, unicorn does the following (note the extra outer array)

irb(main):001:0> thing = Hash[ [ [ 'foo', 'bar' ], ['fizz', 'buzz'] ] ]
=> {"fizz"=>"buzz", "foo"=>"bar"}

so it was fine
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Ernest W. Durbin III]

On Fri, Nov 1, 2013 at 2:54 PM, Eric Wong <normalperson@yhbt.net> wrote:
> "Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
>> On Fri, Nov 1, 2013 at 12:50 PM, Eric Wong <normalperson@yhbt.net> wrote:
>> > "Ernest W. Durbin III" <ewdurbin@gmail.com> wrote:
>> >> This renables the ability for Ruby 1.8 environments to perform reexecs
>> >
>> > Is this for Ruby 1.8.6?  I've only tested on 1.8.7,
>> > I haven't had a 1.8.6 installation in a while.
>> >
>>
>> I'll admit that the reason I need it is for a Ruby 1.8.6 environment...
>
> OK (wow!).   I've been pondering dropping 1.8 entirely, but I think I'll
> keep it for now since CentOS 6.x still uses it
>
> Anyways, was that the only 1.8.6-incompatible thing we did?

That's all I've found so far! Happy to be dropping thin, and I promise
we're on course to get past 1.8.6 at some point... But I'm just the
Ops guy :)

>
>> However, Ruby 1.8.7 does not have support for that Hash constructor
>> either it simply fails silently and in a bug prone manner.
>>
>> ```
>> [ernestd@ewd3do ~]$ ruby --version
>> ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-linux]
>> [ernestd@ewd3do ~]$ irb
>> irb(main):001:0> thing = Hash[ [ 'foo', 'bar' ], ['fizz', 'buzz'] ]
>> => {["foo", "bar"]=>["fizz", "buzz"]}
>
> Actually, on 1.8.7, unicorn does the following (note the extra outer array)
>
> irb(main):001:0> thing = Hash[ [ [ 'foo', 'bar' ], ['fizz', 'buzz'] ] ]
> => {"fizz"=>"buzz", "foo"=>"bar"}
>
> so it was fine

Interesting. Sorry I missed that in the initial irb tests.

Funnily enough, the List of List's Constructor type isn't documented for 1.8.7

Thanks Again!

> _______________________________________________
> Unicorn mailing list - mongrel-unicorn@rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] construct listener_fds Hash in 1.8 compatible way

[Ernest W. Durbin III]

Apologies for the spam, I had a wacked out command line tool go
haywire on a message that got stuck in spool months ago.

On Fri, Nov 1, 2013 at 10:12 AM, Ernest W. Durbin III
<ewdurbin@gmail.com> wrote:
> This renables the ability for Ruby 1.8 environments to perform reexecs
> ---
>  lib/unicorn/http_server.rb | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
> index 2decd77..9a5795c 100644
> --- a/lib/unicorn/http_server.rb
> +++ b/lib/unicorn/http_server.rb
> @@ -449,13 +449,14 @@ class Unicorn::HttpServer
>      end
>
>      self.reexec_pid = fork do
> -      listener_fds = Hash[LISTENERS.map do |sock|
> +      listener_fds = Hash.new
> +      LISTENERS.map do |sock|
>          # IO#close_on_exec= will be available on any future version of
>          # Ruby that sets FD_CLOEXEC by default on new file descriptors
>          # ref: http://redmine.ruby-lang.org/issues/5041
>          sock.close_on_exec = false if sock.respond_to?(:close_on_exec=)
> -        [ sock.fileno, sock ]
> -      end]
> +        listener_fds[sock.fileno] = sock
> +      end
>        ENV['UNICORN_FD'] = listener_fds.keys.join(',')
>        Dir.chdir(START_CTX[:cwd])
>        cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
> --
> 1.8.4
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying