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

Add RegisterFunc on unsupported platforms #234

Merged
merged 45 commits into from
Apr 29, 2024
Merged

Add RegisterFunc on unsupported platforms #234

merged 45 commits into from
Apr 29, 2024
+205 −72

Conversation

TotallyGamerJet
Copy link
Collaborator

Closes #230

@TotallyGamerJet
Enable register func on other platforms
bd9dbbb
@TotallyGamerJet
386 Windows test
b9ea678
@TotallyGamerJet
fix test for 386 Windows test
597f576
@TotallyGamerJet
fix test for 386 Windows test (again)
138608a 
NewCallback can't return bigger than uintptr which is 32bits on 386. Make value the largest that fits on whatever platform this runs on
@TotallyGamerJet
No floats
ea82f58
@TotallyGamerJet
ugh logic hard....
72d4924
@TotallyGamerJet
Copy link
Collaborator Author

Current outstanding issues:

  1. Windows 386 qsort test fails. I don't have a Windows computer to investigate so I just disabled it.
  2. Add more "unsupported" platforms to the GitHub workflows.

TotallyGamerJet
TotallyGamerJet commented Apr 16, 2024
View reviewed changes
func_test.go Outdated Show resolved Hide resolved
@hajimehoshi
Copy link
Member

Hmm, let's fix the issue with 386 and RegisterFunc first, and then address on #230.

@TotallyGamerJet TotallyGamerJet mentioned this pull request Apr 21, 2024
Open
6 tasks
@TotallyGamerJet
Copy link
Collaborator Author

TotallyGamerJet commented Apr 23, 2024 •
edited
Loading

Well it is now confirmed it is something wrong in RegisterFunc as just directly calling SyscallN worked just fine

I had forgotten to enable the test again. It now fails. This deserves a deeper investigation now.

hajimehoshi
hajimehoshi reviewed Apr 24, 2024
View reviewed changes
syscall.go Outdated Show resolved Hide resolved
func_test.go Show resolved Hide resolved
@TotallyGamerJet TotallyGamerJet marked this pull request as ready for review April 24, 2024 16:46
hajimehoshi
hajimehoshi reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

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

We want to have tests with Linux 386, if it is possible.

syscall.go Outdated Show resolved Hide resolved
@TotallyGamerJet
Copy link
Collaborator Author

@hajimehoshi this is ready for review

hajimehoshi
hajimehoshi reviewed Apr 29, 2024
View reviewed changes
Copy link
Member

@hajimehoshi hajimehoshi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the hard work! IIUC, this PR solves multiple problems at the same time: #230 and Linux 32bit. What about splitting this PR? NVM, #230 includes every architectures that PureGo doesn't support.

.github/workflows/test.yml Show resolved Hide resolved
dlfcn_test.go Show resolved Hide resolved
func.go Show resolved Hide resolved
hajimehoshi
hajimehoshi approved these changes Apr 29, 2024
View reviewed changes
Copy link
Member

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@TotallyGamerJet TotallyGamerJet merged commit 1ff8716 into ebitengine:main Apr 29, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hajimehoshi hajimehoshi hajimehoshi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enable RegisterFunc and RegisterLibFunc for non-supported architectures
2 participants
@TotallyGamerJet @hajimehoshi