-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Mkmf improvements #4118
base: master
Are you sure you want to change the base?
Mkmf improvements #4118
Conversation
@joankaradimov We keep our copy of stdlib in jruby/ruby but usually don't bother contributors with committing there. It's not a great process but I think it beats maintaining a patch and always pulling pristine stdlib for our builds. I'll review your changes and get it merged in. |
Ok, quick review and it seems mostly good. Regarding removing It seems to me that doing so may make C extensions produce a very confusing error about not being able to find identifiers that should be present from ruby.h, rather than the nice message indicating we don't have C ext support. If people did not have to add ruby.h to their extconf.rb before, then they're expecting it to be added for them. Am I correct in this? Of course the first fix I can come up with is to make ruby.h stub out ALL identifiers and present a better error when they're accessed at build time. That's not great either, and it would mean we have to keep updating it. |
That sounds correct, yes. It did not occur to me, because I never saw it in practice in the native extension gems I tested with.
I can't really come up with a really good alternative. I do have some ideas, though... One thing that could be done is to change all Another thing I'm thinking of is having two sets of headers. The original ones from MRI can be used during the configuration phase and the ones with A third things is to hard-code some regexes that look at the |
I think our best option would be some way to automate the generation of a ruby.h that defines all symbols but errors if they're accessed. I don't know how to do that at the moment, though :-) |
A short-term improvement on simply removing ruby.h from default headers would be to provide an EMPTY ruby.h. Users will get symbol not found errors if they try to build a C extension on JRuby, but non-extensions would build fine. The error messages would not be very helpful, though. I have a simple script that can generate an error-stubbed header from a Ruby dynamic library. This may be sufficient. Here's the generator for the ruby.h:
Here's the sort of defines it creates for all exported Ruby symbols: #define InitVM_Enumerator(...) "JRuby does not support Ruby C extensions"[-1]
#define Init_Array(...) "JRuby does not support Ruby C extensions"[-1]
#define Init_BareVM(...) "JRuby does not support Ruby C extensions"[-1]
#define Init_Bignum(...) "JRuby does not support Ruby C extensions"[-1]
... This allows the following code to compile just fine: #include "./ruby.h"
int main() {
} But the following code produces an error: #include "./ruby.h"
int main() {
rb_str_new("foo");
} Unfortunately, the error is still a bit cryptic...but it does at least have the cause in plain text. I've had little luck finding a better way:
It's a start, anyway! We can generate it periodically from the latest Ruby major release. |
At least for someone using 'cc' on Yosemite, the following might be slightly more helpful: /* This is a dummy routine which is only used by deliberately "calling"
* it with an unwanted parameter, thus generating a compile-time error.
* But the routine itself needs to compile without any warnings.
*/
static int compile_time_ERROR(void) { return (99); }
#define rb_str_new(...) compile_time_ERROR("JRuby_does_not_support_Ruby_C_Extensions") Which has the advantage that the error that this generates will underline the key information: poison_main.c:21:5: error: too many arguments to function call, expected 0, have
1
rb_str_new("foo");
^~~~~~~~~~~~~~~~~
./ruby.h:8:44: note: expanded from macro 'rb_str_new'
...compile_time_ERROR("JRuby_does_not_support_Ruby_C_Extensions")
~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./ruby.h:6:1: note: 'compile_time_ERROR' declared here
static int compile_time_ERROR(void) { return (99); }
^
1 error generated. Disclaimer: I have tested this on exactly one compiler on exactly one system. I have no idea how other C-compilers would format this error. You'd want the "compile_time_ERROR" routine to have a name which is very unlikely to be used by any legit program, to avoid errors in the declaration of that routine. |
@drosehn Thanks for the suggestion! Yes, I need to test both our versions on gcc; it may display the error message and nothing else, which would make this approach largely moot. I find it interesting that with all the compile-time trickery possible in C, there's still no good way to stub out a function such that it produces a useful error when referenced. |
Oh, another missing bit from my patch: typedefs from MRI's ruby.h. Without those, many C exts won't even get to our clever error tricks; they'll error out before even reaching a function call. So all those types need to be handled in a similar way. |
Something like this could be used for the compile time error: #include <assert.h>
#if defined(static_assert) || __cplusplus >= 201103L
#define compile_time_ERROR(MSG) static_assert(0, MSG)
#else
static int compile_time_ERROR(void) { return (99); }
#endif ... it tries to detect if you are running C 11 or C++ 11 compiler. In these cases the compiler should either have a macro (C) or a keyword (C++) called Here's an example:
|
@joankaradimov That's not bad. |
I have a horrible piece of bash. It works with the Ruby headers instead of the dynamic library file. It downloads everything in https://github.com/jruby/ruby/tree/trunk/include. It uses github's aprils-fools-svn-feature so that only the contents of It then proceeds to grep for anything that looks like it might be a part of the C extension API (anything that starts with Here's the script. Running it will generate a bunch of files in the current working directory... svn export --force https://github.com/jruby/ruby/trunk/include .
header_symbols() {
grep \
--recursive \
--extended-regexp \
--only-matching \
--ignore-case \
--no-filename \
$1 **/*.h | sort --unique
}
all_symbols=$(
header_symbols 'rb_\w+'
)
symbols_with_parameters=$(
header_symbols 'rb_\w+\s*\(' | grep -Eoi 'rb_\w+'
)
symbols_without_parameters=$(
comm -23 <(echo "$all_symbols") <(echo "$symbols_with_parameters")
)
symbol_with_parameter_errors=$(
echo "$symbols_with_parameters" |
awk '{print "#define " $0 "(...) compile_time_ERROR(\"JRuby does not support Ruby C Extensions\");"}'
)
symbol_without_parameter_errors=$(
echo "$symbols_without_parameters" |
awk '{print "#define " $0 " compile_time_ERROR(\"JRuby does not support Ruby C Extensions\");"}'
)
echo "#pragma once
#include <assert.h>
#if defined(static_assert) || __cplusplus >= 201103L
#define compile_time_ERROR(MSG) static_assert(0, MSG)
#else
static int compile_time_ERROR(void) { return (99); }
#endif
$symbol_with_parameter_errors
$symbol_without_parameter_errors" > ruby.h
find ruby -type f -name '*.h' -print0 | while IFS= read -r -d $'\0' file
do
# Replace every directory name in $file with ".."
# and then replace the file name with "ruby.h"
# e.g. "some/path/some_file.h" becomes "../../ruby.h"
main_header_path=$(
echo $file | sed -e 's/[^\/]\+\//..\//g' -e 's/\/[^\/]\+$/\/ruby.h/g'
)
echo "#include \"$main_header_path\"" > $file
done @headius: If something like this makes sense I could rewrite it in Ruby. Or maybe leave it like that and hope that it is not completely unreadable. |
2127fba
to
edad375
Compare
I've rebased on top of 9.1.15.0. I've included headers with a bunch of macros/functions/typedefs and a script that regenerates them. P.S. |
edad375
to
05f473f
Compare
This builds on what is done in #4031. What it does is help some native extensions pass the configuration phase and fail in the build phase. Thus a better error message is reported.
P.S.
As far as I understand the change I did in 96cf5a8 must go to https://github.com/jruby/ruby. However, I am not sure where. Is it only in the
jruby-ruby_2_3_1
branch?