ebitengine / purego

Apache License 2.0
2.16k stars 68 forks source link

Add FreeBSD support #155

Closed ldnetgate closed 1 year ago

ldnetgate commented 1 year ago

This change adds FreeBSD AMD64 and ARM64 targets.

Includes:

Sponsored by: Rubicon Communications LLC (Netgate)

hajimehoshi commented 1 year ago

Nice work!

Can we add tests by https://github.com/vmactions/freebsd-vm ?

ldnetgate commented 1 year ago

Nice work!

Can we add tests by https://github.com/vmactions/freebsd-vm ?

Not sure if this will satisfy your request (vmactions is new to me and FreeBSD is not a first class citizen of github actions); I've made a custom unit specifically for FreeBSD because of this - setup-go and different golang versions are not available for FreeBSD.

https://github.com/ldnetgate/purego/blob/freebsd-wftest/.github/workflows/test.yml

Test trigger passing: https://github.com/ldnetgate/purego/actions/runs/5951411806

I can merge it into this pull request if it's suitable. thanks.

hajimehoshi commented 1 year ago

I can merge it into this pull request if it's suitable.

Yes, I have some requests on the YAML file, but it is very nice to have tests for FreeBSD! Thanks,

TotallyGamerJet commented 1 year ago

As for the missing symbols we can do what Cgo actually does in the runtime. It has a file called runtime/cgo/freebsd.go. Unfortunately, this pragma "requires" Cgo but the compiler can be told to think that the fakecgo package is Cgo by using -gcflags="github.com/ebitengine/purego/internal/fakecgo=-std". I think this is a better approach since it means the produced binary will run without any issues and doesn't require the preload at each invocation. A comment can also be added above the pragmas so that people trying to cross-compile know what flag and why it's needed.

ldnetgate commented 1 year ago

Thanks for the gcflags hint, that was very useful and solves the problem we have here.

The github action passed in my fork: https://github.com/ldnetgate/purego/actions/runs/5957807111/job/16161095503

TotallyGamerJet commented 1 year ago

Great! Did you create the freebsd.go file in fakecgo? I don't see it.

ldnetgate commented 1 year ago

Great! Did you create the freebsd.go file in fakecgo? I don't see it.

Apologies, missed it during push. It's in now as go_freebsd.go and test ran: https://github.com/ldnetgate/purego/actions/runs/5958576447/job/16162985604

TotallyGamerJet commented 1 year ago

Thank you! We can keep the name just freebsd.go to match what it's called in runtime/cgo. Also can you add it to the README in the list of external code. The README also should have the example updated to show FreeBSD support.

TotallyGamerJet commented 1 year ago

Actually maybe the code itself shouldn't be updated since then we have to complicate the example to explain the special flag. I think just update the line that mentions the full example to include FreeBSD

ldnetgate commented 1 year ago

tweaked the two items you mentioned and tested (https://github.com/ldnetgate/purego/actions/runs/5958993688/job/16164058824)

ldnetgate commented 1 year ago

Not sure why github shows changes still requested; I can't see any items pending in my display.

Updated the PR description based on the changes applied.

hajimehoshi commented 1 year ago

@TotallyGamerJet

hajimehoshi commented 1 year ago

@ldnetgate I would like to understand -std of -gcflags more. I think this is undocumented, so could you give me a pointer to a source code (probably in cmd/go)? I appreciate your hard work!

EDIT:

https://cs.opensource.google/go/go/+/master:src/cmd/compile/internal/noder/noder.go;l=298-306;drc=43e69b330a71e0d101bd57f0a1ea83bc4da259f3;bpv=0;bpt=1

So -std is necessary to enable //go:cgo_export_dynamic, right?

TotallyGamerJet commented 1 year ago

I was the one who suggested the flag. Yes it's necessary for that pragma.

The flag is defined here.

hajimehoshi commented 1 year ago

Thanks!