Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Fix build with MRuby 2.0.0 #73

Closed
wants to merge 3 commits into from
Closed

Conversation

cromega
Copy link

@cromega cromega commented Feb 6, 2019

These changes should fix the build against MRuby 2.0.0. [#72]

The example code in the readme runs fine.

At least one test blows up with a stack corruption error so some additional fixes are needed. There may well be better ways to do this, the change in value.go is especially ugly but fixing this has already pushed my cgo to the limits so I can't do more I'm afraid. Hopefully this will help others to pick up the project and give it some more love.

proc.e is now a union type and the only way to assign values to it is by
converting the raw data to a fixed size byte array.
@erikh
Copy link
Collaborator

erikh commented Feb 6, 2019 via email

@cromega
Copy link
Author

cromega commented Feb 6, 2019

@erikh I'm pretty sure I haven't "fixed" it, I just managed to get the code to compile :)

@erikh
Copy link
Collaborator

erikh commented Feb 6, 2019 via email

array.go Outdated
@@ -1,6 +1,7 @@
package mruby

// #include "gomruby.h"
// mrb_int _mrb_ary_len(mrb_value ary) { return RARRAY_LEN(ary); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please put this definition in gomruby.h instead of here.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I thought about this one after I went to bed.

@@ -139,7 +139,7 @@ func (v *MrbValue) GCProtect() {
// when this value is a proc.
func (v *MrbValue) SetProcTargetClass(c *Class) {
proc := C._go_mrb_proc_ptr(v.value)
proc.target_class = c.class
proc.e = *(*[8]byte)(unsafe.Pointer(&c.class))
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah this isn't going to work, unless 2.0 has reverted some of the structs this will fail; the target_class notion has completely been changed in 1.4.

I need to read the 2.0 source before I can comment further.

@erikh
Copy link
Collaborator

erikh commented Feb 22, 2019

any progress with this? I was curious if you had a chance to figure out the trampoline changes.

@take-cheeze
Copy link

@erikh I'm working on some: #75

@cromega cromega closed this Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants