-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add Truffleruby head to CI #10
Conversation
Hi @gogainda, thank you for this! I have a question - does this Ruby Gem actually builds and works correctly with TruffleRuby? I don't have any experience using it, especially not with C extensions. Do you have some experience with it @stanhu? Did you have things running OK for you? If so, that's great, but also we should wait for the #5 to be completed and merged, and then see if everything is still working. Krzysztof |
ok, doesn't look like it's working on truffleruby, will close for now |
Hi @gogainda, thank you for checking! How does it "not work" - does it failed to build? Not loading? Crashing? I would imagine that TruffleRuby would have the same problems with this extension as JRuby. Krzysztof |
@kwilczynski Strange, Truffleruby seems to be hitting an error just compiling the C extension (https://travis-ci.org/github/kwilczynski/ruby-magic/jobs/764661248):
It fails on building the simplest Ruby program:
It doesn't look like this is specific to ruby-magic, though. |
Interestingly, it passed the compilation stage on my Mac, but failed during test run: We may ask @eregon to hear his opinion |
Hi @gogainda and @stanhu, thank you both for looking into this!
The character that allegedly breaks the compilation: [1] pry(main)> 226.chr(Encoding::UTF_8)
"â"
[2] pry(main)> 226.chr(Encoding::ASCII_8BIT)
"\xE2"
[3] pry(main)> [...]
Something is also not passing "/usr/bin/gcc -o conftest -I/home/travis/.rvm/rubies/truffleruby-head/lib/cext/include -I/home/travis/.rvm/rubies/truffleruby-head/lib/cext/include/ruby/backward -I/home/travis/.rvm/rubies/truffleruby-head/lib/cext/include -I../../../../ext/magic -Wimplicit-function-declaration -Wno-int-conversion -Wno-int-to-pointer-cast -Wno-incompatible-pointer-types -Wno-format-invalid-specifier -Wno-format-extra-args -ferror-limit=500 -I /home/travis/build/kwilczynski/ruby-magic/tmp/x86_64-linux/magic/2.7.2/file-5.39/src -std=c99 -fPIC -Wall -Wextra -pedantic -O3 -Wcast-qual -Wwrite-strings -Wconversion -Wmissing-noreturn -Winline -I/include conftest.c -L. -L/home/travis/build/kwilczynski/ruby-magic/lib/ext/lib -Wl,-rpath,/home/travis/build/kwilczynski/ruby-magic/lib/ext/lib -Wl,--as-needed -Wl,--no-undefined -L/lib -lmagic -lgraalvm-llvm -L/home/travis/.rvm/rubies/truffleruby-head/lib/cext -rpath /home/travis/.rvm/rubies/truffleruby-head/lib/cext -ltruffleruby -rpath /home/travis/.rvm/rubies/truffleruby-head/lib/sulong/native/lib"
gcc: error: unrecognized command line option ‘-ferror-limit=500’
gcc: error: unrecognized command line option ‘-rpath’
gcc: error: unrecognized command line option ‘-rpath’ I wonder if this is causing this simple Krzysztof |
Hi @gogainda, this is great!
The problem lies with calling the Would you be able to give the following patch a try? diff --git i/ext/magic/ruby-magic.c w/ext/magic/ruby-magic.c
index 91a1d74..93460cc 100644
--- i/ext/magic/ruby-magic.c
+++ w/ext/magic/ruby-magic.c
@@ -1268,7 +1268,7 @@ magic_lock(VALUE object, VALUE(*function)(ANYARGS), void *data)
magic_object_t *mo;
MAGIC_OBJECT(mo);
- rb_funcall(mo->mutex, rb_intern("lock"), 0, Qundef);
+ rb_funcall(mo->mutex, rb_intern("lock"), 0);
return rb_ensure(function, (VALUE)data, magic_unlock, object);
}
@@ -1279,7 +1279,7 @@ magic_unlock(VALUE object)
magic_object_t *mo;
MAGIC_OBJECT(mo);
- rb_funcall(mo->mutex, rb_intern("unlock"), 0, Qundef);
+ rb_funcall(mo->mutex, rb_intern("unlock"), 0);
return Qnil;
}
diff --git i/ext/magic/ruby-magic.h w/ext/magic/ruby-magic.h
index 413e983..37f756f 100644
--- i/ext/magic/ruby-magic.h
+++ w/ext/magic/ruby-magic.h
@@ -156,8 +156,8 @@ static const char *errors[] = {
static inline VALUE
magic_shift(VALUE v)
{
- return ARRAY_P(v) ? \
- rb_funcall(v, rb_intern("shift"), 0, Qundef) : \
+ return ARRAY_P(v) ? \
+ rb_funcall(v, rb_intern("shift"), 0) : \
Qnil;
}
@@ -180,8 +180,8 @@ magic_join(VALUE a, VALUE b)
static inline VALUE
magic_flatten(VALUE v)
{
- return ARRAY_P(v) ? \
- rb_funcall(v, rb_intern("flatten"), 0, Qundef) : \
+ return ARRAY_P(v) ? \
+ rb_funcall(v, rb_intern("flatten"), 0) : \
Qnil;
}
@@ -192,7 +192,7 @@ magic_fileno(VALUE object)
rb_io_t *io;
if (rb_respond_to(object, rb_intern("fileno"))) {
- object = rb_funcall(object, rb_intern("fileno"), 0, Qundef);
+ object = rb_funcall(object, rb_intern("fileno"), 0);
return NUM2INT(object);
}
@@ -213,13 +213,13 @@ magic_path(VALUE object)
return object;
if (rb_respond_to(object, rb_intern("to_path")))
- return rb_funcall(object, rb_intern("to_path"), 0, Qundef);
+ return rb_funcall(object, rb_intern("to_path"), 0);
if (rb_respond_to(object, rb_intern("path")))
- return rb_funcall(object, rb_intern("path"), 0, Qundef);
+ return rb_funcall(object, rb_intern("path"), 0);
if (rb_respond_to(object, rb_intern("to_s")))
- return rb_funcall(object, rb_intern("to_s"), 0, Qundef);
+ return rb_funcall(object, rb_intern("to_s"), 0);
return Qnil;
} This patch is to be applied to the state of the codebase at the following commit: 3355199 - this will point you are the state before we started to vendor Krzysztof |
Hi @gogainda, I am happy to report, that with the above patch, the extension does work fine - it compiles, tests and works without issues. However, there are some subtle differences in the API between vanilla Ruby and TruffleRuby:
<internal:core> core/throw_catch.rb:36:in `catch'
685: def test_magic_singleton_do_not_auto_load_global
686: assert_false(Magic.do_not_auto_load)
687:
=> 688: fork do
689: Magic.do_not_auto_load = true
690:
691: magic_1 = Magic.new
/Users/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:688:in `test_magic_singleton_do_not_auto_load_global'
<internal:core> core/kernel.rb:758:in `fork'
Error: test_magic_singleton_do_not_auto_load_global(MagicTest): NotImplementedError: fork is not available
<internal:core> core/throw_catch.rb:36:in `catch'
486: @magic.buffer nil
487: end
488:
=> 489: assert_equal('wrong argument type nil (expected String)', error.message)
490: end
491:
492: def test_magic_buffer_argument_with_NULL
/Users/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:489:in `test_magic_buffer_with_nil_argument'
<"wrong argument type nil (expected String)"> expected but was
<"wrong argument type NilClass (expected 5)">
diff:
? wrong argument type nil (expected String)
? N Class 5
Failure: test_magic_buffer_with_nil_argument(MagicTest) But this is all most likely nothing that can't be fixed. Krzysztof |
@kwilczynski feel free to create an issue here if think it should be fixed on Truffleruby side |
I am not in a hurry, but it's better to log the issues for the future development |
We're happy to fix error messages differences, could you file a short issue about that ? The diff in #10 (comment) looks good, those Regarding the non-ASCII character |
Hi @eregon, thank you for getting back to me!
The error message difference is interesting, or rather, the reason why we are seeing it. This is mainly related to subtle differences between Ruby and TruffleRuby (or even JRuby where TruffleRuby has its origins). I am using function and a macro from the Ruby C API to tell me what the type and also name of the object is when printing an error, then Ruby returns TruffleRuby (and JRuby, as seen in the error messages people posted here and there) returns here class name for the type of the object, so The Having said that, I could probably add small wrappers to normalise things in my code so that the correct value would be printed regardless of the underlying platform.
Ruby's Sadly, if you don't pass anything, then compiler will complain, for example: ../../../../ext/magic/ruby-magic.c:1282:46: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] Thus, people have been customary passing Now, for some reason TruffleRuby's version of
VALUE
rb_funcall(VALUE recv, ID mid, int n, ...)
{
VALUE *argv;
va_list ar;
if (n > 0) {
long i;
va_start(ar, n);
argv = ALLOCA_N(VALUE, n);
for (i = 0; i < n; i++) {
argv[i] = va_arg(ar, VALUE);
}
va_end(ar);
}
else {
argv = 0;
}
return rb_funcallv(recv, mid, n, argv);
}
def rb_funcall(recv, meth, n, *args)
# see #call_with_thread_locally_stored_block
thread_local_block = Thread.current[:__C_BLOCK__]
Thread.current[:__C_BLOCK__] = nil
recv.__send__(meth, *args, &thread_local_block)
ensure
Thread.current[:__C_BLOCK__] = thread_local_block
end We can see in the above that
No idea. I believe it's a bug (at least such is my hypothesis). We don't use non-ASCII characters in any of the files used in the project. I also doubt that Krzysztof |
Yes, that's worth reporting. If you can tell which C API function does that it would be helpful, then we should be able to easily write a test for it. nil/true/false are typically treated specially in error messages, so we might have missed this specific case.
I would recommend to just disable this warning ( You can also use What TruffleRuby does is it uses how many arguments are passed to |
Actually that warning is probably only in old Ruby versions, for instance the dev version defines |
Hi @eregon,
The following is what I currently have in my runtime environment: kwilczynski@workstation Ruby/ruby-magic main ✘ $ gcc --version
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/c++/4.2.1
Apple clang version 12.0.0 (clang-1200.0.32.29)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin Ruby version: kwilczynski@workstation Ruby/ruby-magic main ✘ $ ruby -v
ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-linux]
Ruby version 2.7.2 has it added (see: include/ruby/ruby.h#L2683-L2693). Sadly it does not seem to hint the compiler and the warning is still present. Krzysztof |
Hi @eregon,
OK. I will have a look and report back. Thank you! [...]
We are building with
Very true. I have never seen this problem being reported when building Ruby. I have an idea, though, what enables the compiler option which is responsible for the warning. Stay put. I will report shortly.
That would work! Good idea! Thank you :)
Oh I see. Hmm. But, do you consider this an issue worth solving? Krzysztof |
If it appears in many gems TruffleRuby might need to for compatibility, but I think so far we've never encountered the issue (before this) and yet many C extensions run on TruffleRuby already. |
Hi @eregon, [...]
I have refactored the way how custom Krzysztof |
Hi @eregon, sorry for late reply! [...]
I was able to derive a small piece of code that can reproduce what I was talking about: #if defined(__cplusplus)
extern "C" {
#endif
#include <ruby.h>
void
Init_test(void)
{
printf("%s\n", rb_class2name(rb_cNilClass));
printf("%s\n", rb_obj_classname(Qnil));
printf("%s\n", rb_class2name(rb_cString));
printf("%s\n", rb_obj_classname(rb_str_new2("")));
/* Check_Type(Qnil, T_STRING); */
/* Check_Type(INT2NUM(0), T_STRING); */
rb_check_type(Qnil, T_STRING);
}
#if defined(__cplusplus)
}
#endif # frozen_string_literal: true
require 'mkmf'
dir_config('test')
create_header
create_makefile('test') Running it against Ruby and TruffleRuby:
kwilczynski@rocinante Ruby/test $ rbenv local 3.0.0
kwilczynski@rocinante Ruby/test $ ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
kwilczynski@rocinante Ruby/test $ ruby extconf.rb
creating extconf.h
creating Makefile
kwilczynski@rocinante Ruby/test $ make
compiling test.c
linking shared-object test/test.bundle
kwilczynski@rocinante Ruby/test $ RUBYLIB=. ruby -rtest -e ''
NilClass
NilClass
String
String
<internal:/Users/kwilczynski/.rbenv/versions/3.0.0/lib/ruby/3.0.0/rubygems/core_ext/kernel_require.rb>:85:in `require': wrong argument type nil (expected String) (TypeError)
from <internal:/Users/kwilczynski/.rbenv/versions/3.0.0/lib/ruby/3.0.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
kwilczynski@rocinante Ruby/test $ rbenv local truffleruby-21.0.0
kwilczynski@rocinante Ruby/test $ ruby -v
truffleruby 21.0.0, like ruby 2.7.2, GraalVM CE Native [x86_64-darwin]
kwilczynski@rocinante Ruby/test $ ruby extconf.rb
creating extconf.h
creating Makefile
kwilczynski@rocinante Ruby/test $ make
compiling test.c
linking shared-object test/test.bundle
kwilczynski@rocinante Ruby/test $ RUBYLIB=. ruby -rtest -e ''
NilClass
NilClass
String
String
/Users/kwilczynski/.rbenv/versions/truffleruby-21.0.0/lib/truffle/truffle/cext.rb:208:in `rb_check_type': wrong argument type NilClass (expected 5) (TypeError)
from object.c:35:in `rb_check_type'
from /Users/kwilczynski/Development/Projects/Personal/Ruby/test/test.c:16:in `Init_test' The function I am using is RBIMPL_ATTR_ARTIFICIAL()
static inline void
Check_Type(VALUE v, enum ruby_value_type t)
{
if (RB_UNLIKELY(! RB_TYPE_P(v, t))) {
goto slowpath;
}
else if (t != RUBY_T_DATA) {
goto fastpath;
}
else if (rbimpl_rtypeddata_p(v)) {
/* The intention itself is not necessarily clear to me, but at least it
* is intentional to rule out typed data here. See commit
* a7c32bf81d3391cfb78cfda278f469717d0fb794. */
goto slowpath;
}
else {
goto fastpath;
}
fastpath:
return;
slowpath: /* <- :TODO: mark this label as cold. */
rb_check_type(v, t);
} Which in Ruby is trying to do some internal optimisations to make the resolution a bit faster, but then it might end up calling the void
rb_check_type(VALUE x, int t)
{
int xt;
if (x == Qundef) {
rb_bug(UNDEF_LEAKED);
}
xt = TYPE(x);
if (xt != t || (xt == T_DATA && RTYPEDDATA_P(x))) {
unexpected_type(x, xt, t);
}
} TruffleRuby also provides #define Check_Type(v,t) rb_check_type((VALUE)(v),(t)) And the void rb_check_type(VALUE value, int type) {
polyglot_invoke(RUBY_CEXT, "rb_check_type", rb_tr_unwrap(value), type);
} I then found a bit of documentation on what might be happening internally: cexts.md#c-runtime-implementation-details. Should I go deeper into finding where things diverge, or would it be enough for you to decide whether this is something that you would consider something to potentially fix? Let me know. :) Krzysztof |
Could you copy-paste that in https://github.com/oracle/truffleruby/issues/new ?
Right, that makes sense. |
Hi @eregon,
Done! See: oracle/truffleruby#2307
Thank you! Meanwhile, I am going to look into adding some code to normalize this so that @gogainda could use this Ruby Gem without any issues on TruffleRuby. Krzysztof |
Hi @gogainda, I believe we are ready to merge this. With a number of recent changes we can successfully build and run against TruffleRuby, as per: kwilczynski@workstation Ruby/ruby-magic main ✔ $ ruby -v
truffleruby 21.0.0, like ruby 2.7.2, GraalVM CE Native [x86_64-linux]
kwilczynski@workstation Ruby/ruby-magic main ✔ $ rake compile
Building ruby-magic using packaged libraries.
Static linking is disabled.
Cross build is disabled.
Using mini_portile version 2.5.0
Downloading file-5.39.tar.gz (100%)
Extracting file-5.39.tar.gz into tmp/x86_64-unknown-linux-gnu/ports/libmagic/5.39... OK
Running 'configure' for libmagic 5.39... OK
Running 'compile' for libmagic 5.39... OK
Running 'install' for libmagic 5.39... OK
Activating libmagic 5.39 (from /home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/ports/x86_64-unknown-linux-gnu/libmagic/5.39)...
checking for ruby.h... yes
checking for rb_thread_call_without_gvl()... yes
checking for rb_thread_blocking_region()... no
checking for magic.h... yes
checking for -lmagic... yes
checking for magic_getpath()... yes
checking for magic_getflags()... yes
checking for utime.h... yes
checking for sys/types.h... yes
checking for sys/time.h... yes
checking for utime()... yes
checking for utimes()... yes
creating extconf.h
creating Makefile
compiling ../../../../ext/magic/functions.c
compiling ../../../../ext/magic/ruby-magic.c
linking shared-object magic/magic.so
kwilczynski@workstation Ruby/ruby-magic main ✔ $ rake test
Loaded suite /home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/.gems/gems/rake-13.0.3/lib/rake/rake_test_loader
Started
....................O
============================================================================================================================================================================================================================================================================================================================================================================
Omission: Platform does not support GC.compact [test_gc_compaction(MagicTest)]
/home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:872:in `test_gc_compaction'
============================================================================================================================================================================================================================================================================================================================================================================
..............................................................................................................O
============================================================================================================================================================================================================================================================================================================================================================================
Omission: Platform does not support fork [test_magic_singleton_do_not_auto_load_global(MagicTest)]
/home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:686:in `test_magic_singleton_do_not_auto_load_global'
============================================================================================================================================================================================================================================================================================================================================================================
............................
Finished in 1.033 seconds.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
160 tests, 225 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications
100% passed
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
154.89 tests/s, 217.81 assertions/s
kwilczynski@workstation Ruby/ruby-magic main ✔ $ rake compile -- --use-system-libraries
Building ruby-magic using system libraries.
checking for ruby.h... yes
checking for rb_thread_call_without_gvl()... yes
checking for rb_thread_blocking_region()... no
checking for magic.h... yes
checking for -lmagic... yes
checking for magic_getpath()... yes
checking for magic_getflags()... yes
checking for utime.h... yes
checking for sys/types.h... yes
checking for sys/time.h... yes
checking for utime()... yes
checking for utimes()... yes
creating extconf.h
creating Makefile
compiling ../../../../ext/magic/functions.c
compiling ../../../../ext/magic/ruby-magic.c
linking shared-object magic/magic.so
kwilczynski@workstation Ruby/ruby-magic main ✔ $ rake test
Loaded suite /home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/.gems/gems/rake-13.0.3/lib/rake/rake_test_loader
Started
....................O
============================================================================================================================================================================================================================================================================================================================================================================
Omission: Platform does not support GC.compact [test_gc_compaction(MagicTest)]
/home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:872:in `test_gc_compaction'
============================================================================================================================================================================================================================================================================================================================================================================
..............................................................................................................O
============================================================================================================================================================================================================================================================================================================================================================================
Omission: Platform does not support fork [test_magic_singleton_do_not_auto_load_global(MagicTest)]
/home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/test/test_magic.rb:686:in `test_magic_singleton_do_not_auto_load_global'
============================================================================================================================================================================================================================================================================================================================================================================
............................
Finished in 0.6579999999999999 seconds.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
160 tests, 225 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications
100% passed
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
243.16 tests/s, 341.95 assertions/s Things look good! When you have a moment, if you could rebase against
Thank you in advance! Krzysztof |
@kwilczynski done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gogainda, thank you!
Thank you for working on that:) |
Hi @gogainda,
No problem! Let me know if this works for you as intended. Krzysztof |
Add Truffleruby head to CI
No description provided.