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

Add new C APIs used by the new json release #1856

Merged
merged 5 commits into from
Dec 16, 2019
Merged

Conversation

chrisseaton
Copy link
Collaborator

Without these fixes essentially the entire Ruby ecosystem seems to stop working!

Shopify#1

@@ -4715,7 +4715,7 @@ void rb_define_virtual_variable(const char *name, VALUE (*getter)(ANYARGS), void
}

void rb_gc_register_mark_object(VALUE obj) {
rb_tr_error("rb_gc_register_mark_object not implemented");
RUBY_CEXT_INVOKE("rb_gc_register_mark_object", obj);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this could be RUBY_INVOKE_NO_WRAP() to avoid wrapping the return value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean RUBY_CEXT_INVOKE_NO_WRAP I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed.

@eregon
Copy link
Member

eregon commented Dec 14, 2019

Thanks!
@aardvark179 Could you review too?

Without these fixes essentially the entire Ruby ecosystem seems to stop working!

I wonder what depends on the json gem and doesn't use the bundled json, maybe we should figure this out if it doesn't work as expected.

We could also ask for a new json release that checks if those C functions are available before using them, that way TruffleRuby 19.3.0 would still work with latest json.

@eregon eregon self-assigned this Dec 14, 2019
@eregon eregon added this to the 20.0.0 milestone Dec 14, 2019
@ggrossetie
Copy link
Contributor

We could also ask for a new json release that checks if those C functions are available before using them, that way TruffleRuby 19.3.0 would still work with latest json.

That would be great 👍

@eregon
Copy link
Member

eregon commented Dec 14, 2019

We could also ask for a new json release that checks if those C functions are available before using them, that way TruffleRuby 19.3.0 would still work with latest json.

That's unfortunately not so easy because we actually declare and define the function.
I'll check if we can avoid defining functions we don't implement, but I think we can't (i.e., C extensions don't compile anymore) last time I tried.
Maybe we need something like #ifdef GCCOMPACT or so.

@chrisseaton
Copy link
Collaborator Author

I wonder what depends on the json gem and doesn't use the bundled json

i18n is a concrete example at the bottom of many stacks. Non-standard-library json is everywhere.

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Dec 15, 2019
graalvmbot pushed a commit that referenced this pull request Dec 16, 2019
@graalvmbot graalvmbot merged commit ae302a6 into oracle:master Dec 16, 2019
@chrisseaton chrisseaton deleted the json branch December 16, 2019 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed shopify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants