brianmario / escape_utils

Faster string escaping routines for your ruby apps
MIT License
513 stars 52 forks source link

RBASIC(result)->klass = ... breaks on Ruby 2.1.0-dev #42

Closed srawlins closed 11 years ago

srawlins commented 11 years ago

I've attempted to compile escape_utils 0.3.2 against MRI 2.1.0-dev as of May 22, and get the following compilation error:

Gem::Installer::ExtensionBuildError: ERROR: Failed to build gem native extension.

    /usr/local/rbenv/versions/2.1.0-dev/bin/ruby extconf.rb 
creating Makefile

make
compiling escape_utils.c
escape_utils.c: In function ‘rb_eu_escape_html_as_html_safe’:
escape_utils.c:126:2: error: assignment of read-only member ‘klass’
make: *** [escape_utils.o] Error 1

I think I found the commit to Ruby trunk that disallowed changing klass: https://github.com/ruby/ruby/commit/4f401816ffaf9b641cfbc8ad12203eedcdb527de#L0R28

Not entirely sure what needs to be done instead... I think maybe @tmm1 's code from https://github.com/brianmario/escape_utils/commit/5d451a64767baff9dac3bc39cac62fc9e884e8c6 just needs to be the only option here. IE:

result = rb_funcall(rb_html_string_class, ID_new, 1, result);

is the way used to build the safe string, regardless of RBASIC. I assume this hasn't already been done because it has a performance hit?

tmm1 commented 11 years ago

We might be able to use RBASIC_SET_CLASS

srawlins commented 11 years ago

Yeah, I tried that but its not available :( maybe before 2.1.0 final though?

$ rake compile
...
compiling ../../../../ext/escape_utils/escape_utils.c
../../../../ext/escape_utils/escape_utils.c: In function ‘rb_eu_escape_html_as_html_safe’:
../../../../ext/escape_utils/escape_utils.c:127:2: warning: implicit declaration of function ‘RBASIC_SET_CLASS’ [-Wimplicit-function-declaration]
...

and

$ rake
...
# Running:

..../usr/local/rbenv/versions/2.1.0-dev/bin/ruby: symbol lookup error: /home/srawlins/code/escape_utils/lib/escape_utils/escape_utils.so: undefined symbol: RBASIC_SET_CLASS
rake aborted!
Command failed with status (127): [ruby -I"lib" -I"/usr/local/rbenv/versions/2.1.0-dev/lib/ruby/gems/2.1.0/gems/rake-10.0.4/lib" "/usr/local/rbenv/versions/2.1.0-dev/lib/ruby/gems/2.1.0/gems/rake-10.0.4/lib/rake/rake_test_loader.rb" "test/**/*_test.rb" ]
brianmario commented 11 years ago

fixed by https://github.com/brianmario/escape_utils/pull/48