[BSE-4386] Add Linux ARM Support #83
Conversation
| @@ -70,7 +74,7 @@ jobs: | |||
| - name: Upload Conda Package | |||
| uses: actions/upload-artifact@v4 | |||
| with: | |||
| name: bodo-conda-linux-${{ inputs.python-version }}-community | |||
| name: bodo-conda-${{ inputs.python-version }}-community-linux-${{ inputs.arm && 'arm' || 'x86' }} | |||
There was a problem hiding this comment.
We don't need community in the name anymore.
| jobs: | ||
| build_bodo_wheels: | ||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
| name: Build Bodo Wheels for ${{ inputs.os }} | ||
| name: Build Bodo Wheels for ${{ inputs.name }} |
There was a problem hiding this comment.
Thank you :). Having name is more descriptive and easier than numbers 13 and 14
| @@ -41,7 +41,7 @@ jobs: | |||
| - name: 'Download the Bodo conda' | |||
| uses: actions/download-artifact@v4 | |||
| with: | |||
| name: bodo-conda-linux-3.12-community | |||
| name: bodo-conda-3.12-community-linux-x86 | |||
There was a problem hiding this comment.
Same. let's remove community
| bodo_version: ${{ needs.get_version.outputs.bodo_version }} | ||
| secrets: inherit | ||
|
|
||
| build_bodo_macos_wheels: | ||
| build_bodo_other: |
There was a problem hiding this comment.
why change this name? It's still only Mac?
There was a problem hiding this comment.
In expectation of Windows :)
| # Don't cancel other jobs if one fails | ||
| fail-fast: false | ||
| matrix: | ||
| # On pull requests, only test building for 3.12 |
There was a problem hiding this comment.
why is this the only one we testing its build with PRs?
There was a problem hiding this comment.
So its faster. Plus, GitHub limits the number of concurrent MacOS agents, and so starting 3 at once is too much
| std::pair<T, T> get_lower_upper_kth_parallel(std::vector<T, Alloc> my_array, | ||
| int64_t total_size, int myrank, | ||
| int n_pes, int64_t k, | ||
| int type_enum); | ||
| int type_enum) { | ||
| MPI_Datatype mpi_typ = get_MPI_typ(type_enum); |
There was a problem hiding this comment.
It was causing some compiler errors and warnings in the arm build
| } // namespace bodo | ||
| } // namespace bodo::tests | ||
|
|
||
| // Create an array of the desired type without directly setting the null bit |
|
@IsaacWarren not sure if you looked at this last time, but I fixed the issue in CI so it should be ready now |
| @@ -37,7 +37,7 @@ | |||
| create or replace function test_regex_udf(A varchar) RETURNS DOUBLE LANGUAGE JAVASCRIPT AS | |||
| $$ | |||
| try { | |||
| return parseInt(A.match(/(\d+).*?(\d+).*?(\d+)/)[2]); | |||
| return parseInt(A.match(/(\\d+).*?(\\d+).*?(\\d+)/)[2]); | |||
There was a problem hiding this comment.
The file still runs (technically no tests cause I dont have V8 installed locally) but I would expect it to fail if this was wrong since this is a docstring
Changes included in this PR
Add support for Linux on the ARM / Aarch64 architecture. All dependencies already support it, its just a matter of setting up the build correctly
NOTE: I'm a bit blocked on the Azure testing portion, but I want to get the packaging parts to review before going OOO, so opening it up now.
Testing strategy
User facing changes
Checklist
[run CI]in your commit message.