-
Notifications
You must be signed in to change notification settings - Fork 309
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
HPCC-33405: Add support for ubuntu arm #19571
base: master
Are you sure you want to change the base?
HPCC-33405: Add support for ubuntu arm #19571
Conversation
0670a96
to
0901101
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR bumps vcpkg to version 2025.02.13 and extends support for ARM architectures by updating workflows and build scripts. Key changes include:
- Adding new GitHub workflow jobs for ARM-based builds on Ubuntu and macOS.
- Modifying docker build scripts to incorporate an arm-specific tag postfix.
- Adjusting cache key generation to account for the tag postfix in ARM builds.
Reviewed Changes
File | Description |
---|---|
.github/workflows/build-vcpkg.yml | Added ARM build jobs for Ubuntu on different versions. |
.github/workflows/build-docker.yml | Updated inputs and commands to support ARM tagging and builds. |
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
.github/workflows/build-docker.yml:73
- Appending tag_postfix directly to the runner label may cause a mismatch with available GitHub runner labels. Consider using a separate input or matrix variable to set a valid runner label.
runs-on: ubuntu-22.04${{ inputs.tag_postfix }}
.github/workflows/build-docker.yml:160
- The expression '-D$plugin=ON' appears to contain an undefined or incorrectly referenced variable. Verify the intended variable substitution and update it to correctly reference the desired plugin setting.
cmake -G Ninja -S /hpcc-dev/${{ inputs.ln == true && 'LN' || 'HPCC-Platform' }} -B /hpcc-dev/build -DHPCC_SOURCE_DIR=/hpcc-dev/HPCC-Platform -DCMAKE_BUILD_TYPE=${{ inputs.build-type }} -DCONTAINERIZED=${{ inputs.containerized == true && 'ON' || 'OFF' }} -DCPACK_STRIP_FILES=${{ inputs.strip-files == true && 'ON' || 'OFF' }} ${{ inputs.single-package == true && '-DINCLUDE_PLUGINS=ON' || '-D$plugin=ON' }} ${{ inputs.cmake-configuration }} ${{ inputs.cmake-configuration-ex }} && \
3884826
to
02ce562
Compare
@jackdelv Can you look at the mongodb changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. One random comment.
@@ -19,8 +19,15 @@ elseif(APPLE) | |||
set(VCPKG_TARGET_TRIPLET "x64-osx" CACHE STRING "target triplet") | |||
endif() | |||
elseif(UNIX) | |||
execute_process(COMMAND uname -m OUTPUT_VARIABLE ARCHITECTURE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: If we ever want to do cross-compiling, this won't be correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree - I had a mental note to see if there was a better way than this (like the OSX flag).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GordonSmith MongoDB changes look good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GordonSmith the unit test failure looks significant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
It looks like there was some merging issues. Parquet tests were failing due primarily to duplicated lines in the ECL code. testing/regress/ecl/parquetTypes.ecl:65:
Actually, there are many more problems with that file. For this one file, it would be better to take the previous commit from Jack than actually merge. |
Signed-off-by: Gordon Smith <[email protected]>
Signed-off-by: Gordon Smith <[email protected]>
4ac9a44
to
3767da5
Compare
Bump vcpkg to 2025.02.13 with arm support Move antlr3c into vcpkg DC: Fix for arm64 register naming DC: Fix arm64 build issue in _cpyrevn DC: Move large zero array outside of functions (jlib file unittest) Rework dockerfiles/vcpkg/build.sh to support arm Signed-off-by Gordon Smith <[email protected]> Move large zero array outside of functions
215138d
to
87dd939
Compare
Move antlr3c into vcpkg
DC: Fix for arm64 register naming
Rework dockerfiles/vcpkg/build.sh to support arm
Signed-off-by Gordon Smith [email protected]
Type of change:
Checklist:
Smoketest:
Testing: