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

Upgrade pyodide compatibility to v0.24.1 #2402

Merged
merged 1 commit into from
Nov 11, 2023

Conversation

tomjakubowski
Copy link
Contributor

No description provided.

@tomjakubowski
Copy link
Contributor Author

Failing the same way on my Mac laptop.

running build_ext
configure: cmake -DCMAKE_C_COMPILER=/tmp/tmpk5cylkyn/cc -DCMAKE_CXX_COMPILER=/tmp/tmpk5cylkyn/c++ -DCMAKE_AR=/tmp/tmpk5cylkyn/ar -DCMAKE_C_COMPILER_AR=/tmp/tmpk5cylkyn/ar -DCMAKE_CXX_COMPILER_AR=/tmp/tmpk5cylkyn/ar --fresh --version -DCMAKE_CROSSCOMPILING_EMULATOR=/home/runner/work/_temp/64c13a07-2a13-4db9-85db-5254e1118b1b/emsdk-main/node/16.20.0_64bit/bin/node
configure: cmake -DCMAKE_C_COMPILER=/tmp/tmpk5cylkyn/cc -DCMAKE_CXX_COMPILER=/tmp/tmpk5cylkyn/c++ -DCMAKE_AR=/tmp/tmpk5cylkyn/ar -DCMAKE_C_COMPILER_AR=/tmp/tmpk5cylkyn/ar -DCMAKE_CXX_COMPILER_AR=/tmp/tmpk5cylkyn/ar --fresh /home/runner/work/perspective/perspective/python/perspective/dist -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=/home/runner/work/perspective/perspective/python/perspective/build/lib.emscripten_3_1_45_wasm32-cpython-311/perspective/table -DCMAKE_BUILD_TYPE=Release -DPSP_CPP_BUILD=1 -DPSP_WASM_BUILD=0 -DPSP_PYTHON_BUILD=1 -DPSP_PYTHON_VERSION=3.11 -DPython_ADDITIONAL_VERSIONS=3.11 -DPython_FIND_VERSION=3.11 -DPython_EXECUTABLE=/tmp/build-env-o61j6l19/bin/python -DPYTHON_LIBRARY=/root/repo/cpython/installs/python-3.11.3/lib -DPYTHON_INCLUDE_DIR=/root/repo/cpython/installs/python-3.11.3/include/python3.11 -DPython_ROOT_DIR=/tmp/build-env-o61j6l19 -DPython_ROOT=/tmp/build-env-o61j6l19 -DPSP_CMAKE_MODULE_PATH=/home/runner/work/perspective/perspective/python/perspective/dist/cmake -DPSP_CPP_SRC=/home/runner/work/perspective/perspective/python/perspective/dist -DPSP_PYTHON_SRC=/home/runner/work/perspective/perspective/python/perspective/dist/../perspective -DCMAKE_BUILD_TYPE=Release -DCMAKE_CROSSCOMPILING_EMULATOR=/home/runner/work/_temp/64c13a07-2a13-4db9-85db-5254e1118b1b/emsdk-main/node/16.20.0_64bit/bin/node
CMake Error at /opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/cmake/data/share/cmake-3.27/Modules/CMakeDetermineSystem.cmake:154 (message):
  Could not find toolchain file:
  /opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/pyodide_build/tools/cmake/Modules/Platform/Emscripten.cmake
Call Stack (most recent call first):
  CMakeLists.txt:2 (project)

Coming from line 2 of CMakeLists.txt. Not sure why it's looking under site-packages/pyodide_build for the toolchain file

@tomjakubowski
Copy link
Contributor Author

the problem was pyodide/pyodide#4216

I was able to finish a build on my laptop, but it was a native .so for some reason. Building here while I debug locally, just in case it's a result of a quite dirty working tree.

@timkpaine timkpaine added the dependencies Pull requests that update a dependency file label Nov 1, 2023
# PYTHON_VERSION and EMSCRIPTEN_VERSION are determined by PYODIDE_VERSION.
# The appropriate versions can be found in the Pyodide repodata.json
# "info" field, or in Makefile.envs:
# https://github.com/pyodide/pyodide/blob/main/Makefile.envs#L2
PYTHON_VERSION: 3.11.2
EMSCRIPTEN_VERSION: 3.1.32
PYTHON_VERSION: 3.11.3
Copy link
Member

Choose a reason for hiding this comment

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

this also changed? Is this durable to changes in setup-python?

Copy link
Member

Choose a reason for hiding this comment

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

ah meh # PYTHON_VERSION and EMSCRIPTEN_VERSION are determined by PYODIDE_VERSION.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all gucci?

@@ -8,9 +8,6 @@ auto-save-list
tramp
.\#*

# npm lock file
Copy link
Contributor Author

@tomjakubowski tomjakubowski Nov 1, 2023

Choose a reason for hiding this comment

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

unrelated change here --

I was rusty on perspective dev, used npm by mistake instead of yarn and, without the "new untracked file" cue in git status I didn't realize I had gone astray. Ended up with a busted node_modules directory from it

Of course we also don't want package-lock.json checked in. So I am happy to undo this, just wanted to raise the issue. Maybe there is a better way to help people like me avoid using npm in perspective dev

includes a bump to Emscripten 3.1.45 and a workaround for a missing
file upstream
@texodus texodus changed the title Bump pyodide to v0.24.1 Upgrade pyodide compatibility to v0.24.1 Nov 11, 2023
Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

I want to spend more time understanding the emscripten dependency of this build, specifically whether we are over-installing or over-building it.

@texodus texodus merged commit 9ad1b1e into finos:master Nov 11, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants