Skip to content

software: add toolchain integration for Emscripten #960

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

Merged
merged 5 commits into from
Jul 20, 2025

Conversation

whitequark
Copy link
Member

@whitequark whitequark commented Jul 18, 2025

To do:

  • get_bitstream wasn't async but is async now, something downstream probably broke
  • might be nice to use asyncio.create_subprocess_exec instead of Popen to avoid blocking when running *nix/Windows too

@whitequark whitequark changed the title software: add toolchain integration for Emscripten. software: add toolchain integration for Emscripten Jul 18, 2025
@whitequark whitequark force-pushed the pyodide branch 7 times, most recently from 0423618 to 44a0245 Compare July 19, 2025 23:27
@Sazzach
Copy link
Contributor

Sazzach commented Jul 20, 2025

I reviewed all the changes. Everything looks good to me except for two small issues (see diff), and two questions I have (see questions).
fixes.txt

In build_plan.py you added an import for shelx, but it isn't used anywhere.
Also in build_plan.py, the type signature of the version property of SystemTool, no longer matches the parent class. Would it make sense to change Tool.version from Optional[tuple[int, ...]] to Optional[tuple[int|str, ...]]?

@whitequark whitequark force-pushed the pyodide branch 2 times, most recently from 986d4f4 to e0324cc Compare July 20, 2025 12:01
@whitequark
Copy link
Member Author

Thanks, all good calls.

In build_plan.py you added an import for shelx, but it isn't used anywhere.

This is a remnant from an earlier implementation.

Also in build_plan.py, the type signature of the version property of SystemTool, no longer matches the parent class. Would it make sense to change Tool.version from Optional[tuple[int, ...]] to Optional[tuple[int|str, ...]]?

This is a remnant from an earlier bug. The version functions actually all return strings (they aren't compared, but they are hashed). It's kind of an odd API. It should be Optional[tuple[str, ...]]

@whitequark whitequark force-pushed the pyodide branch 2 times, most recently from 76e34f1 to c3fcc3a Compare July 20, 2025 12:25
Primarily useful for the Emscripten port, but can also be used to
amortize startup latency in other cases.
Primarily useful for Emscripten port.
This integration expects a global object like this:

    globalThis.glasgowToolchain = {
        packages: {
            'yosys': '0.55',
            'nextpnr-ice40': '0.8'
        },
        available(packageName) {
            return packageName in this.packages;
        },
        version(packageName) {
            return this.packages[packageName];
        },
        async build(files, scriptName, writeLine) {
            writeLine("starting build...\n")
            // ... do the build...
            return {code: 0, files: ...};
        }
    }
Disconnecting the device seems to produce a `NotFoundError` with
"The device was disconnected." in the message. This isn't a great
solution but I don't have anything better.
@whitequark
Copy link
Member Author

OK, I think this is more or less complete. There are still things that don't work on Emscripten but I think we're better off merging this and fixing the rest incrementally. You can get pretty far with the code in this PR. (The main thing that's still broken is Windows support, and I haven't got a faintest clue why.)

@Sazzach
Copy link
Contributor

Sazzach commented Jul 20, 2025

Thanks, all good calls.

No problem, happy to contribute!

OK, I think this is more or less complete. There are still things that don't work on Emscripten but I think we're better off merging this and fixing the rest incrementally. You can get pretty far with the code in this PR. (The main thing that's still broken is Windows support, and I haven't got a faintest clue why.)

Nice! I also just took a look at the most recent commit (c2be584) and it LTGM.

@whitequark whitequark added this pull request to the merge queue Jul 20, 2025
Merged via the queue into GlasgowEmbedded:main with commit ec2f589 Jul 20, 2025
12 checks passed
@whitequark whitequark deleted the pyodide branch July 20, 2025 20:07
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.

2 participants