Skip to content

Expose emscripten_dir variable via config#321

Merged
agriyakhetarpal merged 5 commits intomainfrom
expose-emcc-path-via-config
Apr 3, 2026
Merged

Expose emscripten_dir variable via config#321
agriyakhetarpal merged 5 commits intomainfrom
expose-emcc-path-via-config

Conversation

@agriyakhetarpal
Copy link
Copy Markdown
Member

@agriyakhetarpal agriyakhetarpal commented Apr 3, 2026

Make the Emscripten directory available via config so that we don't have to compute the xbuildenv_path mess and go back and forth to find out where the Emscripten compiler binary and other tool binaries are located.

  • Changelog

Comment thread pyodide_build/config.py Outdated
Comment thread pyodide_build/config.py
Comment on lines +118 to +121
# We compute this here rather than via Makefile.envs or DEFAULT_CONFIG so
# that the path is always the resolved, versioned path – not the shared
# xbuildenv symlink, because there are some issues I have noticed with
# concurrent builds especially when being used in cibuildwheel etc.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you elaborate on this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is because the Emscripten version was being mis-found during concurrent builds. For example, in pypa/cibuildwheel#2800, we were previously relying on the symlinks. When running the test suite in parallel, it would find Emscripten 3.1.58 when we would need 4.0.9 and cause weird failures in the tests. I kept the comment a bit vague, though, indeed. Happy to add more details if you need me to?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. Makes sense, thanks!

Comment thread pyodide_build/tests/test_config.py Outdated
@agriyakhetarpal agriyakhetarpal changed the title Expose emcc_path variable via config Expose emscripten_dir variable via config Apr 3, 2026
Copy link
Copy Markdown
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks!

@agriyakhetarpal
Copy link
Copy Markdown
Member Author

agriyakhetarpal commented Apr 3, 2026

Thanks for the review! Leaving a note here about the comment, if we want to take a look at this later:

  • We compute the Emscripten directory from pyodide_root. We know that the on-disk layout is:

    {xbuildenv_root}/
        xbuildenv  ->  {version}/          # symlink, which is updated on each install
        {version}/
            xbuildenv/
                pyodide-root/              # which is `self.pyodide_root`
            emsdk/
                upstream/
                    emscripten/            # what we want
    
  • We compute this from self.pyodide_root (which is already a fully resolved, version-specific path) rather than via Makefile.envs or DEFAULT_CONFIG for two reasons:

    1. Makefile.envs does not know about the emsdk location.
    2. The xbuildenv symlink is rewritten on each pyodide xbuildenv install call. If multiple builds for different Pyodide versions run concurrently (e.g. in cibuildwheel, when running the test suite in parallel), re-resolving through the symlink at a later point could yield the wrong versioned directory. Using self.pyodide_root, which was resolved at construction time, gives us a stable, race-free path.

@agriyakhetarpal agriyakhetarpal merged commit 81758f5 into main Apr 3, 2026
7 checks passed
@agriyakhetarpal agriyakhetarpal deleted the expose-emcc-path-via-config branch April 3, 2026 07:46
@ryanking13
Copy link
Copy Markdown
Member

The xbuildenv symlink is rewritten on each pyodide xbuildenv install call. If multiple builds for different Pyodide versions run concurrently (e.g. in cibuildwheel, when running the test suite in parallel), re-resolving through the symlink at a later point could yield the wrong versioned directory. Using self.pyodide_root, which was resolved at construction time, gives us a stable, race-free path.

Thanks for the investigation. Actually, I guess this is something that we need to think more carefully about, not just for emscripten but for other config variables as well. I personally never considered concurrent builds when designing xbuildenv installation. If cibuildwheel is using parallel builds a lot, we should be make pyodide-build more concurrent-robust.

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