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

compat: address "old-style-definition" compiler error on Ruby 3.1+ #2

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dmke
Copy link

@dmke dmke commented Apr 29, 2024

This fixes an issue with Ruby 3.1+, where the installation fails on compiling the extension.

Compare the following outputs of rake compile of various Ruby versions:

Ruby 3.0.7

https://github.com/digineo/visibility_monitor/actions/runs/8877708931/job/24371823796

/opt/hostedtoolcache/Ruby/3.0.7/x64/bin/ruby -I. ../../../../ext/visibility_monitor_c/extconf.rb
mkdir -p tmp/x86_64-linux/visibility_monitor_c/3.0.7
cd tmp/x86_64-linux/visibility_monitor_c/3.0.7
creating Makefile
/usr/bin/gmake
cd -
cd tmp/x86_64-linux/visibility_monitor_c/3.0.7
compiling ../../../../ext/visibility_monitor_c/ext.c
linking shared-object visibility_monitor_c.so
/usr/bin/gmake install sitearchdir=../../../../lib sitelibdir=../../../../lib target_prefix=
cd -
mkdir -p tmp/x86_64-linux/stage/lib
/usr/bin/install -c -m 0755 visibility_monitor_c.so ../../../../lib
cp tmp/x86_64-linux/visibility_monitor_c/3.0.7/visibility_monitor_c.so tmp/x86_64-linux/stage/lib/visibility_monitor_c.so
Ruby 3.1.5

https://github.com/digineo/visibility_monitor/actions/runs/8877708931/job/24371824086

/opt/hostedtoolcache/Ruby/3.1.5/x64/bin/ruby -I. ../../../../ext/visibility_monitor_c/extconf.rb
mkdir -p tmp/x86_64-linux/visibility_monitor_c/3.1.5
cd tmp/x86_64-linux/visibility_monitor_c/3.1.5
creating Makefile
/usr/bin/gmake
cd -
cd tmp/x86_64-linux/visibility_monitor_c/3.1.5
compiling ../../../../ext/visibility_monitor_c/ext.c
../../../../ext/visibility_monitor_c/ext.c: In function ‘Init_visibility_monitor_c’:
../../../../ext/visibility_monitor_c/ext.c:39:6: error: old-style function definition [-Werror=old-style-definition]
   39 | void Init_visibility_monitor_c() {
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/visibility_monitor_c/ext.c: At top level:
cc1: note: unrecognized command-line option ‘-Wno-self-assign’ may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-parentheses-equality’ may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-constant-logical-operand’ may have been intended to silence earlier diagnostics
cc1: all warnings being treated as errors
gmake: *** [Makefile:247: ext.o] Error 1
rake aborted!
Command failed with status (2): [/usr/bin/gmake]
/home/runner/work/visibility_monitor/visibility_monitor/vendor/bundle/ruby/3.1.0/gems/rake-compiler-1.2.7/lib/rake/extensiontask.rb:195:in `block (2 levels) in define_compile_tasks'
/home/runner/work/visibility_monitor/visibility_monitor/vendor/bundle/ruby/3.1.0/gems/rake-compiler-1.2.7/lib/rake/extensiontask.rb:194:in `block in define_compile_tasks'
/home/runner/work/visibility_monitor/visibility_monitor/vendor/bundle/ruby/3.1.0/gems/rake-13.2.1/exe/rake:27:in `<top (required)>'
/opt/hostedtoolcache/Ruby/3.1.5/x64/bin/bundle:25:in `load'
/opt/hostedtoolcache/Ruby/3.1.5/x64/bin/bundle:25:in `<main>'
Tasks: TOP => compile => compile:x86_64-linux => compile:visibility_monitor_c:x86_64-linux => copy:visibility_monitor_c:x86_64-linux:3.1.5 => tmp/x86_64-linux/visibility_monitor_c/3.1.5/visibility_monitor_c.so
(See full trace by running task with --trace)
Ruby 3.2.4

https://github.com/digineo/visibility_monitor/actions/runs/8877708931/job/24371824399

Output similar to Ruby 3.1.

Ruby 3.3.1

https://github.com/digineo/visibility_monitor/actions/runs/8877708931/job/24371824707

Output similar to Ruby 3.1.

The core of this PR is the last commit (HEAD):

diff --git a/ext/visibility_monitor_c/ext.c b/ext/visibility_monitor_c/ext.c
index 3964e7b..7a95a87 100644
--- a/ext/visibility_monitor_c/ext.c
+++ b/ext/visibility_monitor_c/ext.c
@@ -36,7 +36,7 @@ static VALUE monitor_set_visibility_placeholder(VALUE mod, VALUE method_name, VA
     return Qnil;
 }
 
-void Init_visibility_monitor_c() {
+void Init_visibility_monitor_c(void) {
     VALUE mVisibilityMonitor = rb_define_module("VisibilityMonitor");
 
     rb_define_method(mVisibilityMonitor, "public", monitor_public, -1);

You can ignore the remaining commits, although they've proven useful for further development:

  • HEAD~1 relaxes dev-dependency version constraints (basically to allow a more modern Bundler version)
  • HEAD~2 removes Travis CI and introduces Github Actions

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.

1 participant