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

Undefine uneeded T_DATA allocators #721

Merged
merged 1 commit into from
Apr 2, 2022
Merged

Conversation

casperisfine
Copy link

This is now required to avoid warnings on ruby-head.

See: https://bugs.ruby-lang.org/issues/18007

This is now required to avoid warnings on ruby-head.

See: https://bugs.ruby-lang.org/issues/18007
@mattr-
Copy link
Collaborator

mattr- commented Jan 18, 2022

This LGTM but I don't know what the merge requirements are for redcarpet nowadays and will wait for @robin850 to take action.

@casperisfine
Copy link
Author

No worries.

By the way CI seems dead, I can put up a PR to setup GitHub Actions if you wish.

@robin850 robin850 merged commit 50961ad into vmg:master Apr 2, 2022
@robin850
Copy link
Collaborator

robin850 commented Apr 2, 2022

Thank you very much @casperisfine!

By the way CI seems dead, I can put up a PR to setup GitHub Actions if you wish.

My answer comes late so I don't know if it's still true but you can if you wish. :-) We will have to poke Vicent to enable something (and disable Travis) I guess though.

@tisba
Copy link

tisba commented Jan 16, 2023

Thanks for the fix 🙇

Given that this gem hasn't seen a release in the last 2 years… How are chances for getting a new release sometime soonish? Trying to get a idea for how "durable" my workaround needs to be :)

@inductor
Copy link

@robin850 sorry for the ping, but could I know if there could potentially be a next release? :)

@robin850
Copy link
Collaborator

Hello everyone! Sorry, I've not been very active on my open source projects recently but I will try to release a new version this week-end.

@evaneykelen
Copy link

That would be great @robin850, the warning: undefining the allocator of T_DATA class Redcarpet::Markdown is a sight for sore eyes 🥲 in our tests

robin850 added a commit that referenced this pull request Jan 29, 2023
@robin850
Copy link
Collaborator

Redcarpet 3.6.0 has been released folks! 🎉 Sorry for the inconvenience.

stoivo added a commit to stoivo/ruby-filemagic that referenced this pull request Apr 25, 2023
* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
stoivo added a commit to stoivo/ruby-filemagic that referenced this pull request Apr 25, 2023
* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
joast pushed a commit to joast/ruby-filemagic that referenced this pull request Dec 7, 2023
* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
joast pushed a commit to joast/ruby-filemagic that referenced this pull request Dec 8, 2023
* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
stoivo added a commit to stoivo/ruby-filemagic that referenced this pull request Jan 29, 2024
* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants