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

Compiling with emscripten version 3.1.52 does not work #16160

Open
ethanaobrien opened this issue Jan 23, 2024 · 4 comments
Open

Compiling with emscripten version 3.1.52 does not work #16160

ethanaobrien opened this issue Jan 23, 2024 · 4 comments

Comments

@ethanaobrien
Copy link
Contributor

ethanaobrien commented Jan 23, 2024

(see emscripten-core/emscripten#21128)

As said in the title, Compiling with emscripten version 3.1.52 does not work because the emscripten compiler actually checks to see if input files are what they are now (emscripten-core/emscripten#20922) (It appears the initial emscripten port was incorrect in that field)

Before I went and did anything I wanted input on what should be done. There are 2 options from my point of view

  1. State compiling with emscripten 3.1.52 and up is not supported. Though note this will quickly make the emscripten builds outdated and isnt optimal for the long term
  2. Enable static linking for all cores, and properly name the files .a as emscripten expects, as well as change the targets in the makefile and dist-cores script

In my project (EmulatorJS) I have chosen the second option, since I try to stay with up to date emscripten, but wanted input before I went and submited about 20 prs.

Thanks!

@ethanaobrien ethanaobrien changed the title Compiling with emscripten version 2.1.52 does not work Compiling with emscripten version 3.1.52 does not work Jan 23, 2024
@JoeOsborn
Copy link
Contributor

Ahhhh, I wish I had thought about this when I filed PRs to bring cores up to date with Emscripten 3.1.29’s build process. It might be easier to patch the makefile to rename the bc file to a .a suffix , though I suppose thats’s a bit of a hack.

Are you on the retroarch discord? I’m also doing a project using emscripten-compiled RA + cores. Happy to chat/share notes anytime.

@ethanaobrien
Copy link
Contributor Author

It might be easier to patch the makefile to rename the bc file to a .a suffix , though I suppose thats’s a bit of a hack.

I attempted this originally, though static linking is not set consistently across cores. Some cores build a real .bc file while some just build a .a file

What I was thinking was opening a pr in each core that emscripten supports, and opening a pr here, listing all those prs are dependencies

Are you on the retroarch discord? I’m also doing a project using emscripten-compiled RA + cores. Happy to chat/share notes anytime.

I don't join discord servers too often, so I'm not in it, though it would be interesting to chat with you. There are a few things I'd like to make available on the web that you might know more than i do about.

@JoeOsborn
Copy link
Contributor

JoeOsborn commented Jan 27, 2024

Interesting, I thought all emscripten cores used static linking (if any don’t that might be a bug, since emscripten doesn’t really support dynamic linking that well). And a lot of the pipeline does seem to assume archive files are coming in. Which ones are you seeing produce actual bc files?

One thing I learned about RA’s CI pipeline is that if any cores don’t compile cleanly the whole platform doesn’t get released, so coordinating with @LibretroAdmin is really important for these ecosystem level changes.

@ethanaobrien
Copy link
Contributor Author

Interesting, I thought all emscripten cores used static linking

Nope. About half don't actually - since it doesn't actually check the input file type I didn't even know this.

Which ones are you seeing produce actual bc files?

I don't have a full list with me, though you can check through the repos mentioned here and see which cores I've had to enable it on. I can say the only core I missed enabling static linking on was Gearcoleco and was quite surprised when I found out it was disabled.

if any cores don’t compile cleanly the whole platform doesn’t get released

Exactly why I opened this issue :)

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

No branches or pull requests

2 participants