Skip to content

Win arm64 support#46

Open
salmanmkc wants to merge 6 commits intogristlabs:mainfrom
salmanmkc:salmanmkc-win-arm64-support
Open

Win arm64 support#46
salmanmkc wants to merge 6 commits intogristlabs:mainfrom
salmanmkc:salmanmkc-win-arm64-support

Conversation

@salmanmkc
Copy link
Copy Markdown

@salmanmkc salmanmkc commented Aug 11, 2024

Not sure if this is all that is needed, looks like there's some binding for SQLLite3?

Also would need self-hosted arm agents I think, I don't think GitHub has it yet

@SleepyLeslie
Copy link
Copy Markdown
Collaborator

Hi @salmanmkc, thanks for your contribution. Unfortunately it seems that GitHub indeed doesn't offer arm64 Windows runners (actions/runner-images#768) at this moment. I don't think Grist Labs plans to connect a self-hosted runner, so we likely couldn't merge this PR as of now. Leaving it to @paulfitz for a decision.

Plus, SQLite3 compatibility is something you must explicitly address. The SQLite3 Node.js binding is architecture-specific. During yarn install, it uses prebuild-install to download a prebuilt binding (see https://www.npmjs.com/package/sqlite3). The hosted builds do not cover Windows arm64, so you will have to build it manually. In fact we are already doing this for Windows ia32:

- name: Fix Windows x86 sqlite3 binding
if: steps.yarn-cache.outputs.cache-hit != 'true' && startsWith(matrix.os, 'windows') && matrix.target == 'x86'
run: yarn upgrade sqlite3
# Prebuilt binding is for x64. We must build from source for x86 target.
# See:
# https://stackoverflow.com/questions/72553650/how-to-get-node-sqlite3-working-on-mac-m1
# https://yarnpkg.com/lang/en/docs/envvars/#toc-npm-config
env:
npm_config_build_from_source: true
npm_config_target_arch: ia32
npm_config_fallback_to_build: true

For some reasons if fallback_to_build is not set it will still download the x64 binding even if target_arch is set to ia32, so we actually manually build the ia32 binding in our workflow.

Furthermore, you will need to resolve the merge conflict caused by #45.

@salmanmkc
Copy link
Copy Markdown
Author

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