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.

Sign up for GitHub

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

Jump to bottom

Fix --ext=c generating buggy code #6269

Closed
wants to merge 1 commit into from
Closed

Fix --ext=c generating buggy code #6269

wants to merge 1 commit into from

Conversation

skull-squadron
Copy link
Contributor

@skull-squadron skull-squadron commented Jan 12, 2023 •
edited

CentOS, Fedora, RHEL and so forth break without this.

Also, Rakefile doesn't need those options.

@skull-squadron
Copy link
Contributor Author

skull-squadron commented Jan 12, 2023 •
edited

Where the hack is the squash rebase button in Microsoft Github? 🦺

@hsbt
Copy link
Member

hsbt commented Jan 12, 2023

puma/puma#2371 seems not related this. Can you explain it?

@skull-squadron
Copy link
Contributor Author

skull-squadron commented Jan 12, 2023 •
edited

@hsbt I misread puma/puma#2381 about offtopic puma/puma#2371. It's puma/puma#2381 only.

@skull-squadron
Copy link
Contributor Author

This also fixes #6268 and #6204

@simi simi mentioned this pull request Jan 12, 2023
Closed
Fix use of require_relative -> relative
a9c2d98 
And, the `spec.extensions` setting from rb-sys

Also, add a trivial C code example
@skull-squadron
Copy link
Contributor Author

I fixed both bundle gem --ext=c foo and bundle gem --ext=rust bar on several machines (CentOS 9 Stream x86_64, Ubuntu 22 LTS x86_64, and macOS 13 arm) I ran test benches against.

It's not ideal and probably not doing everything right, but it's closer to tried-and-true based on what already exists. I'm sure I'll see it's 100% wrong after I sleep.

@deivid-rodriguez
Copy link
Member

This should've been fixed by #6298, so good to close, right?

@deivid-rodriguez
Copy link
Member

(Thanks for your work regardless @steakknife)

@hsbt hsbt closed this Jan 30, 2023
@skull-squadron
Copy link
Contributor Author

@deivid-rodriguez No worries. I'll give it a shot to validate.

@skull-squadron
Copy link
Contributor Author

skull-squadron commented Feb 2, 2023 •
edited

@deivid-rodriguez Broken on mac and Linuxes. Absolutely no improvement. It's still not setting load paths correctly and it pollutes lib/ with a binary shared object. And, it also isn't cleaned by rake clean. It belongs where native extensions always belong by maintaining convention over configuration and convention over useless churn. I have no horse in this race, so it can stay broken. "Engineers" who broken this should own it because it's a huge mess.

@rubygems rubygems locked as too heated and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@skull-squadron @hsbt @deivid-rodriguez