socketry / nio4r

Cross-platform asynchronous I/O primitives for scalable network clients and servers.
Other
965 stars 86 forks source link

Please update intree copy of libev #220

Closed darix closed 5 years ago

darix commented 5 years ago

the version that you have right now fails for some distros.

https://build.opensuse.org/package/live_build_log/home:darix:apps/rubygem-nio4r/openSUSE_Tumbleweed/x86_64

while our newer libev copy just works

https://build.opensuse.org/package/live_build_log/openSUSE:Factory/libev/standard/x86_64

tarcieri commented 5 years ago

@ioquatix can you take care of this? I can provide some instructions (which I should probably write down somewhere better than this issue but):

  1. Download the latest libev
  2. Copy the files that are in-tree over the existing ones. You shouldn't need any of the files that aren't currently in-tree
  3. Look for the labeled patches to ev.c and apply them (sorry I don't have a proper patch file or way of generating one automatically at the moment): https://github.com/socketry/nio4r/search?q=patchery&unscoped_q=patchery
  4. Apply any other previous changes: https://github.com/socketry/nio4r/commit/785ab43ec827b3cf9aaeef2676d496289c3ec232

Not the greatest process. Perhaps we can look at writing a rake task that automates this or something.

ioquatix commented 5 years ago

Sure - I did this recently - but is there a newer update? I'll check.

darix commented 5 years ago

your patching introduces a bug:

ev_backend_poll

../libev/ev.c:3643:1: error: no return statement in function returning non-void [-Werror=return-type]

and it also complains about:

 In file included from /usr/include/ruby-2.6.0/ruby/ruby.h:29,                                                                                                                                 
                  from /usr/include/ruby-2.6.0/ruby.h:33,                                                                                                                                      
                  from nio4r.h:9,                                                                                                                                                              
                  from nio4r_ext.c:6:                                                                                                                                                          
 nio4r_ext.c: In function ‘Init_nio4r_ext’:                                                                                                                                                    
 /usr/include/ruby-2.6.0/ruby/defines.h:211:18: warning: passing argument 1 of ‘ev_set_allocator’ from incompatible pointer type [-Wincompatible-pointer-types]                                
   211 | #define xrealloc ruby_xrealloc                                                                                                                                                        
       |                  ^~~~~~~~~~~~~                                                                                                                                                        
       |                  |                                                                                                                                                                    
       |                  void * (*)(void *, size_t) {aka void * (*)(void *, long unsigned int)}                                                                                               
 nio4r_ext.c:15:22: note: in expansion of macro ‘xrealloc’                                                                                                                                     
    15 |     ev_set_allocator(xrealloc);                                                                                                                                                       
       |                      ^~~~~~~~                                                                                                                                                         
 In file included from nio4r_ext.c:7:                                                                                                                                                          
 ../libev/ev.c:1776:27: note: expected ‘void * (*)(void *, long int)’ but argument is of type ‘void * (*)(void *, size_t)’ {aka ‘void * (*)(void *, long unsigned int)’}                       
  1776 | ev_set_allocator (void *(*cb)(void *ptr, long size) EV_NOEXCEPT) EV_NOEXCEPT                                                                                                          
       |                   ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                   
darix commented 5 years ago

just adding return 0 at the end would make it build but i am almost sure that is not what you want there :)

darix commented 4 years ago

tested with 2.5.1. still broken ev_backend_poll:

error: no return statement in function returning non-void 
[-Werror=return-type]
ioquatix commented 4 years ago

Hmmm it looks like issues with libev, if you have a few minutes you can rewrite long size to size_t and I don’t know about the other issue will need to review code.

ioquatix commented 4 years ago

return Qnil should be sufficient for the second issue. I’m on mobile phone and have meetings for the rest of the day, but can review later.

darix commented 4 years ago

Yes those other issues are in upstream libev as well. but the important thing is the code patched in by you for the extension. the upstream issues are annoying warnings (and libev has a lot of them) but can be ignored for now.

ioquatix commented 4 years ago

Yes agreed

ioquatix commented 4 years ago

Okay this has been released in v2.5.2. It was live streamed https://youtu.be/YqYYrpvGWo4

In the future please create new issues for bugs like this.

darix commented 4 years ago

confirmed.